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

Reactor is not RefUnwindSafe #285

Open
Marwes opened this issue Dec 9, 2017 · 5 comments
Open

Reactor is not RefUnwindSafe #285

Marwes opened this issue Dec 9, 2017 · 5 comments

Comments

@Marwes
Copy link

Marwes commented Dec 9, 2017

Specifically this is because of the use of a UnsafeCell inside a long list of types so I am not sure which part exactly needs to implement this (guessing it is the Queue that would assert that it is safe?).

error[E0277]: the trait bound `std::cell::UnsafeCell<*mut futures::sync::mpsc::queue::Node<std::option::Option<tokio_core::reactor::Message>>>: std::panic::RefUnwindSafe` is not satisfied in `gluon::Thread`
  --> tests\main.rs:91:9
   |
91 |         tensile::test(name.clone(), move || -> Result<(), String> {
   |         ^^^^^^^^^^^^^ the type std::cell::UnsafeCell<*mut futures::sync::mpsc::queue::Node<std::option::Option<tokio_core::reactor::Message>>> may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
   |
   = help: within `gluon::Thread`, the trait `std::panic::RefUnwindSafe` is not implemented for `std::cell::UnsafeCell<*mut futures::sync::mpsc::queue::Node<std::option::Option<tokio_core::reactor::Message>>>`
   = note: required because it appears within the type `futures::sync::mpsc::queue::Queue<std::option::Option<tokio_core::reactor::Message>>`
   = note: required because it appears within the type `futures::sync::mpsc::Inner<tokio_core::reactor::Message>`
   = note: required because it appears within the type `alloc::arc::ArcInner<futures::sync::mpsc::Inner<tokio_core::reactor::Message>>`
   = note: required because it appears within the type `std::marker::PhantomData<alloc::arc::ArcInner<futures::sync::mpsc::Inner<tokio_core::reactor::Message>>>`
   = note: required because it appears within the type `std::ptr::Shared<alloc::arc::ArcInner<futures::sync::mpsc::Inner<tokio_core::reactor::Message>>>`
   = note: required because it appears within the type `std::sync::Arc<futures::sync::mpsc::Inner<tokio_core::reactor::Message>>`
   = note: required because it appears within the type `futures::sync::mpsc::Sender<tokio_core::reactor::Message>`
   = note: required because it appears within the type `futures::sync::mpsc::UnboundedSender<tokio_core::reactor::Message>`
   = note: required because it appears within the type `tokio_core::reactor::Remote`
   = note: required because it appears within the type `std::option::Option<tokio_core::reactor::Remote>`
   = note: required because it appears within the type `gluon::<unnamed>::vm::GlobalVmState`
   = note: required because it appears within the type `alloc::arc::ArcInner<gluon::<unnamed>::vm::GlobalVmState>`
   = note: required because it appears within the type `std::marker::PhantomData<alloc::arc::ArcInner<gluon::<unnamed>::vm::GlobalVmState>>`
   = note: required because it appears within the type `std::ptr::Shared<alloc::arc::ArcInner<gluon::<unnamed>::vm::GlobalVmState>>`
   = note: required because it appears within the type `std::sync::Arc<gluon::<unnamed>::vm::GlobalVmState>`
   = note: required because it appears within the type `gluon::Thread`
   = note: required because of the requirements on the impl of `std::panic::UnwindSafe` for `*const gluon::Thread`
   = note: required because it appears within the type `gluon::<unnamed>::gc::GcPtr<gluon::Thread>`
   = note: required because it appears within the type `gluon::RootedThread`
   = note: required because it appears within the type `[closure@tests\main.rs:91:37: 93:10 vm:gluon::RootedThread, name:std::string::String, filename:std::path::PathBuf]`
   = note: required because of the requirements on the impl of `tensile::Testable` for `[closure@tests\main.rs:91:37: 93:10 vm:gluon::RootedThread, name:std::string::String, filename:std::path::PathBuf]`
   = note: required by `tensile::test`

@alexcrichton
Copy link
Contributor

Thanks for the report! I think though that since aribtrary trait objects are contained, I'm not sure this can be unwind safe? Could you also elaborate a bit on why you'd like the reactor to be unwind safe?

@Marwes
Copy link
Author

Marwes commented Dec 10, 2017

Chain of consequences here are.

Need to dispatch futures from an interpreter ->
I need to hold a Reactor in the interpreter -
The test library I use for some tests catches panics and I pass interpreters across this boundary ->
Need to make the gluon interpreter RefUnwindSafe (which Remote is the only part that is not)

Nothing I can't work around locally but if it can be fixed upstream then that is obviously preferably (since I think the types here should all be RefUnwindSafe, they just don't have the impl).

@alexcrichton
Copy link
Contributor

Ok thanks for the explanation! My point here though is that the reactor isn't unwind safe as it contains arbitrary trait objects, so I don't actually think we can fix this at the Core layer. That being said I was hoping to be able to help out with the chain of things that led to this, but I'm not so sure now :(

@Marwes
Copy link
Author

Marwes commented Dec 10, 2017

Ah, didn't get that Remote holds arbitrary trait objects. I suppose that if it were to only hold arbitrary trait objects but not execute any code in them then it could be made RefUnwindSafe, (As it is right now I think it executes arbitrary objects as well).

@alexcrichton
Copy link
Contributor

Oh sorry I thought this was Core not Remote! In that sense I think this should definitely be RefUnwindSafe most likely (although this may best be tested against tokio-rs/tokio perhaps

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