Ephemeral (or "weak") closures

I couldn't rest and had to think about this again :weary:

This led me to consider trying something more easy:

  • store all closures directly in the scope handle
  • move a (wide) pointer to the extended closure to allow calling the original closure from there (while keeping the dropping managed entirely by the scope handle)
  • Use an Arc<RwLock<bool>> to syncronize execution of closure and dropping of scope

Here is my code (Playground) including comments and my original tests.

With my new approach, the string will be printed before it's dropped: (Playground)

Output:

Hello World
DROPPED


Amendment: It's possible to be generic over a (single) argument and return value by using an extra indirection. See modified example here (Playground), where the closures can retrieve an argument and return a value (requires double-boxing in line 57 though).

(Assuming the whole thing is safe now… :sweat_smile:)

1 Like

Now, I've tried to abstract the concept of something like an Arc<Foo<'a>> where you can hold a Weak reference into it without needing to mention the lifetime; and with a scope that makes sure that the the contents are dropped before the scope is left (so that only currently unused weak pointers remain). For now, I came up with something like

////////////////////////////////////////////////////////////////////////////////
// This section from https://docs.rs/existential/0.1.1/src/existential/lib.rs.html#106-113

pub trait TyConFor<'a> {
    type Applied;
}

pub type Apply<'a, C> = <C as TyConFor<'a>>::Applied;

pub trait TyCon: for<'a> TyConFor<'a> {}
impl<C: ?Sized> TyCon for C where C: for<'a> TyConFor<'a> {}

////////////////////////////////////////////////////////////////////////////////

pub mod scoped_existential {
    use super::*;

    use std::mem;
    use std::sync::{Arc, RwLock, TryLockError, Weak};

    pub fn scope<'scope, F, R>(f: F) -> R
    where
        F: FnOnce(&mut Scope<'scope>) -> R,
    {
        f(&mut Scope {
            data: <_>::default(),
            lock: <_>::default(),
        })
    }

    pub struct Scope<'scope> {
        data: Vec<Arc<dyn Send + Sync + 'scope>>,
        lock: Arc<RwLock<()>>,
    }

    impl<C: TyCon> Clone for ScopedExistential<C> {
        fn clone(&self) -> Self {
            Self {
                payload: self.payload.clone(),
                lock: self.lock.clone(),
            }
        }
    }

    // TODO: Send/Sync implementation is slightly inaccurate (due to 'static)
    pub struct ScopedExistential<C: TyCon> {
        payload: Weak<Apply<'static, C>>,
        lock: Arc<RwLock<()>>,
    }

    impl<C: TyCon> ScopedExistential<C> {
        pub fn with_upgrade<F, R>(&self, f: F) -> R
        where
            F: FnOnce(Option<&Apply<'_, C>>) -> R,
        {
            match self.lock.try_read() {
                Err(TryLockError::Poisoned(_)) => unreachable!(),
                Err(TryLockError::WouldBlock) => f(None),
                Ok(guard) => match self.payload.upgrade() {
                    None => {
                        drop(guard);
                        f(None)
                    }
                    Some(payload) => {
                        let r = f(Some(&payload));
                        drop(payload);
                        drop(guard);
                        r
                    }
                },
            }
        }
    }

    impl<'scope> Scope<'scope> {
        /// erases lifetime "'a" from a type Arc<Foo<'a>>
        pub fn erase<'a: 'scope, C: TyCon>(&mut self, val: Apply<'a, C>) -> ScopedExistential<C>
        where
            C: 'scope,
            Apply<'a, C>: Send + Sync,
        {
            let val = Arc::new(val);
            let weak = Arc::downgrade(&val);
            self.data.push(val);
            let payload =
                unsafe { mem::transmute::<Weak<Apply<'a, C>>, Weak<Apply<'static, C>>>(weak) };
            ScopedExistential {
                payload,
                lock: self.lock.clone(),
            }
        }
    }

