From 43fec3faa84883d546ab0d09ffcbb83a116ee050 Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Thu, 26 Jul 2018 23:02:47 +0900 Subject: [PATCH 1/7] Use vector for dtors in thread --- src/thread.rs | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/thread.rs b/src/thread.rs index a635801..e26085d 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -159,25 +159,11 @@ where pub struct Scope<'env> { /// The list of the deferred functions and thread join jobs. - dtors: RefCell>>>, + dtors: RefCell> + 'env>>>, // !Send + !Sync _marker: PhantomData<*const ()>, } -struct DtorChain<'env, T> { - dtor: Box + 'env>, - next: Option>>, -} - -impl<'env, T> DtorChain<'env, T> { - pub fn pop(chain: &mut Option>) -> Option + 'env>> { - chain.take().map(|mut node| { - *chain = node.next.take().map(|b| *b); - node.dtor - }) - } -} - struct JoinState { join_handle: thread::JoinHandle<()>, result: ScopedThreadResult @@ -237,7 +223,7 @@ where F: FnOnce(&Scope<'env>) -> R, { let mut scope = Scope { - dtors: RefCell::new(None), + dtors: RefCell::new(Vec::new()), _marker: PhantomData, }; let ret = f(&scope); @@ -262,9 +248,13 @@ impl<'env> Scope<'env> { // and, if any dtor panics, can be resumed in the unwinding this causes. By initially running // the method outside of any destructor, we avoid any leakage problems due to // @rust-lang/rust#14875. + // + // FIXME(jeehoonkang): @rust-lang/rust#14875 is fixed, so maybe we can remove the above comment. + // But I'd like to write tests to check it before removing the comment. fn drop_all(&mut self) -> thread::Result<()> { let mut ret = Ok(()); - while let Some(dtor) = DtorChain::pop(&mut self.dtors.borrow_mut()) { + let mut dtors = self.dtors.borrow_mut(); + for dtor in dtors.drain(..) { ret = ret.and(dtor.call_box()); } ret @@ -274,11 +264,7 @@ impl<'env> Scope<'env> { where F: (FnOnce() -> thread::Result<()>) + 'env, { - let mut dtors = self.dtors.borrow_mut(); - *dtors = Some(DtorChain { - dtor: Box::new(f), - next: dtors.take().map(Box::new), - }); + self.dtors.borrow_mut().push(Box::new(f)); } /// Schedule code to be executed when exiting the scope. @@ -407,8 +393,11 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { impl<'env> Drop for Scope<'env> { fn drop(&mut self) { - // Actually, there should be no deferred functions left to be run. - self.drop_all().unwrap(); + // Note that `self.dtors` can be non-empty when the code inside a `scope()` panics and + // `drop()` is called in unwinding. Even if it's the case, we will join the unjoined threads + // and execute the deferred functions. We ignore panics from any threads or functions + // because we're in course of unwinding anyway. + let _ = self.drop_all(); } } From bb139b6d5d10bc3cf04f9c536b1dcbfa0b07abb4 Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Fri, 27 Jul 2018 10:44:15 +0900 Subject: [PATCH 2/7] Add tests --- src/thread.rs | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/thread.rs b/src/thread.rs index e26085d..f022534 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -402,3 +402,85 @@ impl<'env> Drop for Scope<'env> { } type ScopedThreadResult = Arc>>; + +#[cfg(test)] +mod tests { + use super::*; + use std::{thread, time}; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + + const TIMES: usize = 10; + const SMALL_STACK_SIZE: usize = 20; + + #[test] + fn join() { + let counter = AtomicUsize::new(0); + scope(|scope| { + let handle = scope.spawn(|| { + counter.store(1, Ordering::Relaxed); + }); + assert!(handle.join().is_ok()); + + let panic_handle = scope.spawn(|| { + panic!("\"My honey is running out!\", said Pooh."); + }); + assert!(panic_handle.join().is_err()); + }).unwrap(); + } + + #[test] + fn counter() { + let counter = AtomicUsize::new(0); + scope(|scope| { + for _ in 0..TIMES { + scope.spawn(|| { + counter.fetch_add(1, Ordering::Relaxed); + }); + } + }).unwrap(); + assert_eq!(TIMES, counter.load(Ordering::Relaxed)); + } + + #[test] + fn counter_builder() { + let counter = AtomicUsize::new(0); + scope(|scope| { + for i in 0..TIMES { + scope.builder() + .name(format!("child-{}", i)) + .stack_size(SMALL_STACK_SIZE) + .spawn(|| { + counter.fetch_add(1, Ordering::Relaxed); + }) + .unwrap(); + } + }).unwrap(); + assert_eq!(TIMES, counter.load(Ordering::Relaxed)); + } + + #[test] + fn counter_defer_panic() { + let counter = AtomicUsize::new(0); + let result = scope(|scope| { + scope.spawn(|| { + panic!("\"My honey is running out!\", said Pooh."); + }); + thread::sleep(time::Duration::from_millis(100)); + + for _ in 0..TIMES { + scope.defer(|| { + counter.fetch_add(1, Ordering::Relaxed); + }); + } + + for _ in 0..TIMES { + scope.spawn(|| { + counter.fetch_add(1, Ordering::Relaxed); + }); + } + }); + assert_eq!(TIMES + TIMES, counter.load(Ordering::Relaxed)); + assert!(result.is_err()); + } +} From 54568104e01c6d20f2f0cd4fedf7d4ecb105da9d Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Fri, 27 Jul 2018 11:06:31 +0900 Subject: [PATCH 3/7] Fix semantics of defer and join --- src/thread.rs | 105 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 28 deletions(-) diff --git a/src/thread.rs b/src/thread.rs index f022534..2c8da37 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -106,17 +106,17 @@ /// ``` /// /// Much more straightforward. - +use std::any::Any; use std::cell::RefCell; use std::fmt; +use std::io; use std::marker::PhantomData; use std::mem; use std::ops::DerefMut; use std::panic::{self, AssertUnwindSafe}; use std::rc::Rc; -use std::thread; -use std::io; use std::sync::{Arc, Mutex}; +use std::thread; #[doc(hidden)] trait FnBox { @@ -158,8 +158,12 @@ where } pub struct Scope<'env> { - /// The list of the deferred functions and thread join jobs. - dtors: RefCell> + 'env>>>, + /// The list of the thread join jobs. + joins: RefCell> + 'env>>>, + /// The list of the deferred functions. + funcs: RefCell + 'env>>>, + /// Thread and function panics invoked so far. + panics: RefCell>>, // !Send + !Sync _marker: PhantomData<*const ()>, } @@ -223,11 +227,25 @@ where F: FnOnce(&Scope<'env>) -> R, { let mut scope = Scope { - dtors: RefCell::new(Vec::new()), + joins: RefCell::new(Vec::new()), + funcs: RefCell::new(Vec::new()), + panics: RefCell::new(Vec::new()), _marker: PhantomData, }; + + // Executes the scoped function. let ret = f(&scope); - scope.drop_all()?; + + // Executes deferred functions and joins threads. + scope.drop_all(); + let panic = scope.panics.borrow_mut().pop(); + + // If any of the functions and threads panicked, returns the panic's payload. + if let Some(payload) = panic { + return Err(payload); + } + + // Returns the result of the scoped function. Ok(ret) } @@ -251,31 +269,33 @@ impl<'env> Scope<'env> { // // FIXME(jeehoonkang): @rust-lang/rust#14875 is fixed, so maybe we can remove the above comment. // But I'd like to write tests to check it before removing the comment. - fn drop_all(&mut self) -> thread::Result<()> { - let mut ret = Ok(()); - let mut dtors = self.dtors.borrow_mut(); - for dtor in dtors.drain(..) { - ret = ret.and(dtor.call_box()); + fn drop_all(&mut self) { + let mut funcs = self.funcs.borrow_mut(); + for func in funcs.drain(..) { + let result = panic::catch_unwind(AssertUnwindSafe(|| func.call_box())); + if let Err(payload) = result { + self.panics.borrow_mut().push(payload); + } } - ret - } - fn defer_inner(&self, f: F) - where - F: (FnOnce() -> thread::Result<()>) + 'env, - { - self.dtors.borrow_mut().push(Box::new(f)); + let mut joins = self.joins.borrow_mut(); + for join in joins.drain(..) { + let result = join.call_box(); + if let Err(payload) = result { + self.panics.borrow_mut().push(payload); + } + } } /// Schedule code to be executed when exiting the scope. /// /// This is akin to having a destructor on the stack, except that it is *guaranteed* to be - /// run. It is guaranteed that the function is called after all the spawned threads are joined. + /// run. It is guaranteed that the function is called before all the spawned threads are joined. pub fn defer(&self, f: F) where F: FnOnce() + 'env, { - self.defer_inner(move || panic::catch_unwind(AssertUnwindSafe(f))); + self.funcs.borrow_mut().push(Box::new(f)); } /// Create a scoped thread. @@ -349,14 +369,14 @@ impl<'scope, 'env: 'scope> ScopedThreadBuilder<'scope, 'env> { let deferred_handle = Rc::new(RefCell::new(Some(join_state))); let my_handle = deferred_handle.clone(); - self.scope.defer_inner(move || { + self.scope.joins.borrow_mut().push(Box::new(move || { let state = deferred_handle.borrow_mut().deref_mut().take(); if let Some(state) = state { state.join().map(|_| ()) } else { Ok(()) } - }); + })); Ok(ScopedJoinHandle { inner: my_handle, @@ -395,9 +415,11 @@ impl<'env> Drop for Scope<'env> { fn drop(&mut self) { // Note that `self.dtors` can be non-empty when the code inside a `scope()` panics and // `drop()` is called in unwinding. Even if it's the case, we will join the unjoined threads - // and execute the deferred functions. We ignore panics from any threads or functions - // because we're in course of unwinding anyway. - let _ = self.drop_all(); + // and execute the deferred functions. + // + // We ignore panics from any threads or functions because we're in course of unwinding + // anyway. + self.drop_all(); } } @@ -406,9 +428,9 @@ type ScopedThreadResult = Arc>>; #[cfg(test)] mod tests { use super::*; - use std::{thread, time}; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; + use std::{thread, time}; const TIMES: usize = 10; const SMALL_STACK_SIZE: usize = 20; @@ -427,6 +449,9 @@ mod tests { }); assert!(panic_handle.join().is_err()); }).unwrap(); + + // There should be sufficient synchronization. + assert_eq!(1, counter.load(Ordering::Relaxed)); } #[test] @@ -439,6 +464,7 @@ mod tests { }); } }).unwrap(); + assert_eq!(TIMES, counter.load(Ordering::Relaxed)); } @@ -447,7 +473,8 @@ mod tests { let counter = AtomicUsize::new(0); scope(|scope| { for i in 0..TIMES { - scope.builder() + scope + .builder() .name(format!("child-{}", i)) .stack_size(SMALL_STACK_SIZE) .spawn(|| { @@ -456,6 +483,7 @@ mod tests { .unwrap(); } }).unwrap(); + assert_eq!(TIMES, counter.load(Ordering::Relaxed)); } @@ -480,7 +508,28 @@ mod tests { }); } }); + assert_eq!(TIMES + TIMES, counter.load(Ordering::Relaxed)); assert!(result.is_err()); } + + #[test] + fn counter_defer_join() { + let counter = AtomicUsize::new(0); + scope(|scope| { + let _handle = scope.spawn(|| { + counter.store(1, Ordering::Relaxed); + }); + + // FIXME(jeehoonkang): currently we cannot defer to join `handle`, because `defer()` + // requires the closure to outlive `'env` while `handle` doesn't. Related #33. + // + // // It is safe to defer to join a thread because deferred functions are executed before + // // unjoined threads are joined. + // scope.defer(move || handle.join().unwrap()); + }).unwrap(); + + // There should be sufficient synchronization. + assert_eq!(1, counter.load(Ordering::Relaxed)); + } } From fb0edaf90c389a8c15b0bbfa3c2a9a37696669e0 Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Sun, 29 Jul 2018 21:33:49 +0900 Subject: [PATCH 4/7] Rustfmt --- src/thread.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/thread.rs b/src/thread.rs index 2c8da37..721bb5c 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -152,9 +152,7 @@ where { let closure: Box + 'a> = Box::new(f); let closure: Box + Send> = mem::transmute(closure); - builder.spawn(move || { - closure.call_box() - }) + builder.spawn(move || closure.call_box()) } pub struct Scope<'env> { @@ -170,22 +168,22 @@ pub struct Scope<'env> { struct JoinState { join_handle: thread::JoinHandle<()>, - result: ScopedThreadResult + result: ScopedThreadResult, } impl JoinState { fn new(join_handle: thread::JoinHandle<()>, result: ScopedThreadResult) -> JoinState { JoinState { join_handle, - result + result, } } fn join(self) -> thread::Result { let result = self.result; - self.join_handle.join().map(|_| { - result.lock().unwrap().take().unwrap() - }) + self.join_handle + .join() + .map(|_| result.lock().unwrap().take().unwrap()) } } From f355b1f351ac76580e5a1a71f621a6b0e9bbeed5 Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Sun, 29 Jul 2018 21:43:50 +0900 Subject: [PATCH 5/7] Remove Scope::defer() --- src/thread.rs | 83 ++++++++++++++------------------------------------- 1 file changed, 22 insertions(+), 61 deletions(-) diff --git a/src/thread.rs b/src/thread.rs index 721bb5c..eeab2ff 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -113,7 +113,7 @@ use std::io; use std::marker::PhantomData; use std::mem; use std::ops::DerefMut; -use std::panic::{self, AssertUnwindSafe}; +use std::panic; use std::rc::Rc; use std::sync::{Arc, Mutex}; use std::thread; @@ -158,8 +158,6 @@ where pub struct Scope<'env> { /// The list of the thread join jobs. joins: RefCell> + 'env>>>, - /// The list of the deferred functions. - funcs: RefCell + 'env>>>, /// Thread and function panics invoked so far. panics: RefCell>>, // !Send + !Sync @@ -200,14 +198,13 @@ unsafe impl<'scope, T> Sync for ScopedJoinHandle<'scope, T> {} /// Create a new `Scope` for [*scoped thread spawning*](struct.Scope.html#method.spawn). /// -/// In addition, you can [register ad-hoc functions](struct.Scope.html#method.defer) that are -/// deferred to be run. No matter what happens, before the `Scope` is dropped, it is guaranteed that -/// all the unjoined spawned scoped threads are joined and the deferred functions are run. +/// No matter what happens, before the `Scope` is dropped, it is guaranteed that all the unjoined +/// spawned scoped threads are joined. /// -/// `thread::scope()` returns `Ok(())` if all the unjoined spawned threads and the deferred -/// functions did not panic. It returns `Err(e)` if one of them panics with `e`. If many of them -/// panics, it is still guaranteed that all the threads are joined and all the functions are run, -/// and `thread::scope()` returns `Err(e)` with `e` from a panicking thread or function. +/// `thread::scope()` returns `Ok(())` if all the unjoined spawned threads did not panic. It returns +/// `Err(e)` if one of them panics with `e`. If many of them panics, it is still guaranteed that all +/// the threads are joined and all the functions are run, and `thread::scope()` returns `Err(e)` +/// with `e` from a panicking thread or function. /// /// # Examples /// @@ -215,7 +212,6 @@ unsafe impl<'scope, T> Sync for ScopedJoinHandle<'scope, T> {} /// /// ``` /// crossbeam_utils::thread::scope(|scope| { -/// scope.defer(|| println!("Exiting scope")); /// scope.spawn(|| println!("Running child thread in scope")); /// }).unwrap(); /// // Prints messages @@ -226,15 +222,14 @@ where { let mut scope = Scope { joins: RefCell::new(Vec::new()), - funcs: RefCell::new(Vec::new()), panics: RefCell::new(Vec::new()), _marker: PhantomData, }; - // Executes the scoped function. - let ret = f(&scope); + // Executes the scoped function. Panic will be catched as `Err`. + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| f(&scope))); - // Executes deferred functions and joins threads. + // Joins all the threads. scope.drop_all(); let panic = scope.panics.borrow_mut().pop(); @@ -244,7 +239,7 @@ where } // Returns the result of the scoped function. - Ok(ret) + result } impl<'env> fmt::Debug for Scope<'env> { @@ -268,14 +263,6 @@ impl<'env> Scope<'env> { // FIXME(jeehoonkang): @rust-lang/rust#14875 is fixed, so maybe we can remove the above comment. // But I'd like to write tests to check it before removing the comment. fn drop_all(&mut self) { - let mut funcs = self.funcs.borrow_mut(); - for func in funcs.drain(..) { - let result = panic::catch_unwind(AssertUnwindSafe(|| func.call_box())); - if let Err(payload) = result { - self.panics.borrow_mut().push(payload); - } - } - let mut joins = self.joins.borrow_mut(); for join in joins.drain(..) { let result = join.call_box(); @@ -285,17 +272,6 @@ impl<'env> Scope<'env> { } } - /// Schedule code to be executed when exiting the scope. - /// - /// This is akin to having a destructor on the stack, except that it is *guaranteed* to be - /// run. It is guaranteed that the function is called before all the spawned threads are joined. - pub fn defer(&self, f: F) - where - F: FnOnce() + 'env, - { - self.funcs.borrow_mut().push(Box::new(f)); - } - /// Create a scoped thread. /// /// `spawn` is similar to the [`spawn`][spawn] function in Rust's standard library. The @@ -412,8 +388,8 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { impl<'env> Drop for Scope<'env> { fn drop(&mut self) { // Note that `self.dtors` can be non-empty when the code inside a `scope()` panics and - // `drop()` is called in unwinding. Even if it's the case, we will join the unjoined threads - // and execute the deferred functions. + // `drop()` is called in unwinding. Even if it's the case, we will join the unjoined + // threads. // // We ignore panics from any threads or functions because we're in course of unwinding // anyway. @@ -486,7 +462,7 @@ mod tests { } #[test] - fn counter_defer_panic() { + fn counter_panic() { let counter = AtomicUsize::new(0); let result = scope(|scope| { scope.spawn(|| { @@ -494,12 +470,6 @@ mod tests { }); thread::sleep(time::Duration::from_millis(100)); - for _ in 0..TIMES { - scope.defer(|| { - counter.fetch_add(1, Ordering::Relaxed); - }); - } - for _ in 0..TIMES { scope.spawn(|| { counter.fetch_add(1, Ordering::Relaxed); @@ -507,27 +477,18 @@ mod tests { } }); - assert_eq!(TIMES + TIMES, counter.load(Ordering::Relaxed)); + assert_eq!(TIMES, counter.load(Ordering::Relaxed)); assert!(result.is_err()); } #[test] - fn counter_defer_join() { - let counter = AtomicUsize::new(0); - scope(|scope| { - let _handle = scope.spawn(|| { - counter.store(1, Ordering::Relaxed); + fn panic_twice() { + let result = scope(|scope| { + scope.spawn(|| { + panic!(); }); - - // FIXME(jeehoonkang): currently we cannot defer to join `handle`, because `defer()` - // requires the closure to outlive `'env` while `handle` doesn't. Related #33. - // - // // It is safe to defer to join a thread because deferred functions are executed before - // // unjoined threads are joined. - // scope.defer(move || handle.join().unwrap()); - }).unwrap(); - - // There should be sufficient synchronization. - assert_eq!(1, counter.load(Ordering::Relaxed)); + panic!(); + }); + assert!(result.is_err()); } } From 0052bd1d9b3dc94b8b5f52ed3dd1d57343833b33 Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Sun, 29 Jul 2018 21:49:18 +0900 Subject: [PATCH 6/7] Update comments --- src/thread.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/thread.rs b/src/thread.rs index eeab2ff..045fea6 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -158,7 +158,7 @@ where pub struct Scope<'env> { /// The list of the thread join jobs. joins: RefCell> + 'env>>>, - /// Thread and function panics invoked so far. + /// Thread panics invoked so far. panics: RefCell>>, // !Send + !Sync _marker: PhantomData<*const ()>, @@ -196,15 +196,14 @@ pub struct ScopedJoinHandle<'scope, T: 'scope> { unsafe impl<'scope, T> Send for ScopedJoinHandle<'scope, T> {} unsafe impl<'scope, T> Sync for ScopedJoinHandle<'scope, T> {} -/// Create a new `Scope` for [*scoped thread spawning*](struct.Scope.html#method.spawn). +/// Creates a new `Scope` for [*scoped thread spawning*](struct.Scope.html#method.spawn). /// /// No matter what happens, before the `Scope` is dropped, it is guaranteed that all the unjoined /// spawned scoped threads are joined. /// /// `thread::scope()` returns `Ok(())` if all the unjoined spawned threads did not panic. It returns -/// `Err(e)` if one of them panics with `e`. If many of them panics, it is still guaranteed that all -/// the threads are joined and all the functions are run, and `thread::scope()` returns `Err(e)` -/// with `e` from a panicking thread or function. +/// `Err(e)` if one of them panics with `e`. If many of them panic, it is still guaranteed that all +/// the threads are joined, and `thread::scope()` returns `Err(e)` with `e` from a panicking thread. /// /// # Examples /// @@ -212,9 +211,9 @@ unsafe impl<'scope, T> Sync for ScopedJoinHandle<'scope, T> {} /// /// ``` /// crossbeam_utils::thread::scope(|scope| { +/// scope.spawn(|| println!("Exiting scope")); /// scope.spawn(|| println!("Running child thread in scope")); /// }).unwrap(); -/// // Prints messages /// ``` pub fn scope<'env, F, R>(f: F) -> thread::Result where @@ -233,7 +232,7 @@ where scope.drop_all(); let panic = scope.panics.borrow_mut().pop(); - // If any of the functions and threads panicked, returns the panic's payload. + // If any of the threads panicked, returns the panic's payload. if let Some(payload) = panic { return Err(payload); } @@ -391,8 +390,7 @@ impl<'env> Drop for Scope<'env> { // `drop()` is called in unwinding. Even if it's the case, we will join the unjoined // threads. // - // We ignore panics from any threads or functions because we're in course of unwinding - // anyway. + // We ignore panics from any threads because we're in course of unwinding anyway. self.drop_all(); } } From afd90a21638c21cc9b81aef6bfe074841db6d238 Mon Sep 17 00:00:00 2001 From: Jeehoon Kang Date: Sun, 29 Jul 2018 22:29:06 +0900 Subject: [PATCH 7/7] Revise (@stjepang's comments) --- src/thread.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/thread.rs b/src/thread.rs index 045fea6..6bc0fd8 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -229,7 +229,7 @@ where let result = panic::catch_unwind(panic::AssertUnwindSafe(|| f(&scope))); // Joins all the threads. - scope.drop_all(); + scope.join_all(); let panic = scope.panics.borrow_mut().pop(); // If any of the threads panicked, returns the panic's payload. @@ -255,13 +255,13 @@ impl<'scope, T> fmt::Debug for ScopedJoinHandle<'scope, T> { impl<'env> Scope<'env> { // This method is carefully written in a transactional style, so that it can be called directly - // and, if any dtor panics, can be resumed in the unwinding this causes. By initially running - // the method outside of any destructor, we avoid any leakage problems due to + // and, if any thread join panics, can be resumed in the unwinding this causes. By initially + // running the method outside of any destructor, we avoid any leakage problems due to // @rust-lang/rust#14875. // // FIXME(jeehoonkang): @rust-lang/rust#14875 is fixed, so maybe we can remove the above comment. // But I'd like to write tests to check it before removing the comment. - fn drop_all(&mut self) { + fn join_all(&mut self) { let mut joins = self.joins.borrow_mut(); for join in joins.drain(..) { let result = join.call_box(); @@ -386,12 +386,12 @@ impl<'scope, T> ScopedJoinHandle<'scope, T> { impl<'env> Drop for Scope<'env> { fn drop(&mut self) { - // Note that `self.dtors` can be non-empty when the code inside a `scope()` panics and + // Note that `self.joins` can be non-empty when the code inside a `scope()` panics and // `drop()` is called in unwinding. Even if it's the case, we will join the unjoined // threads. // // We ignore panics from any threads because we're in course of unwinding anyway. - self.drop_all(); + self.join_all(); } }