diff --git a/listings/ch21-web-server/listing-21-23/src/lib.rs b/listings/ch21-web-server/listing-21-23/src/lib.rs index eea339b027..b4d81571bc 100644 --- a/listings/ch21-web-server/listing-21-23/src/lib.rs +++ b/listings/ch21-web-server/listing-21-23/src/lib.rs @@ -61,12 +61,10 @@ impl Drop for ThreadPool { fn drop(&mut self) { drop(self.sender.take()); - for worker in &mut self.workers { + for worker in self.workers.drain(..) { println!("Shutting down worker {}", worker.id); - if let Some(thread) = worker.thread.take() { - thread.join().unwrap(); - } + worker.thread.join().unwrap(); } } } @@ -74,7 +72,7 @@ impl Drop for ThreadPool { struct Worker { id: usize, - thread: Option>, + thread: thread::JoinHandle<()>, } impl Worker { @@ -87,9 +85,6 @@ impl Worker { job(); }); - Worker { - id, - thread: Some(thread), - } + Worker { id, thread } } } diff --git a/listings/ch21-web-server/listing-21-24/src/lib.rs b/listings/ch21-web-server/listing-21-24/src/lib.rs index 28c0dea262..10a50ef7b6 100644 --- a/listings/ch21-web-server/listing-21-24/src/lib.rs +++ b/listings/ch21-web-server/listing-21-24/src/lib.rs @@ -51,19 +51,17 @@ impl Drop for ThreadPool { fn drop(&mut self) { drop(self.sender.take()); - for worker in &mut self.workers { + for worker in self.workers.drain(..) { println!("Shutting down worker {}", worker.id); - if let Some(thread) = worker.thread.take() { - thread.join().unwrap(); - } + worker.thread.join().unwrap(); } } } struct Worker { id: usize, - thread: Option>, + thread: thread::JoinHandle<()>, } // ANCHOR: here @@ -85,10 +83,7 @@ impl Worker { } }); - Worker { - id, - thread: Some(thread), - } + Worker { id, thread } } } // ANCHOR_END: here diff --git a/listings/ch21-web-server/listing-21-25/src/lib.rs b/listings/ch21-web-server/listing-21-25/src/lib.rs index 54c0489abf..2727118495 100644 --- a/listings/ch21-web-server/listing-21-25/src/lib.rs +++ b/listings/ch21-web-server/listing-21-25/src/lib.rs @@ -51,19 +51,17 @@ impl Drop for ThreadPool { fn drop(&mut self) { drop(self.sender.take()); - for worker in &mut self.workers { + for worker in self.workers.drain(..) { println!("Shutting down worker {}", worker.id); - if let Some(thread) = worker.thread.take() { - thread.join().unwrap(); - } + worker.thread.join().unwrap(); } } } struct Worker { id: usize, - thread: Option>, + thread: thread::JoinHandle<()>, } impl Worker { @@ -84,9 +82,6 @@ impl Worker { } }); - Worker { - id, - thread: Some(thread), - } + Worker { id, thread } } } diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/404.html b/listings/ch21-web-server/no-listing-04-update-drop-definition/404.html similarity index 100% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/404.html rename to listings/ch21-web-server/no-listing-04-update-drop-definition/404.html diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/Cargo.lock b/listings/ch21-web-server/no-listing-04-update-drop-definition/Cargo.lock similarity index 100% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/Cargo.lock rename to listings/ch21-web-server/no-listing-04-update-drop-definition/Cargo.lock diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/Cargo.toml b/listings/ch21-web-server/no-listing-04-update-drop-definition/Cargo.toml similarity index 100% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/Cargo.toml rename to listings/ch21-web-server/no-listing-04-update-drop-definition/Cargo.toml diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/hello.html b/listings/ch21-web-server/no-listing-04-update-drop-definition/hello.html similarity index 100% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/hello.html rename to listings/ch21-web-server/no-listing-04-update-drop-definition/hello.html diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/output.txt b/listings/ch21-web-server/no-listing-04-update-drop-definition/output.txt similarity index 100% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/output.txt rename to listings/ch21-web-server/no-listing-04-update-drop-definition/output.txt diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/src/lib.rs b/listings/ch21-web-server/no-listing-04-update-drop-definition/src/lib.rs similarity index 94% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/src/lib.rs rename to listings/ch21-web-server/no-listing-04-update-drop-definition/src/lib.rs index 6ef710a26b..86f92766cf 100644 --- a/listings/ch21-web-server/no-listing-04-update-worker-definition/src/lib.rs +++ b/listings/ch21-web-server/no-listing-04-update-drop-definition/src/lib.rs @@ -44,22 +44,22 @@ impl ThreadPool { } } +// ANCHOR: here impl Drop for ThreadPool { fn drop(&mut self) { - for worker in &mut self.workers { + for worker in self.workers.drain(..) { println!("Shutting down worker {}", worker.id); worker.thread.join().unwrap(); } } } +// ANCHOR_END: here -// ANCHOR: here struct Worker { id: usize, - thread: Option>, + thread: thread::JoinHandle<()>, } -// ANCHOR_END: here impl Worker { fn new(id: usize, receiver: Arc>>) -> Worker { diff --git a/listings/ch21-web-server/no-listing-04-update-worker-definition/src/main.rs b/listings/ch21-web-server/no-listing-04-update-drop-definition/src/main.rs similarity index 100% rename from listings/ch21-web-server/no-listing-04-update-worker-definition/src/main.rs rename to listings/ch21-web-server/no-listing-04-update-drop-definition/src/main.rs diff --git a/src/ch21-03-graceful-shutdown-and-cleanup.md b/src/ch21-03-graceful-shutdown-and-cleanup.md index 36e85f32b3..fc4fbada88 100644 --- a/src/ch21-03-graceful-shutdown-and-cleanup.md +++ b/src/ch21-03-graceful-shutdown-and-cleanup.md @@ -47,63 +47,41 @@ Here is the error we get when we compile this code: {{#include ../listings/ch21-web-server/listing-21-22/output.txt}} ``` -The error tells us we can’t call `join` because we only have a mutable borrow -of each `worker` and `join` takes ownership of its argument. To solve this -issue, we need to move the thread out of the `Worker` instance that owns -`thread` so `join` can consume the thread. We did this in Listing 17-15: if -`Worker` holds an `Option>` instead, we can call the -`take` method on the `Option` to move the value out of the `Some` variant and -leave a `None` variant in its place. In other words, a `Worker` that is running -will have a `Some` variant in `thread`, and when we want to clean up a -`Worker`, we’ll replace `Some` with `None` so the `Worker` doesn’t have a -thread to run. - -So we know we want to update the definition of `Worker` like this: +The error tells us we can’t call `join` because we only have a mutable borrow of +each `worker` and `join` takes ownership of its argument. To solve this issue, +we need to move the thread out of the `Worker` instance that owns `thread` so +`join` can consume the thread. One way to do this is by taking the same approach +we did in Listing 18-15. If `Worker` held an `Option>`, +we could call the `take` method on the `Option` to move the value out of the +`Some` variant and leave a `None` variant in its place. In other words, a +`Worker` that is running would have a `Some` variant in `thread`, and when we +wanted to clean up a `Worker`, we would replace `Some` with `None` so the +`Worker` doesn’t have a thread to run. + +However, the _only_ time this would come up would be when dropping the `Worker`. +In exchange, we would have to deal with an `Option>` +everywhere we access `worker.thread`. Idiomatic Rust uses `Option` quite a bit, +but when you find yourself wrapping something in `Option` as a workaround even +though you know the item will always be present, it is a good idea to look for +alternative approaches. They can make your code cleaner and less error-prone. + +In this case, there is a better alternative: the `Vec::drain` method. It accepts +a range parameter to specify which items to remove from the `Vec`, and returns +an iterator of those items. Passing the `..` range syntax will remove *every* +value from the `Vec`. + +So we need to update the `ThreadPool` `drop` implementation like this: ```rust,ignore,does_not_compile -{{#rustdoc_include ../listings/ch21-web-server/no-listing-04-update-worker-definition/src/lib.rs:here}} +{{#rustdoc_include ../listings/ch21-web-server/no-listing-04-update-drop-definition/src/lib.rs:here}} ``` -Now let’s lean on the compiler to find the other places that need to change. -Checking this code, we get two errors: - -```console -{{#include ../listings/ch21-web-server/no-listing-04-update-worker-definition/output.txt}} -``` - -Let’s address the second error, which points to the code at the end of -`Worker::new`; we need to wrap the `thread` value in `Some` when we create a -new `Worker`. Make the following changes to fix this error: - -- -```rust,ignore,does_not_compile -{{#rustdoc_include ../listings/ch21-web-server/no-listing-05-fix-worker-new/src/lib.rs:here}} -``` - - - -The first error is in our `Drop` implementation. We mentioned earlier that we -intended to call `take` on the `Option` value to move `thread` out of `worker`. -The following changes will do so: - -- -```rust,ignore,not_desired_behavior -{{#rustdoc_include ../listings/ch21-web-server/no-listing-06-fix-threadpool-drop/src/lib.rs:here}} -``` - - - -As discussed in Chapter 18, the `take` method on `Option` takes the `Some` -variant out and leaves `None` in its place. We’re using `if let` to destructure -the `Some` and get the thread; then we call `join` on the thread. If a worker’s -thread is already `None`, we know that worker has already had its thread -cleaned up, so nothing happens in that case. +This resolves the compiler error and does not require any other changes to our +code. ### Signaling to the Threads to Stop Listening for Jobs @@ -120,9 +98,9 @@ implementation and then a change in the `Worker` loop. First, we’ll change the `ThreadPool` `drop` implementation to explicitly drop the `sender` before waiting for the threads to finish. Listing 21-23 shows the -changes to `ThreadPool` to explicitly drop `sender`. We use the same `Option` -and `take` technique as we did with the thread to be able to move `sender` out -of `ThreadPool`: +changes to `ThreadPool` to explicitly drop `sender`. Unlike with the `workers`, +here we *do* need to use an `Option` to be able to move `sender` out of +`ThreadPool` with `Option::take`.