`borrowed data escapes outside of closure` in event handling

Hello everyone.
I am trying to implement a sort of multilevel EventHandler (Callback with parameter), where each layer, handles the parameter as a reference.
Obviously those handlers won't be able to store that event, because of the lifetime - just mutate inner state without storing the event (or it's reference).

In this minimal Example I have Reader, that does not forward this event yet. It's empty for purpopse of being minimal, in the normal case it would somehow mutate inner state and the pass it to the next EventHandler layer.

use core::marker::PhantomData;

struct Event<'a> {
    msg: &'a str,
}

struct Listener(i32);
impl Listener {
    fn for_each(mut self, mut f: impl FnMut(Event)) {
        for i in 1..10 {
            let message = String::from("Hello, World!");
            self.0 = i;
            f(Event { msg: &message });
        }
    }
}

struct Engine {}
impl Engine {
    fn foo(&mut self, listener: Listener, mut reader: Reader<Event>) {
        let f = move |event: Event|reader.read(&event); //`event` escapes the closure body here
        listener.for_each(f);
    }
}

struct Reader<E>(PhantomData<E>);
impl<E> Reader<E> {
    fn read(&mut self, _: &E) {}
}

I have no clue, why this would not compile. I thought maybe the compiler thinks that Reader::read tries to store the event because of the &mut self (since it compiles with plain &self), but it wouldn't even be able to because of missing lifetimes on Reader. Listener and Event are 3rd party, so I can't change them. I can't imagine why being able to mutate the inner state of Reader would be not safe in this case (Idk if that even is the problem). All in all I can't see how what I am trying to do would be unsafe.

To understand the lifetimes in this code, let’s start out by addressing the lifetime elision that is going on.

If you have a struct with a lifetime argument like Event<'a>, it is generally not advisable to ever write “Eventwithout that lifetime argument, since that’s relying on old (and hopefully, eventually, deprecated) syntax for lifetime elision which does regularly and easily lead to confusion simply by the fact that there is any lifetime elision going on in the first place is very well-hidden.

We end up with elided lifetimes in three places, marked below:

use core::marker::PhantomData;

struct Event<'a> {
    msg: &'a str,
}

struct Listener(i32);
impl Listener {
    fn for_each(mut self, mut f: impl FnMut(Event<'_>)) { // <- here
        for i in 1..10 {
            let message = String::from("Hello, World!");
            self.0 = i;
            f(Event { msg: &message });
        }
    }
}

struct Engine {}
impl Engine {
    fn foo(&mut self, listener: Listener, mut reader: Reader<Event<'_>>) {  // <- here
        let f = move |event: Event<'_>| reader.read(&event);  // <- here
        listener.for_each(f);
    }
}

struct Reader<E>(PhantomData<E>);
impl<E> Reader<E> {
    fn read(&mut self, _: &E) {}
}

By the way note that the line f(Event { msg: &message }) is different as “Event” in this line is not the type Event itself, but the value-constructor for Event.


Next up, since lifetime elision can be confusing, especially since the meaning depends heavily on context, let’s de-sugar the elided Event<'_> lifetimes into explicit ones. In one place that’s straightforward:

fn foo<'a>(&mut self, listener: Listener, mut reader: Reader<Event<'a>>) {

one place needs knowledge of the special elision rules for Fn… traits,

fn for_each(mut self, mut f: impl for<'b> FnMut(Event<'b>)) {

and one place does, really, not allow any desugaring at all; but the meaning should be clear if we want to match the FnMut signature; it’s a generic lifetime here as well, and we can use unstable features to spell it out if we want to be very explicit:

#![feature(closure_lifetime_binder)]

……
let f = for<'b> move |event: Event<'b>| -> () { reader.read(&event) };

(full code at this point in the playground)


Now to analyze the problem: It’s a simple type mismatch!

impl Engine {
    fn foo<'a>(&mut self, listener: Listener, mut reader: Reader<Event<'a>>) {
        let f = for<'b> move |event: Event<'b>| -> () { reader.read(&event) };
        listener.for_each(f);
    }
}
impl<E> Reader<E> {
    fn read(&mut self, _: &E) {}
}