    impl<'scope> Drop for Scope<'scope> {
        fn drop(&mut self) {
            let guard = self.lock.write().unwrap();
            self.data.clear();
            drop(guard);
        }
    }
}

and this seems to allow re-implementing your API without further unsafe code: Rust Playground

2 Likes

Very nice!! :smiley:

I don't understand most your code yet (particularly difficult for me with the code from the existential crate, but I'll try to understand in time). The code passes my tests and your fn main, really nice!

If it works, it would also allow me to use it for a concrete interface for my Lua machine (where I might not even need Send or being generic about the closures argument(s) and return value). And unsafe code would be in a seperate module or crate (hopefully published by someone else, so I'm not to blame :innocent:).

(Btw, I just learned you can have tests and a main function on Rust Playground.)

Without having fully understood your code yet, I noticed that you omitted the invariant PhantomData for 'scope. Is that intended? I remember I had some problems when I left it out, but to be honest, I get confused over this. I included it due to having problems and @Yandros' hint in this post. Unfortunately I don't remember the problem I ran into (Edit: if there was one at all).


See also crossbeam::Scope, where a PhantomData with invariance is used. I still try to remember if I had any problem when leaving it out and if there was one at all, but can't reproduce any error if I take it out from my code.

Semi-intended. I initially thought about completely preventing swaps and other shenanigans by only providing a Pin<&mut Self> for the Scop<'scope>, but then I came to the conclusion that not disallowing those might actually not be unsound. The variance was irrelevant with the Pin-involving design; I forgot to think about it again.. then later (actually after already posting my previous answer) considering the question again, I notived the difference in variance compared to your Scope, but couldn't find any way in which the lifetime being covariant is unsound.

Edit: Not Now that I'm thinking about it again, there probably is a soundness issue after all, if it's not invariant... I can try to give more details later.

Not sure if it makes a difference, but I just noticed that I changed the reference to the scope handle passed to the FnOnce closure (run in my case, f in your case, I think) from being immutable to being mutable. In @Yandros post as well as in case of crossbeam, an immutable reference is used.


Rethinking about this (it's really tough for me to understand it), if Scope<'scope> is covariant in 'scope, then (i think) 'scope is allowed to be shorter than the runtime of run (or f in your case). Thus, the bound F: 'scope + … (or C: 'scope in your erase method) is too lax. But not sure how to exploit it. But it does feel like there is a PhantomData with invariance needed to make it sound.

@jbe, so the problem when it's an immutable reference is simply that your safety condition breaks down since 'scope can become arbitrarily small, even though it's used as a lower bound on how long the closures must be able to live.

With a mutable reference, that's not all that easy anymore because mutable references themselves are immutable in the target type. And turning the mutable reference into an immutable one wouldn't help because you can't do anything with a &Scope<'scope> when all of Scope's methods require &mut self.

However, with API such as replace_with::replace_with_or_abort, you can take the Scope<'large_scope> out of a &mut Scope<'large_scope>, then convert that into a Scope<'small_scope>, and use it with mem::replace to avoid a nested/inner scope from being dropped. You won't be able to actually finish the outser (large_scope) scope anymore because you don't have a Scope<'large_scope> left anymore; but before the inevitable abort, you can still cause unsoundness by accessing stuff related to the inner scoped that wasn't dropped properly.

Now, even when it's invariant, you can still swap / replace stuff like this in weird ways, but only between scopes with identical lifetime argument; and I believe that whatever you do, there's still the invariant that: every Scope<'a> will be dropped before the lifetime 'a becomes invalid.

Just realized, you don't even need the second/inner scope(...) call. Just:

use std::mem;
fn main() {
    scope(|s| {
        replace_with::replace_with_or_abort(s, |s| {
            let string = "Hello World".to_owned();
            let string_ref = &string;
            let mut s = s; // variance!
            let mut f = s.extend_mut(|()| (), |()| {
                println!("{}", string);
            });
            mem::forget(s);
            drop(string);
            f(()); // accesses the string after free
            panic!(); // results in abort
        })
    });
}

(gist (not a playground because of replace_with dependency))

So to say it in very simple terms (give me time to understand all the details later :hot_face:), it's always wrong to omit the invariant PhantomData. In case of methods with an immutable reference to the scope handle it's just easier to construct / run into an example that is not sound?

(That might explain why I wasn't capable of reproducing the problems I experienced when removing the PhantomData again.)

P.S.: Or does that mean that replace_with::replace_with_or_abort would need to be marked unsafe?

P.P.S.: Or would the scope need to be marked !UnwindSafe somehow?

As I tried to explain above, it's not wrong if you're only providing Pin<&mut Scope<'scope> and Scope: !Unpin (which is something I did for some time). But in this case making it invariant wouldn't make a difference either. So it's never wrong to have the marker for invariance, and in most cases it's also necessary.

No, that API is usually considered safe as far as I know.

!UnwindSafe is merely a "lint" if you will; it can always be overridden by AssertUnwindSafe without unsafe code. Also unwind safety is about catching panics; the code example above doesn't catch any panic.

@steffahn, thanks for all the info already, I'll try to fully understand it in time. Allow me to go two steps back (mostly for my own sanity / understanding, but also in case other readers of the thread got caught up by the complexity).

I tried to understand what happens here, and I made this tiny example:

use std::marker::PhantomData;

struct Handle<'scope> {
    phantom: PhantomData<&'scope ()>,
    //phantom: PhantomData<&'scope mut &'scope()>,
}

fn run<'a>(_s: &'a str) {
    let handle: Handle<'static> = Handle { phantom: PhantomData };
    let handle_ref: &Handle<'static> = &handle;
    let _handle_ref_shrunk: &Handle<'a> = &*handle_ref;
}

fn main() {
    let s = String::from("ABC");
    run(&s);
    drop(s)
}

(Playground)

This demonstrates how a &Handle<'static> can be shrunk to a &Handle<'a>, because Handle<'a> is a supertype of Handle<'static>. That's counter-intuitive, as 'a is shorter than 'static, but actually Handle<'static> is a special case of Handle<'a> (i.e. we can store a Handle<'static> within a variable of type Handle<'a>). (See Nomicon on Subtyping and Variance.)

When making Handle<'scope> invariant over 'scope by replacing the first phantom field definition with the second phantom field definition, then this is no longer valid. (For invariance of PhantomData, see Nomicon on PantomData.) Also, using mutable references to Handle<'scope> makes it difficult to do the conversion (but not impossible, as @steffahn showed).

I tried to find something in the standard lib to do the trick, but I didn't find anything. std::mem::take etc. won't work as something needs to be filled into where you took the value from. I guess that's why you need something like replace_with_or_abort, as you can defer putting in the replacement value.

Ah, because only that makes the value non-movable.

That's confusing. I need to learn more about unwinding.

Oh, the crate certainly enables safe code to do things that are previously impossible (using e.g. only the standard library). Which also means that it takes away certain things that unsafe code might have been able to rely on otherwise, based on the assumption that safe code couldn't e.g. move something out of a reference and obtain an owned value, unless they get some owned value of the same type in the first place (in which case mem::replace would work). It's just, AFAIR multiple people -- perhaps even people involved in the Rust language design (I don't remember) -- say that what the replace_with crate, or equivalently the take_mut crate offers should be sound, and hence that unsafe code relying on this kind of API being disallowed should be unsound; but ultimately it's a question of convention.

2 Likes

Just out of curiosity, does the reference (or any other official documentation) take up a stance on that issue?

I don't think there's any official documentation doing that, no. Not out of the question that there's a discussion on the UCG repo or something like that; I haven't searched for that yet.

Hmmm okay. Either way, I guess that means making the scope invariant over the lifetime argument, i.e. adding a phantom: PhantomData<&'scope mut &'scope ()> is the way to be on the safe side here.

1 Like

I have looked into your code a bit, and I feel like I understand a tiny bit of it now. No need to answer, I just would like to write down some things I understand so far. (Again, mostly to sort my own thoughts, but maybe some of these thoughts might be interesting for the other readers as well.) Feel free to correct me where I'm wrong, of course.

As far as I understand, C (which implements TyCon) is a particular type constructor, which uses the Apply<'a, C> type alias to convert a lifetime 'a to a particular type (<C as TyConFor<'a>>::Applied) with lifetime 'a.

So in a way you can think of C: TyCon as a generalized "function" that takes a lifetime as an argument and returns a type. Of course it isn't a real function, as a function works on values and not on lifetimes or types. Also, calling this "function" has to follow a specific syntax, using Apply<'a, C> to feed 'a into the type constructor C, to get a particular type as result of the type constructor.

In Haskell, a type constructor that gets a type as argument and returns a type as result is said to be of kind * -> *. Viewing this from Rust's perspective, you could say Vec is a type constructor of kind * -> *, as it gets a type as an argument, e.g. i32, and returns a new type, e.g. Vec<i32>.

However, your type constructor gets a lifetime as an argument. So maybe that means C: TyCon is of kind:

' -> *

:laughing:

(Not sure if someone came up with this syntax before, or if it makes sense at all.)

I'm still not having a good understanding of the rest of your code, but I noticed that you use the following particular type constructor in your code (C is BoxDynFnMutSend<A, R> here):

struct BoxDynFnMutSend<A, R>(PhantomData<fn() -> (A, R)>);
impl<'a, A, R> TyConFor<'a> for BoxDynFnMutSend<A, R> {
    type Applied = Mutex<Box<dyn 'a + FnMut(A) -> R + Send>>;
}

Using fn() -> (A, R) instead of just (A, R) for the PhantomData means you deliberately disable drop checking (as suggested in this table). I remember that when working on my code, I also wondered if drop checking would be an issue, but I came to the conclusion that I don't need to worry about the drop check because it's done elsewhere:

Maybe you had a similar thought, or this isn't related to my case at all. I just noticed the fn() -> part in your PhantomType.

Haha, I like it. Although, AFAIK in Haskell nowadays writing things like Type -> Type instead of * -> * is preferred, so perhaps a less cryptic Lifetime -> Type notation might be all that's necessary.

By the way, I just noticed the misnomer that this type is. AFAIR, I added the Mutex around it later, but didn't change it so something like MutexBoxDynFnMutSend xD

Anyways, good thing your familiar with Haskell and "type constructor"s, as that's all that this whole approach was supposed to model. Rust doesn't properly support being generic over a higher-kinded type variable, so using such a TyCon trait with an argument and associated type is best way to work around the limitation.

It's probably irrelevant for such a phantom type, but I used the fn out of habit. In general, my main motivation when using this is that it prevents auto-trait implementations (like Send/Sync) that would be unnecessarily conditional on the type arguments otherwise. I initially wanted to use an empty enum for the BoxDynFnMutSend type (since it doesn't need to have any values at run time. But it doesn't really matter if it's not uninhabited either, so I didn't do anything to keep it uninhabited when I realized that there's a need for PhantomData because the compiler doesn't like unused type arguments :sweat_smile:.

no, as mentioned above my only thought with these was about auto-trait bounds and/or doing it out of habit. I haven't considered drop check at all. I'm not even entirely sure I'm aware of any effects on drop check at all that a PhantomData field could have in any case. You linked the table that claims some relevance... feel free to also link me to any additional information or example that demonstrates some real effects on drop check that PhantomData<T> fields can have.

Oh I see. I just made myself a small example to see what happens:

use std::marker::PhantomData;
use std::rc::Rc;

struct SendAndSync<T: ?Sized>(PhantomData<fn() -> T>);
struct MaybeSendAndSync<T: ?Sized>(PhantomData<T>);

trait Dropable {}
impl<T: ?Sized> Dropable for T {}

fn main() {
    let _b1: Box<dyn Send + Sync> = Box::new(SendAndSync::<Rc<i32>>(PhantomData));
    let _b2: Box<dyn Dropable> = Box::new(MaybeSendAndSync::<Rc<i32>>(PhantomData));
    // The following won't work:
    // let _b3: Box<dyn Send + Sync> = Box::new(MaybeSendAndSync::<Rc<i32>>(PhantomData));
}

(Playground)

I didn't realize that, as it is not mentioned in the table of PhantomData patterns in the Nomicon. However, a bit more up, it says:

Raw pointers that own an allocation is such a pervasive pattern that the standard library made a utility for itself called Unique<T> which:

  • wraps a *const T for variance
  • includes a PhantomData<T>
  • auto-derives Send / Sync as if T was contained
  • marks the pointer as NonZero for the null-pointer optimization

But the auto-derivation on Send / Sync is only mentioned in regard to Unique<T> (which appears to have been removed from std anyway), so perhaps that section should get an update to explicitly mention all effects of marking a type as being contained (opposed to using fn() -> T), and not just the drop check part. Or is that "obvious", and/or I just missed it somewhere else?

I didn't do it in-depth either, just had some shallow thoughts about it (as cited in my previous reply). This stuff still feels so complex that it's hard for me to make a judgement I feel safe with.

The section on PhantomData in the Nomicon (same page as the table, but more up) mentions the relevance of PhantomData for drop checking:

Because of the troubles this has historically caused, unbounded lifetimes and types are forbidden in struct definitions. Therefore we must somehow refer to these types in the body. Correctly doing this is necessary to have correct variance and drop checking.

We do this using PhantomData, which is a special marker type. PhantomData consumes no space, but simulates a field of the given type for the purpose of static analysis. This was deemed to be less error-prone than explicitly telling the type-system the kind of variance that you want, while also providing other useful things such as the information needed by drop check.

Other links that might be helpful:

That's right. It's perfectly fine to use atomic operations for the reference count even if it never ends up being on multiple threads.

And note that it only affects the reference count operations -- there's no difference for things like reading the value. So using Rc really only matters in places where the reference count changes frequently, but that's something that you generally should be avoiding anyway.

1 Like

FYI, when experimenting around auto-traits, it's often immensely useful to inspect the generated documentation for the struct as that contains the exact bounds for the automatic implementation.

1 Like