futures-rs icon indicating copy to clipboard operation
futures-rs copied to clipboard

Should types like `AtomicWaker` and `AbortHandle` be declared `UnwindSafe`?

Open najamelan opened this issue 5 years ago • 3 comments

I run into issues with a type I would like to be UnwindSafe. It contains an AbortHandle which contains a AtomicWaker and these types aren't unwindsafe because AtomicWaker contains an UnsafeCell.

In the current state of affairs it is up to a user in such situation to go figure out from the futures source code whether these are UnwindSafe but it was forgotten to mark them as such, or whether they are not UnwindSafe but that was not mentioned in the docs.

From a quick look over the code for Abortable I can't immediately see how it could be in an invalid state because of a panic in an inopportune moment, but guaranteeing that with certainty for code one hasn't written is quite some work.

So should these types be marked UnwindSafe and has anyone enough understanding of their internals to quickly verify that? I presume there are other types in futures that are concerned as well (eg. channel Senders/Receivers).

najamelan avatar Sep 09 '20 17:09 najamelan

Seems AtomicWaker is not actually UnwindSafe. e.g., If the following clone panicked, AtomicWaker will probably not work well.

https://github.com/rust-lang/futures-rs/blob/c77cd3c9ba19b65db3d1931d2087032752fe9c7d/futures-core/src/task/__internal/atomic_waker.rs#L267

see also https://github.com/tokio-rs/tokio/pull/3689

taiki-e avatar Apr 18 '21 07:04 taiki-e

I suppose that depends on whether the clone can panic?

https://doc.rust-lang.org/src/core/task/wake.rs.html#190-192

It's all unsafe, and I have never looked into this code, so I have no idea.

najamelan avatar Apr 19 '21 06:04 najamelan

clone can cause panic because you can pass a function that can cause panic.

taiki-e avatar Apr 23 '21 12:04 taiki-e