Memory leak in crossbeam_utils::thread::scope(): Unbounded memory usage with long-lived scopes

[BGR360]

crossbeam_utils::thread::Scope owns a vector of shared join handles:

crossbeam/crossbeam-utils/src/thread.rs

Lines 193 to 203 in 450d237

	/// A scope for spawning threads.
	pub struct Scope<'env> {
	/// The list of the thread join handles.
	handles: SharedVec<SharedOption<thread::JoinHandle<()>>>,
	/// Used to wait until all subscopes all dropped.
	wait_group: WaitGroup,
	/// Borrows data with invariant lifetime `'env`.
	_marker: PhantomData<&'env mut &'env ()>,
	}

Each call to scope.spawn() pushes a new handle to the vec:

crossbeam/crossbeam-utils/src/thread.rs

Lines 458 to 459 in 450d237

	// Add the handle to the shared list of join handles.
	self.scope.handles.lock().unwrap().push(Arc::clone(&handle));

The vec never shrinks until it is drained all at once at the end of scope():

crossbeam/crossbeam-utils/src/thread.rs

Lines 167 to 176 in 450d237

	// Join all remaining spawned threads.
	let panics: Vec<_> = scope
	.handles
	.lock()
	.unwrap()
	// Filter handles that haven't been joined, join them, and collect errors.
	.drain(..)
	.filter_map(|handle| handle.lock().unwrap().take())
	.filter_map(|handle| handle.join().err())
	.collect();

Thus, any long-lived thread scope will exhibit unbounded memory usage as it spawns more and more threads, even if those threads finish executing or are joined by the user. This is a huge problem for servers or other such long-running applications that want to spawn scoped threads.

I would understand if there are reasons that scope() can't be implemented in such a way where these handles are cleaned up as threads complete. But if that is the case, then I feel this needs to be a very explicitly warned about in the crate's documentation and README. Cuz this is really bad. We believe that this issue is the root cause of a recent out-of-memory crash on an enterprise customer's deployment of our software product.

[nickkuk]

Partially related: as I know this functionality is slowly moving to std. As far as I can see their implementation doesn't have such a problem.

[m-ou-se]

Partially related: as I know this functionality is rust-lang/rust#93203. As far as I can see their implementation doesn't have such a problem.

Indeed! The not-yet-stable std::thread::scope's memory usage stays constant, independent of how many threads it spawned over its lifetime.

[m-ou-se]

Note that since crossbeam's scope function returns an Err containing a Vec of all the panic payloads of all the threads that panicked, this is partially a problem of the API itself. Even if it didn't keep join handles for already-finished threads, it'd still have to keep all the panic payloads around to be able to return them, resulting in unbounded memory usage when threads keep panicking.

[taiki-e]

If someone can implement an equivalent to the standard library's scoped thread API without relying on unstable features, I would like to change the crossbeam's scoped thread API in the next breaking release.

In any case, once the standard library's scoped thread is stable, I plan to soft deprecate the crossbeam's scoped thread.

[taiki-e]

Filed #844 to fix this.