Open abonander opened 2 years ago
I want to note that the standard library argues that its mutex should be UnwindSafe because it has poisoning, and our mutexes doesn't.
The std Mutex
and RwLock
had poisoning long before UnwindSafe
existed, however. The very nature of the types allows their contained values to survive unwinding from other threads.
Given that no other lock API, including Tokio's, has bothered to implement poisoning, I assume the conclusion is that bugs due to broken invariants in the state of a type, left by unwinding, are rare enough that it's not worth the inconvenience.
If that's the case, then why should their handling with catch_unwind
be any different?
Maybe related, but when using gotham
I'm unable to use tokio::sync::Mutex
to wrap shared state because of this. It prevents me from having async
methods in the StateData
struct.
Additionally, an unlocked Mutex
is UnwindSafe
in all cases. It is only MutexGuard
and friends that should potentially be !UnwindSafe
.
Hi, is there any updates, especially about RwLock / Mutex's unwind safety, and Runtime's? Thanks!
Other than #6189 (that you filed), no.
@Darksonn Thank you all the same.
As for the Mutex/RwLock, do you suggest we just use AssertUnwindSafe and/or make a PR to make it UnwindSafe, or is forbidden to use it across panic at all?
P.S. My use case is like this: To develop https://github.com/fzyzcjy/flutter_rust_bridge, I need to have catch_unwind
because panic should never across FFI boundary. But I want to hold a Mutex
/RwLock
that can be used in multiple calls, thus it crosses the panic boundary.
Unwind safety is just a lint to act as a speedbump when catching panics. :woman_shrugging: If you are sure that your code is correct even if it panics, then go ahead and use AssertUnwindSafe.
@Darksonn Thanks for the information! However, I am not sure, because I am not the user but the package developer... I am only sure that the usage scenario is above.
Additionally, an unlocked
Mutex
isUnwindSafe
in all cases. It is onlyMutexGuard
and friends that should potentially be!UnwindSafe
.
Actually, this isn't true. You can still lock the mutex inside your closure and then panic while holding the lock. The MutexGuard wont be captured from the outer scope but it's still leaving your data in a potentially inconsistent state.
I would guess that UnwindSafe
is fine for Mutex, but RefUnwindSafe
not.
Tokio's API has a number of types that are
!UnwindSafe
and/or!RefUnwindSafe
even though a lot of them are meant to be used concurrently from multiple threads/tasks which inherently will require them to survive unwinding anyway.Searching the issue tracker and PR history, it appears that this is likely not intentional for most of these types, as patches to add explicit
UnwindSafe
impls appear to always be accepted:3689
4336
4384
4414
4418
Of course, it is possible to just use
AssertUnwindSafe
, as has been suggested before, but I don't think this is a tenable solution as it's easy to be overinclusive when it comes to futures (since you can just wrap a wholeasync {}
block with it) and unintentionally assert unwind safety on a type that is!UnwindSafe
on purpose, which could lead to logic errors, leaks or UB depending on the implementation of the API that's opting-out.However, I don't see any documentation in general on what types are intended to be
!UnwindSafe
and which simply haven't been addressed yet, so I'm opening this issue to enumerate the ones I know of and seek discussion on either fixing them or explicitly leaving them as they are. I don't have time to create an exhaustive list, but I'm hoping that this issue can serve as a nexus for collecting this knowledge to make it easier to find in the future.tokio::sync
moduleThese types are all meant to be used concurrently, so
!UnwindSafe
seems unintentional. I suspect it should be possible to fix most of them in one fell swoop by fixing the underlying primitives. The guard types should probably also take into account whether the contained type isRefUnwindSafe
.Barrier
MappedMutexGuard
Mutex
MutexGuard
Notify
OnceCell
OwnedMutexGuard
OwnedRwLockMappedWriteGuard
OwnedRwLockReadGuard
OwnedRwLockWriteGuard
OwnedSemaphorePermit
RwLock
RwLockMappedWriteGuard
RwLockReadGuard
RwLockWriteGuard
Semaphore
SemaphorePermit
Channels
All channel types appear to be
!UnwindSafe
as well, even ones likempsc::Sender
which can be used from multiple threads concurrently.