Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

catch_unwind and Flume channels #90

Open
itamarst opened this issue Sep 10, 2021 · 3 comments
Open

catch_unwind and Flume channels #90

itamarst opened this issue Sep 10, 2021 · 3 comments

Comments

@itamarst
Copy link

itamarst commented Sep 10, 2021

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
Copy link
Author

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

@zesterer
Copy link
Owner

zesterer commented Sep 11, 2021

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.

@itamarst
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants