rust-lang / futures-rs

Zero-cost asynchronous programming in Rust
https://rust-lang.github.io/futures-rs/
Apache License 2.0
5.43k stars 628 forks source link

Fix use after free of task in FuturesUnordered when dropped future panics #2886

Closed Imberflur closed 1 month ago

Imberflur commented 1 month ago

Fixes https://github.com/rust-lang/futures-rs/issues/2863

taiki-e commented 1 month ago

Hmm. I see AddressSanitizer also complains about the same leak.

Given that it would complicate CI, and how Vec and VecDeque behave, trying to avoid the risk of potential abort for bad future here may not actually be a worthwhile goal...

Imberflur commented 1 month ago

I could just ignore this test there? The main thing it tests for is caught by miri.

It feels kind of bad to change this due to testing issues....

TBH if there was a test for hitting the abort case that would probably have issues too since iirc there isn't a good way to test should_abort :(