The reader.read call for Reader<Event<'a>> expects a reference to Event<'a>, whereas the event you pass a reference to is of type Event<'b> with a different lifetime. The fact that the 'a lifetime is one provided to foo<'a>, and thus a lifetime that outlives the function, whereas event: Event<'b> is contains a &'b str reference that is potentially much more short-lived, inside of a closure, is the reason why we get the particular error message that we get.

error[E0521]: borrowed data escapes outside of closure
  --> src/lib.rs:23:57
   |
22 |     fn foo<'a>(&mut self, listener: Listener, mut reader: Reader<Event<'a>>) {
   |                                               ---------- `reader` declared here, outside of the closure body
23 |         let f = for<'b> move |event: Event<'b>| -> () { reader.read(&event) };
   |                               -----                     ^^^^^^^^^^^^^^^^^^^ `event` escapes the closure body here
   |                               |
   |                               `event` is a reference that is only valid in the closure body

Whether this error message would have been more helpful if it called out the lifetime mismatch more explicitly, is something that could be discussed.[1]


As for how to solve the problem: There’s no good way after all, by just changing the lifetimes. Looking at your code you are passing very short-lived references to local String variables with the listener.for_each(f); call. Those references are already dead again, and the Strings deallocated, after the call to .for_each was finished. On the other hand (at least, I assume, based on the signature), Reader<E> will probably assume to actually obtain some values of type E that it can keep around for longer, which of course it cannot if the thing it actually gets passed is (containing) a short-lived reference that dies long before the Reader goes out of scope.

There are 3 options I can think of how to go about solving the problem now:

  • the Reader could possibly be living longer than it needs to, so so maybe foo needs re-factoring of some sort (though in foo alone I don't have any concrete ideas how to fully address the issue)
  • the Reader type might actually not need to retain any E values; perhaps, and maybe even likely, judging by the &E type of the argument, it only need to be able to look at the value of type E during the call to read, but not retain any copies or anything. It’s not trivial to make this more generic so different, but related, types of E are supported by a single Reader – I don’t think I can give any good suggestions without seeing more than this minimal/toy example, from which alone it’s impossible to guess what Reader actually does, and what kind of abstraction it represents / what types besides Event<…> it’s used with, etc… E.g. perhaps the real code involves some trait bounds on E, in which case that trait could possibly be re-worked to allow for usage generic over some lifetime. Also it isn’t clear whether E only exists as PhantomData in the real code, too, or whether actual E values are being stored, which would be a strong indicator that perhaps moving to owned values (see below) is the only option…
  • the usage of a borrowed &str might be unwanted. Perhaps you do want some owned value after all. Maybe a String; maybe something shared like Arc<String> or Arc<str>, or something else

  1. Out of curiosity, I tried out what happens if the lifetime was invariant and the error message gets even more confusing :sweat_smile: ↩︎

6 Likes

Hi, thanks for the reply,

this is the "Real code"

Reader changed to EventPublisher

EventPublisher::publish just forwards the event (the reference) to it's event_handlers.
The event should NOT be hold onto, meaning they don't store it somehow. Only mutate inner state without storing the event.

struct Event<'a> {
    msg: &'a str,
}

struct Listener(i32);
impl Listener {
    fn for_each(mut self, mut f: impl FnMut(Event)) {
        for i in 1..10 {
            let message = String::from("Hello, World!");
            self.0 = i;
            f(Event { msg: &message });
        }
    }
}

struct Engine {}
impl Engine {
    fn foo(&mut self, listener: Listener, mut publisher: EventPublisher<Event>) {
        let f = move |event: Event|publisher.publish(&event); //`event` escapes the closure body here
        listener.for_each(f);
    }
}


pub struct EventPublisher<'a, T> {
    event_handlers: Vec<Box<dyn EventHandler<T> + 'a>>,
}

