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

[Question] Why does event listener need 'static lifetime #239

Closed
xmclark opened this issue Jun 20, 2018 · 5 comments
Closed

[Question] Why does event listener need 'static lifetime #239

xmclark opened this issue Jun 20, 2018 · 5 comments

Comments

@xmclark
Copy link

xmclark commented Jun 20, 2018

I'm starting out with some of the examples and am exploring ways to manage mutable state. One restriction I have run into is the 'static requirement for the event listener closure. This is from the IEventTarget trait:

    fn add_event_listener< T, F >( &self, listener: F ) -> EventListenerHandle
        where T: ConcreteEvent, F: FnMut( T ) + 'static

In the examples, a Rc<RefCell<_>> is passed to the closure so we can safely mutate the state. It is a convenient tool for getting around the borrow checker. Both Rc and RefCell add to runtime overhead which makes me a little uncomfortable, so I started looking for a different solution. The best one I have so far is using a static mut state and wrapping all usage inside an unsafe block. This feels wrong to me.

Why is the trait written this way, and what other options do I have for mutating my state in the event listener closure?

@xmclark xmclark changed the title [Question] Why does event listener need to have 'static lifetime [Question] Why does event listener need 'static lifetime Jun 20, 2018
@Pauan
Copy link
Contributor

Pauan commented Jun 21, 2018

The 'static restriction simply means "it cannot use references which exist on the stack."

The reason is because JavaScript can call the closure at any time (including asynchronously).

Imagine that you have some stack variable which the closure is using:

let mut x = "foobar";

foo.add_event_listener(|e: MouseClickEvent| {
    x = "quxcorge";
});

There's two problems here:

  1. The closure is called asynchronously, so when the event has happened, the stack frame is already gone, which means the variable x no longer exists: that's a use-after-free bug!

  2. Even if the closure was called synchronously, JavaScript might take that closure, store it away somewhere, and then call it later (after the stack frame is gone). Again, a use-after-free bug!

In order to fix this, you can simply move the variable into the closure:

let mut x = "foobar";

foo.add_event_listener(move |e: MouseClickEvent| {
    x = "quxcorge";
});

Since the closure now owns the variable x, the variable x is no longer on the stack, so everything works out perfectly. In addition, this has absolutely no performance cost.

However, the downside of the above approach is that you can only access the variable within a single closure. If you have some state that you want to share with many closures, then you must use Rc + RefCell / Cell (or Arc + Mutex / RwLock / AtomicUsize / etc.):

let x = Rc::new(RefCell::new("foobar"));

{
    let x = x.clone();

    foo.add_event_listener(move |e: MouseClickEvent| {
        let x = x.borrow_mut();
        *x = "quxcorge";
    });
}

{
    let x = x.clone();

    bar.add_event_listener(move |e: MouseClickEvent| {
        let x = x.borrow_mut();
        *x = "quxcorge";
    });
}

This works because Rc moves the value into the heap, so it's no longer on the stack, so now it can be safely used inside of the closure. The RefCell is needed because Rc doesn't allow for mutation, so you need to use RefCell / Cell / etc. to add mutation.

As another alternative, you can use lazy_static! instead:

lazy_static! {
    static ref X: Mutex<&'static str> = Mutex::new("foobar");
}

foo.add_event_listener(|e: MouseClickEvent| {
    let x = X.lock().unwrap();
    *x = "quxcorge";
});

bar.add_event_listener(|e: MouseClickEvent| {
    let x = X.lock().unwrap();
    *x = "quxcorge";
});

This still has a performance cost (from the Mutex), and it also means that the state will live forever, it will never be dropped. In addition, lazy_static requires the data type to implement Sync, and not all data types implement Sync! Sometimes this is what you want, but I personally recommend Rc instead.

Technically you could use a mutable static (and unsafe) to avoid the performance cost, but I also don't recommend that, for similar reasons as lazy_static.

In summary:

If you have some state which only needs to be used by a single closure, just move it. This has no performance cost, and it's very convenient.

If you have some state which needs to be shared between multiple closures, then you will pay a performance cost. The performance cost is extremely small, so it's not worth worrying about.

To put it into perspective, Rc does a single heap-allocation (when the Rc is created), and RefCell does an integer check and integer set (every time that you call borrow or borrow_mut).

In addition, using Cell has essentially no performance cost, so if you're using Copy types (like bool, u32, etc.) then you only pay a one-time cost when you create the Rc.

In contrast, every object in JavaScript is heap-allocated, and the overhead is generally much worse than an integer check/set!

@xmclark
Copy link
Author

xmclark commented Jun 21, 2018

Wow this write-up was excellent. Thanks for some of the rust background.

The additional option of lazy_static is one I did not think of. I'm also not informed of the differences between RefCell and Cell, so I can read up on that in my own time.

These statements were pretty helpful for me:

If you have some state which needs to be shared between multiple closures, then you will pay a performance cost. The performance cost is extremely small, so it's not worth worrying about.

To put it into perspective, Rc does a single heap-allocation (when the Rc is created), and RefCell does an integer check and integer set (every time that you call borrow or borrow_mut).

Again, I did not know too much about how well RefCell is implemented. If it does a simple int set/clr then I already feel a lot better about using it.

One reason I could see for preferring state stay on the stack is that you might get better performance accessing a reference on the stack than an object on heap. It may be trivial, and it may be a wasm browser implementation detail.

