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

Implement the UnwindSafe/RefUnwindSafe traits on channels #2772

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions futures-channel/src/mpsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ use futures_core::stream::{FusedStream, Stream};
use futures_core::task::__internal::AtomicWaker;
use futures_core::task::{Context, Poll, Waker};
use std::fmt;
use std::panic::{RefUnwindSafe, UnwindSafe};
use std::pin::Pin;
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering::SeqCst;
Expand Down Expand Up @@ -147,6 +148,35 @@ pub struct UnboundedReceiver<T> {
// `Pin<&mut UnboundedReceiver<T>>` is never projected to `Pin<&mut T>`
impl<T> Unpin for UnboundedReceiver<T> {}

// The `UnwindSafe` trait is not easy to reason about. Rather than demonstrate
// why `UnwindSafe` is required on `T`, it is easier to give counter-examples
// where unwind safety would be broken without this requirement.
// For the `Receiver`, a counter-example is trivial: without the trait bound
// requirement, one could pass a `Receiver` through the `catch_unwind`
// boundary then send a non-`UnwindSafe` trait to it from the outside.
// For the `Sender`, a counter-example is:
// let (tx, rx) = channel::<Arc<RefCell<Foo>>>();
// catch_unwind(|| {
// let a = Arc::new(RefCell::new(...));
// tx.send(a.clone());
// a.borrow_mut().half_modification();
// panic!();
// })
// let a = rx.recv().unwrap(); // `a` is in a corrupted state.
impl<T: UnwindSafe> UnwindSafe for UnboundedReceiver<T> {}
impl<T: UnwindSafe> UnwindSafe for UnboundedSender<T> {}
impl<T: UnwindSafe> UnwindSafe for Receiver<T> {}
impl<T: UnwindSafe> UnwindSafe for Sender<T> {}
// There's nothing the API user can do with a `&Sender<T>` or `&Receiver<T>`.
// They act as a potential container for a `T` and are thus similar to, say,
// a `Box` for unwind-safety-related purposes. If it was possible to
// send/receive items through a `&Sender` or `&Receiver`, then this would
// require `T: UnwindSafe` instead.
impl<T: RefUnwindSafe> RefUnwindSafe for UnboundedReceiver<T> {}
impl<T: RefUnwindSafe> RefUnwindSafe for UnboundedSender<T> {}
impl<T: RefUnwindSafe> RefUnwindSafe for Receiver<T> {}
impl<T: RefUnwindSafe> RefUnwindSafe for Sender<T> {}

/// The error type for [`Sender`s](Sender) used as `Sink`s.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct SendError {
Expand Down
26 changes: 26 additions & 0 deletions futures-channel/src/oneshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use alloc::sync::Arc;
use core::fmt;
use core::panic::{RefUnwindSafe, UnwindSafe};
use core::pin::Pin;
use core::sync::atomic::AtomicBool;
use core::sync::atomic::Ordering::SeqCst;
Expand Down Expand Up @@ -31,6 +32,31 @@ pub struct Sender<T> {
impl<T> Unpin for Receiver<T> {}
impl<T> Unpin for Sender<T> {}

// The `UnwindSafe` trait is not easy to reason about. Rather than demonstrate
// why `UnwindSafe` is required on `T`, it is easier to give counter-examples
// where unwind safety would be broken without this requirement.
// For the `Receiver`, a counter-example is trivial: without the trait bound
// requirement, one could pass a `Receiver` through the `catch_unwind`
// boundary then send a non-`UnwindSafe` trait to it from the outside.
// For the `Sender`, a counter-example is:
// let (tx, rx) = channel::<Arc<RefCell<Foo>>>();
// catch_unwind(|| {
// let a = Arc::new(RefCell::new(...));
// tx.send(a.clone());
// a.borrow_mut().half_modification();
// panic!();
// })
// let a = rx.recv().unwrap(); // `a` is in a corrupted state.
impl<T: UnwindSafe> UnwindSafe for Receiver<T> {}
impl<T: UnwindSafe> UnwindSafe for Sender<T> {}
// There's nothing the API user can do with a `&Sender<T>` or `&Receiver<T>`.
// They act as a potential container for a `T` and are thus similar to, say,
// a `Box` for unwind-safety-related purposes. If it was possible to
// send/receive items through a `&Sender` or `&Receiver`, then this would
// require `T: UnwindSafe` instead.
impl<T: RefUnwindSafe> RefUnwindSafe for Receiver<T> {}
impl<T: RefUnwindSafe> RefUnwindSafe for Sender<T> {}

/// Internal state of the `Receiver`/`Sender` pair above. This is all used as
/// the internal synchronization between the two for send/recv operations.
struct Inner<T> {
Expand Down