impl<'a, T> EventPublisher<'a, T> {
    pub fn add_event_handler(&mut self, event_handler: impl EventHandler<T> + 'a) {
        self.event_handlers.push(Box::new(event_handler));
    }

    pub fn publish(&mut self, payload: &T) {
        for event_handler in &mut self.event_handlers {
            event_handler.handle_event(payload);
        }
    }
}

pub trait EventHandler<T> {
    fn handle_event(&mut self, event: &T);
}

Is EventPublisher<'_, T> ever used with a type T that’s different from Event<'a> of some lifetime 'a? If yes, are these all some sort of “event” types? If yes, this might be solvable e.g. with a trait, perhaps with a generic associated type… I can explain more if I know whether it’s even a useful approach ^^

Right… if I understand the code correctly though, it’s currently not impossible that these event handlers could be copying the &str reference out of the Event. If that’s something that’s actually happening, then of course, there’d be an actual problem; otherwise, it’s necessary to refactor the types in a way that it becomes impossible for event handlers to keep the &str references around for too long; which is what my above discussion in the previous paragraph is starting to get into.

1 Like

EventPublisher<'_, T> is used whenever there is some generic event to be handled (Not to be confused with Event<'a>). For now it's just T without any constraint so practically anything could be an Eventpayload. But I am flexible to change it if needed.

Ohh I assumed that, because I do not specify any lifetime in EventHandler::handle_event(&mut self, event: &T) it would be impossible to store the event (or any references inside of it), since I did not define fn handle_event<'a>(&'a mut self, event: &'a T);

Correct me if I got anything wrong :sweat_smile:

You’re mixing up lifetimes. The one I’m referring to is hidden in the T, which you instantiate by T = Event<'some_lifetime> :wink:


E.g.

struct Handler<'a>(Vec<&'a str>);

impl<'a> EventHandler<Event<'a>> for Handler<'a> {
    fn handle_event(&mut self, event: &Event<'a>) {
        self.0.push(event.msg);
    }
}

fn demo() {
    let s = "hi".to_string();
    let mut pu = EventPublisher {
        event_handlers: vec![],
    };
    pu.add_event_handler(Handler(vec![]));
    pu.publish(&Event{ msg: &s });
}

I’ll simply present the basic idea that I mentioned of using a trait with a GAT, in the form of this playground :wink:

2 Likes

Ahh yes, this looks promising, thank you very much.
But I am not able to implement a simple Logger EventHandler. For example to log an Event<'_>.

struct Logger;
impl EventHandler<Event<'_>> for Logger {
    fn handle_event(&mut self, event: &Event<'_>) {
        dbg!(event);
    }
}

the type Event<'_> does not fulfill the required lifetime
impl EventHandler<Event<'_>> for Logger {

I think I need to somehow implement it for every 'e ,but I do not know how to do this.
What do I do wrong? Do you maybe have an example?
thanks in advance

Every time you use '_ you are referring to a possibly different lifetime, so each Event<'_> is a different type. You need to use a named lifetime instead.

I think I need to somehow implement it for every 'e ,but I do not know how to do this.

Yes. The way you do that is by declaring a lifetime parameter. For impls, the declarations go right after the impl keyword:

