Tracking Issue for context_ext

[jkarneges]

Feature gate: #![feature(context_ext)]

This is a tracking issue for allowing std::task::Context to carry arbitrary extension data.

Public API

impl Context {
    fn ext(&mut self) -> &mut dyn Any;
}

impl ContextBuilder {
    fn ext(self, data: &'a mut dyn Any) -> Self;

    fn from(cx: &'a mut Context<'_>) -> Self;
    fn waker(self, waker: &'a Waker) -> Self;
}

Steps / History

• Implementation: Add Context::ext #123203
• Final comment period (FCP)¹
• Stabilization PR

Unresolved Questions

• passing refs maybe unsound
• too complex
• This causes Context to no longer be UnwindSafe, which may be a breaking change: regression: the type (dyn Any + 'static) may contain interior mutability #125193 (the issue is closed but there still is a FIXME in the code)

Footnotes

1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩

[jkarneges]

One of the concerns raised was about the soundness of trying to force the dyn Any to carry non-static references via unsafe casts.

I believe this may be theoretically achievable by requiring any references contained within the dyn Any to be of dyn types, which are !Sized and thus prevent move-ins and move-outs of themselves, and for the trait definitions of those types to not expose any held lifetimes. Constraining inner references to such types could be done by introducing an unsafe trait:

// SAFETY: only implement on dyn types that don't expose
// lifetimes held by the type. i.e. traits with methods
// that only work with argument lifetimes. this allows
// us to safely cast any held lifetimes, because there
// is no way to move-in, move-out, or obtain a reference
// to data with any such held lifetimes
pub unsafe trait DynWithStaticInterface {
    type Static: ?Sized + 'static;
    type Clamped<'a>: ?Sized
    where
        Self: 'a;

    // return type is safe to use because it does not expose
    // whatever lifetimes were converted to 'static
    fn to_static(&mut self) -> &mut Self::Static;

    // return type is safe to use because it does not expose
    // whatever lifetimes were converted to 'a
    fn to_clamped<'a>(&'a mut self) -> &'a mut Self::Clamped<'a>;
}

Since we are working with dyn references, we need the trait to be object-safe. This means we cannot allow casting to arbitrary lifetimes. Fortunately, we only ever need to cast to static or to the lifetime of self (which we here refer to as a "clamped" lifetime), and so we can provide methods for exactly those.

Implementing on a dyn type could look like this:

use std::mem;

trait StringRef {
    fn get(&mut self) -> &mut String;
}

// SAFETY: implementing on a dyn type that does not expose any lifetimes held by the type
unsafe impl DynWithStaticInterface for (dyn StringRef + '_) {
    type Static = dyn StringRef + 'static;
    type Clamped<'a> = dyn StringRef + 'a;

    fn to_static(&mut self) -> &mut Self::Static {
        unsafe { mem::transmute(self) }
    }

    fn to_clamped<'a>(&'a mut self) -> &'a mut Self::Clamped<'a> {
        unsafe { mem::transmute(self) }
    }
}

struct StringSource<'a> {
    s: &'a mut String,
}

impl<'a> StringSource<'a> {
    fn new(s: &'a mut String) -> Self {
        Self { s }
    }
}

impl<'a> StringRef for StringSource<'a> {
    fn get(&mut self) -> &mut String {
        self.s
    }
}

Above we have a non-static concrete type holding data with lifetime 'a that implements a trait which does not expose the 'a. If all a user has is a &mut dyn StringRef, then there is no way to move a dyn StringRef into that address or move the dyn StringRef out of that address, and the available methods on the type (get()) only expose data with the lifetime of self. The lifetime of the concrete type is not exposed, and so it is safe to lie about it. Whether the lifetime is casted to 'static or some 'foo, the user can't do anything with it.

The Any::downcast_mut() method is limited to casting to references of types that are Sized, so we can't cast a dyn reference directly to &mut dyn Any. However, we can cast a dyn reference to a pointer, and then cast a reference to the pointer to the &mut dyn Any. To make this safe, we can wrap the pointer:

#![feature(local_waker)]
#![feature(context_ext)]

struct ContainerPrivate<T: ?Sized>(*mut T);

pub struct Container<'a, T: ?Sized, U: ?Sized> {
    p: ContainerPrivate<T>,
    _marker: PhantomData<&'a mut U>,
}

impl<'a, T: DynWithStaticInterface + ?Sized> Container<'a, T::Static, T> {
    pub fn new(r: &'a mut T) -> Self {
        Self {
            p: ContainerPrivate(r.to_static()),
            _marker: PhantomData,
        }
    }
}

impl<T: ?Sized + 'static, U: ?Sized> Container<'_, T, U> {
    pub fn as_any<'a>(&'a mut self) -> &'a mut (dyn Any + 'static) {
        &mut self.p
    }
}