Some additional thoughts:
The 'static requirement is strict. I wonder if you could tie the lifetime requirement to the lifetime of a event listener token that gets returned to you from add_event_listener. If an event_listener was tied to to the lifetime of a corresponding event_listener_token, then the listener would automatically detach when the token goes out of scope. You could then have the lifetime of stack state depend on the lifetime of the token and you would never worry about a use-after-free.

Thanks, and keep up the good work!

@xmclark xmclark closed this as completed Jun 21, 2018
@Pauan
Copy link
Contributor

Pauan commented Jun 22, 2018

Again, I did not know too much about how well RefCell is implemented. If it does a simple int set/clr then I already feel a lot better about using it.

I personally find it very enlightening to read the source code for the standard library. Everything's in Rust, so it's usually quite easy to read.

  1. First, you find the thing you're interested in.

  2. Then you click the [src] button, which shows you the source code.

  3. Then you poke around, investigate things, etc.

In the case of RefCell, you can see that it contains a Cell (which is a way of mutating things that has no performance cost).

When you call borrow_mut, it creates a BorrowRefMut, which contains a reference to the RefCell's Cell. This will all get inlined away, so don't worry about any overhead.

When it creates the BorrowRefMut, it does an integer check to make sure that the Cell isn't currently being used, and then it mutates the Cell to WRITING (which is an integer which indicates that the RefCell is currently being used).

And that's pretty much it! It might seem like a lot of code, but because it's all inlined it will end up being extremely small and fast.

One reason I could see for preferring state stay on the stack is that you might get better performance accessing a reference on the stack than an object on heap. It may be trivial, and it may be a wasm browser implementation detail.

I'm not 100% sure, but I'm pretty sure there's zero difference between accessing a stack reference and a heap reference. They both end up going into the wasm linear memory (just in different spots).

The 'static requirement is strict.

It both is and it isn't. It's extremely annoying when you first find out about it, but after you get used to using Rc + RefCell then it isn't that big of a deal, just a minor annoyance.

For my own programs, I usually create a single State struct which contains everything I need, and then I put it into an Rc, and then I use Cell / RefCell for the individual fields:

struct State {
    foo: Cell<bool>,
    bar: RefCell<String>,
}

fn main() {
    let state = Rc::new(State {
        foo: Cell::new(false),
        bar: RefCell::new("".to_owned()),
    });
}

I also created a simple clone! macro which makes it easy to clone things:

foo.add_event_listener(clone!(state => move |e: MouseClickEvent| {
    state.foo.set(true);

    let bar = state.bar.borrow_mut();

    // ...
}));

This significantly improves the ergonomics of using Rc / Cell / RefCell.

Alternatively, rather than putting RefCell / Cell on individual fields, you can use Rc<RefCell<State>>, which is also completely fine.

I wonder if you could tie the lifetime requirement to the lifetime of a event listener token that gets returned to you from add_event_listener. If an event_listener was tied to to the lifetime of a corresponding event_listener_token, then the listener would automatically detach when the token goes out of scope. You could then have the lifetime of stack state depend on the lifetime of the token and you would never worry about a use-after-free.

It's a great idea (which I had thought about before), but unfortunately it doesn't work. Consider this simple example:

let mut x = "foobar";

let listener = foo.add_event_listener(|e| {
    x = "quxcorge";
});

std::mem::forget(listener);

In this case we're using std::mem::forget to leak the event listener token, which means the event listener is never cleaned up, which means the event listener outlives the stack.

There's many ways to leak things in Rust (including Rc cycles), std::mem::forget is just one of them. So banning/avoiding std::mem::forget isn't enough.

The exact same problem happened with thread::scoped (which was used to access stack variables in other threads). They had implemented it with a JoinGuard<'a>, which was supposed to tie the thread to the stack lifetime. But because it's possible to leak the JoinGuard<'a> that caused use-after-free bugs.

Also, even if we could tie the event listener token to the stack, it wouldn't be as useful as you might think. Consider this example:

fn add_listener<'a>(foo: &'a Node) -> EventListener<'a> {
    let mut x = "foobar";

    let listener = foo.add_event_listener(|e| {
        x = "quxcorge";
    });

    listener
}

In this case we're returning the EventListener token, so the EventListener token outlives the add_listener function. But the event listener is accessing the variable x, which will be destroyed when the add_listener function returns. In this case Rust will correctly give a compile-time error telling you that the event listener outlives the variable x.

And if you create the listeners in the main function, you end up with a different problem:

fn main() {
    let foo = ...;

    let mut x = "foobar";

    let listener1 = foo.add_event_listener(|e| {
        x = "quxcorge";
    });

    let listener2 = foo.add_event_listener(|e| {
        x = "quxcorge";
    });
}

In this case the event listener tokens are tied to the stack, so as soon as the main function returns the event listeners will be removed. Since the main function returns immediately after creating the event listeners, that means the event listeners will be immediately removed and will never fire.

@xmclark
Copy link
Author

xmclark commented Jun 22, 2018

Then you click the [src] button, which shows you the source code.

Wow I did not know this was a thing! This will be my new favorite link when browsing rust docs.

I'm not 100% sure, but I'm pretty sure there's zero difference between accessing a stack reference and a heap reference. They both end up going into the wasm linear memory (just in different spots).

This makes a lot of sense. I assumed it might not matter due to implementation details like the linear memory.

...

Holy cow you must have spent sometime putting that last explanation together. I really appreciate it. I think I can learn to live comfortably using the Cells.

@Pauan
Copy link
Contributor

Pauan commented Jun 24, 2018

Wow I did not know this was a thing!

It even works for third-party crates like stdweb!

I really appreciate it.

Sure, I'm glad I was able to help.

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