impl<'e> EventHandler<Event<'e>> for Logger {
    fn handle_event(&mut self, event: &Event<'e>) {

But this is still not higher ranked trait bound. (for<'e> syntax)

It might help if you provide a full error message (as e.g. reported on the terminal with cargo check), and perhaps the relevant trait implementations in question, as well as the code where the error appears ^^. At least I myself don’t have a good picture of the precise compilation error you are facing :wink:

use std::vec;

pub trait Event {
    type EventPayload<'e>;
}

impl<'a> Event for MessageEvent<'a> {
    type EventPayload<'e> = MessageEvent<'e>;
}

#[derive(Debug)]
struct MessageEvent<'a> {
    msg: &'a str,
}

struct Listener(i32);
impl Listener {
    fn for_each(mut self, mut f: impl FnMut(MessageEvent<'_>)) {
        for i in 1..10 {
            let message = String::from("Hello, World!");
            self.0 = i;
            f(MessageEvent { msg: &message });
        }
    }
}

struct Engine {}
impl Engine {
    fn foo(
        &mut self,
        listener: Listener,
        mut publisher: EventPublisher<MessageEvent<'_>>,
    ) {
        let f = move |event: MessageEvent<'_>| publisher.publish(&event);
        listener.for_each(f);
    }
}

pub struct EventPublisher<'a, T: Event> {
    event_handlers: Vec<Box<dyn for<'e> EventHandler<T::EventPayload<'e>> + 'a>>,
}

impl<'a, T: Event> EventPublisher<'a, T> {
    pub fn add_event_handler(
        &mut self,
        event_handler: impl for<'e> EventHandler<T::EventPayload<'e>> + 'a,
    ) {
        self.event_handlers.push(Box::new(event_handler));
    }

    pub fn publish(&mut self, payload: &T::EventPayload<'_>) {
        for event_handler in &mut self.event_handlers {
            event_handler.handle_event(payload);
        }
    }
}

pub trait EventHandler<T> {
    fn handle_event(&mut self, event: &T);
}

struct Logger;
impl<'a> EventHandler<MessageEvent<'a>> for Logger {
    fn handle_event(&mut self, event: &MessageEvent<'a>) {
        dbg!(event);
    }
}




fn main() {
    let mut publisher = EventPublisher {
        event_handlers: vec![],
    };

    let logger = Logger{};
    publisher.add_event_handler(logger);

    let listener = Listener(10);
    Engine {}.foo(listener, publisher)
}

    Checking new_event v0.1.0 (/home/deni/programming/rust/new_event)
error[E0277]: the trait bound `for<'e> Logger: EventHandler<<_ as Event>::EventPayload<'e>>` is not satisfied
  --> src/main.rs:78:33
   |
78 |     publisher.add_event_handler(logger);
   |               ----------------- ^^^^^^ the trait `for<'e> EventHandler<<_ as Event>::EventPayload<'e>>` is not implemented for `Logger`
   |               |
   |               required by a bound introduced by this call
   |
   = help: the trait `EventHandler<MessageEvent<'a>>` is implemented for `Logger`
note: required by a bound in `EventPublisher::<'a, T>::add_event_handler`
  --> src/main.rs:46:29
   |
44 |     pub fn add_event_handler(
   |            ----------------- required by a bound in this associated function
45 |         &mut self,
46 |         event_handler: impl for<'e> EventHandler<T::EventPayload<'e>> + 'a,
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `EventPublisher::<'a, T>::add_event_handler`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `new_event` due to previous error

I think I kinda understand what the problem is, but I do not know how to fix / impl it

The error message seems quite misleading (maybe one should consider this a compiler bug…), the actual problem is just the underspecified type parameter the compiler calls _ in “EventHandler<<_ as Event>::EventPayload<'e>>”. Instead of complaining about the underspecified type parameter, it instead errors complaining about some trait bound involving this unknown type not being fulfilled. Simply ignoring the compiler’s phrasing of the error, and adding the necessary type information like this

    let mut publisher = EventPublisher::<'_, MessageEvent<'_>> {
        event_handlers: vec![],
    };

makes the whole code compile successfully. My suggestion in the other code was to actually introduce a new helper type MessageEvent_ to abstract over the whole family of types MessageEvent<'e> for all lifetimes 'e (that’s also why I called it “EventTypeFamily” to suggest this kind of notion). So I would personally like the approach better of using

struct MessageEvent_;
impl<'a> Event for MessageEvent_ {
    type EventPayload<'e> = MessageEvent<'e>;
}

and

    let mut publisher = EventPublisher::<'_, MessageEvent_> {
        event_handlers: vec![],
    };

and thus eliminating the redundant additional lifetime parameter in the EventPublisher<…> type. The way you do it means that any MessageEvent<'a> type will play both the role of a concrete type, and of a marker type for identifying the whole family of MessageEvent<'e> types for all lifetimes 'e, via the impl<'a> Event for MessageEvent<'a> implementations.