pub fn ctx_downcast<'a, T: DynWithStaticInterface + ?Sized + 'static>(
    cx: &mut Context<'a>,
) -> Option<&'a mut T::Clamped<'a>> {
    let a = cx.ext();

    let p = match a.downcast_mut::<ContainerPrivate<T>>() {
        Some(p) => p,
        None => return None,
    };

    // SAFETY: the inner pointer is guaranteed to be valid because
    // p has the same lifetime as the reference that was
    // originally casted to the pointer
    let r: &'a mut T = unsafe { p.0.as_mut().unwrap() };

    Some(r.to_clamped())
}

With the above API, it becomes possible for the Context extension to carry non-static data:

#![feature(noop_waker)]

use std::task::{ContextBuilder, Waker};

let mut data = String::from("hello");

{
    let mut s = StringSource::new(&mut data);
    let mut c = Container::new(&mut s as &mut dyn StringRef);
    let mut cx = ContextBuilder::from_waker(&Waker::noop())
        .ext(c.as_any())
        .build();

    let r = ctx_downcast::<dyn StringRef>(&mut cx).unwrap();
    r.get().push_str(" world");
}

assert_eq!(&data, "hello world");

This is a bit more complicated than I would have liked, but I can't think of a better way to do it, and I can't think of a better data structure than Any to allow carrying arbitrary data.

[jkarneges]

I should note that passing 'static data, whether single owner or with something like Rc, is of course possible and a lot simpler:

Single owner:

let mut data = String::from("hello");

{
    let mut cx = ContextBuilder::from_waker(&Waker::noop())
        .ext(&mut data)
        .build();

    let data = cx.ext().downcast_mut::<String>().unwrap();
    data.push_str(" world");
}

assert_eq!(&data, "hello world");

With Rc:

let mut data = Rc::new(RefCell::new(String::from("hello")));

{
    let mut cx = ContextBuilder::from_waker(&Waker::noop())
        .ext(&mut data)
        .build();

    let data = cx.ext().downcast_ref::<Rc<RefCell<String>>>().unwrap();
    data.borrow_mut().push_str(" world");
}

assert_eq!(&*data.borrow(), "hello world");

I would guess many executors could reasonably use something like Rc/Arc for their extension data without runtime cost by pre-allocating such structures. In my experience, executors typically keep most of their state on the heap behind smart pointers anyway.

Passing non-static references in extension data may be more interesting for combinators or nested executors, i.e. things that may want to avoid allocations and can't easily pre-allocate.

[jkarneges]

I would like to move this towards stabilization.

The goal of this feature is to enable experimentation of Context extensions, that could later be promoted to direct fields/methods of Context. Extensions that could not actually be promoted to direct fields, for example those requiring additional generics, are out of scope.

Using this feature, I've successfully implemented my own extension, TopLevelPoller in the waker-waiter crate, accessible by reference, which conceivably could be promoted to a &'a mut dyn TopLevelPoller field of Context.

To respond to the unresolved questions:

passing refs may be unsound

I believe the approach I described earlier is sound. Would love feedback.

too complex

Specifically, @joshtriplett earlier wrote:

This is a lot of complexity to add to this interface.

It's one field. Is this saying we should not allow Context to communicate more kinds of data?

dyn Any is always painful.

After trying to shoehorn non-static references into it, I can agree with this statement. It kind of is what it is though.

Sharing one pointer between multiple potential users is even more painful, and a Provider-style API would be even more complexity.

True and of course not what is being proposed.

Everyone would pay the cost of this, whether they use it or not. And to the best of my knowledge, this isn't something needed by all async runtimes.

It's an optional feature with no cost if not used, other than increasing the size of Context by the size of a fat pointer. Is this saying we should not add a field to Context unless it is needed by all async runtimes?

I see a future where there are many extensions that executors and futures can opt-in to, that are best negotiated via the Context, and not every app would use all possible extensions. I think this is a reasonable stance? Maybe we need some community consensus on the notion that it would be valuable for Context to be able to carry optional data.

The potential users of this (AIUI) currently do have alternative solutions. Those solutions may not be ideal, but we have to weigh that against the complexity (which is a cost to everyone).

Unless there's more to it, I assume this a weighing between using thread-local/global storage vs slightly increasing the size of Context. I suspect most people developing async apps would prefer the latter.

This causes Context to no longer be UnwindSafe, which may be a breaking change

Does anyone know how meaningful this is?

[traviscross]

@rustbot labels +AsyncAwait-Triaged

We discussed this in the WG-async meeting today. We have enough reservations about this approach that we'd like to explore two main other approaches to solving this problem first.

One alternative is the Provider API, as in:

• Tracking Issue for error_generic_member_access #99301

The idea there would be to broaden the scope of that producer/consumer API from just Error, as it is now. We're interested in exploring how this could be done and unblocked.

The other approach is making Future generic, as in:

• Generic Futures rfcs#3434

For this one, in particular, we'd need to do a lot of feasibility and migration analysis.

In general, we're interested in looking into and comparing these approaches.

We find the problem that the context extension is trying to solve highly motivating, but we want to be sure to find the right solution.