Skip to content

Commit 67f8869

Browse files
committed
Leak internal queue in FuturesUnordered drop if dropping a future panics and avoid potential abort if we try to continue dropping futures and another one panics
1 parent 8b678f4 commit 67f8869

File tree

2 files changed

+22
-25
lines changed
  • .github/workflows
  • futures-util/src/stream/futures_unordered

2 files changed

+22
-25
lines changed

.github/workflows/ci.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,11 +246,17 @@ jobs:
246246
- uses: taiki-e/checkout-action@v1
247247
- name: Install Rust
248248
run: rustup toolchain install nightly --component miri && rustup default nightly
249-
- run: cargo miri test --workspace --all-features
249+
- run: cargo miri test --workspace --all-features -- --skip panic_on_drop_fut
250250
env:
251251
MIRIFLAGS: -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation
252252
RUSTDOCFLAGS: ${{ env.RUSTDOCFLAGS }} -Z randomize-layout
253253
RUSTFLAGS: ${{ env.RUSTFLAGS }} -Z randomize-layout
254+
# This test is expected to leak.
255+
- run: cargo miri test --workspace --all-features --test stream_futures_unordered -- panic_on_drop_fut
256+
env:
257+
MIRIFLAGS: -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation -Zmiri-ignore-leaks
258+
RUSTDOCFLAGS: ${{ env.RUSTDOCFLAGS }} -Z randomize-layout
259+
RUSTFLAGS: ${{ env.RUSTFLAGS }} -Z randomize-layout
254260

255261
san:
256262
name: cargo test -Z sanitizer=${{ matrix.sanitizer }}

futures-util/src/stream/futures_unordered/mod.rs

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -576,33 +576,24 @@ impl<Fut> Drop for FuturesUnordered<Fut> {
576576
// Before the strong reference to the queue is dropped we need all
577577
// futures to be dropped. See note at the bottom of this method.
578578
//
579-
// This ensures we drop all futures even in the case of one drop
580-
// panicking and unwinding.
581-
//
582-
// If a second future panics, that will trigger an abort from panicking
583-
// while panicking.
584-
struct DropGuard<'a, Fut>(&'a mut FuturesUnordered<Fut>);
585-
impl<Fut> DropGuard<'_, Fut> {
586-
fn drop_futures(&mut self) {
587-
// When a `FuturesUnordered` is dropped we want to drop all futures
588-
// associated with it. At the same time though there may be tons of
589-
// wakers flying around which contain `Task<Fut>` references
590-
// inside them. We'll let those naturally get deallocated.
591-
while !self.0.head_all.get_mut().is_null() {
592-
let head = *self.0.head_all.get_mut();
593-
let task = unsafe { self.0.unlink(head) };
594-
self.0.release_task(task);
595-
}
596-
}
597-
}
598-
impl<Fut> Drop for DropGuard<'_, Fut> {
579+
// If there is a panic before this completes, we leak the queue.
580+
struct LeakQueueOnDrop<'a, Fut>(&'a mut FuturesUnordered<Fut>);
581+
impl<Fut> Drop for LeakQueueOnDrop<'_, Fut> {
599582
fn drop(&mut self) {
600-
self.drop_futures();
583+
mem::forget(Arc::clone(&self.0.ready_to_run_queue));
601584
}
602585
}
603-
let mut guard = DropGuard(self);
604-
guard.drop_futures();
605-
mem::forget(guard); // no need to check head_all again
586+
let guard = LeakQueueOnDrop(self);
587+
// When a `FuturesUnordered` is dropped we want to drop all futures
588+
// associated with it. At the same time though there may be tons of
589+
// wakers flying around which contain `Task<Fut>` references
590+
// inside them. We'll let those naturally get deallocated.
591+
while !guard.0.head_all.get_mut().is_null() {
592+
let head = *guard.0.head_all.get_mut();
593+
let task = unsafe { guard.0.unlink(head) };
594+
guard.0.release_task(task);
595+
}
596+
mem::forget(guard); // safe to release strong reference to queue
606597

607598
// Note that at this point we could still have a bunch of tasks in the
608599
// ready to run queue. None of those tasks, however, have futures

0 commit comments

Comments
 (0)