flume icon indicating copy to clipboard operation
flume copied to clipboard

catch_unwind and Flume channels

Open itamarst opened this issue 4 years ago • 3 comments

Hi,

I'm trying to catch panics in some code with Flume:

    pub fn add_allocation(&self, address: usize, size: usize) {
        // simplified from real code.
        catch_unwind(|| {
            self.sender.send(
                AddAllocationCommand {
                    address,
                    size,
                }.into(),
            ).unwrap_or(());
        });
    }

This gives the following error:

error[E0277]: the type `UnsafeCell<flume::Chan<TrackingCommandEnum>>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
     --> src/api.rs:359:9
      |
  359 |         catch_unwind(|| {
      |         ^^^^^^^^^^^^ `UnsafeCell<flume::Chan<TrackingCommandEnum>>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
      |
     ::: /home/itamarst/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:430:40
      |
  430 | pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
      |                                        ---------- required by this bound in `catch_unwind`
      |
      = help: within `&SendToStateThread`, the trait `RefUnwindSafe` is not implemented for `UnsafeCell<flume::Chan<TrackingCommandEnum>>`
      = note: required because it appears within the type `spin::mutex::spin::SpinMutex<flume::Chan<TrackingCommandEnum>>`
      = note: required because it appears within the type `spin::mutex::Mutex<flume::Chan<TrackingCommandEnum>>`
      = note: required because it appears within the type `flume::Shared<TrackingCommandEnum>`
      = note: required because it appears within the type `alloc::sync::ArcInner<flume::Shared<TrackingCommandEnum>>`
      = note: required because it appears within the type `PhantomData<alloc::sync::ArcInner<flume::Shared<TrackingCommandEnum>>>`
      = note: required because it appears within the type `Arc<flume::Shared<TrackingCommandEnum>>`
      = note: required because it appears within the type `flume::Sender<TrackingCommandEnum>`
      = note: required because it appears within the type `SendToStateThread`
      = note: required because it appears within the type `&SendToStateThread`
      = note: required because of the requirements on the impl of `UnwindSafe` for `&&SendToStateThread`
      = note: required because it appears within the type `[closure@src/api.rs:359:22: 368:10]`
  
  error: aborting due to previous error; 1 warning emitted
  1. Is this actually a problem? I can work around this AssertUnwindSafe... but that's a bad idea if it really isn't unwind safe.
  2. If it is unwind safe, probably the relevant code should have the UnwindSafe trait.
  3. If it is not unwind safe... any suggestions? I need to really really make sure no panics make it out, due to being called from FFI boundary (I don't want to abort the process).

itamarst avatar Sep 10 '21 17:09 itamarst

I guess this might be a spin issue? But since it seems like you've looked at their codebase you might have some sense.

itamarst avatar Sep 10 '21 20:09 itamarst

I can look into implementing the necessary unwinding traits, this should be fine to do.

The trait should probably be implemented in spin, however. Thankfully, I am the maintainer of that crate too so I can also look into that.

zesterer avatar Sep 11 '21 23:09 zesterer

Thank you!

itamarst avatar Sep 12 '21 11:09 itamarst