smol-rs / event-listener

Notify async tasks or threads
Apache License 2.0
449 stars 30 forks source link

Question about new API #118

Closed mattklein123 closed 9 months ago

mattklein123 commented 9 months ago

With the previous version I was able to do something like this:

  async fn recv(&self) -> Vec<ParsedMetric> {
    let listener = EventListener::new();
    tokio::pin!(listener);

    loop {
      if let Some(metrics) = self.lifo.lock().pop_back() {
        return metrics;
      }

      if listener.is_listening() {
        listener.as_mut().await;
      } else {
        listener.as_mut().listen(&self.event);
      }
    }
  }

In the new version this seems to roughly translate to:

  async fn recv(&self) -> Vec<ParsedMetric> {
    loop {
      if let Some(metrics) = self.lifo.lock().pop_back() {
        return metrics;
      }

      listener!(self.event => listener);

      if let Some(metrics) = self.lifo.lock().pop_back() {
        return metrics;
      }

      listener.await
    }
  }

However this fails compile with:

rror: future cannot be sent between threads safely
   --> pulse-metrics/src/pipeline/processor/buffer/mod.rs:67:7
    |
67  |       tokio::spawn(async move {
    |       ^^^^^^^^^^^^ future created by async block is not `Send`
    |
    = help: within `event_listener::sys::Inner<()>`, the trait `std::marker::Send` is not implemented for `NonNull<event_listener::sys::Link<()>>`
note: future is not `Send` as this value is used across an await
   --> pulse-metrics/src/pipeline/processor/buffer/mod.rs:87:16
    |
81  |       listener!(self.event => listener);
    |       --------------------------------- has type `StackSlot<'_, ()>` which is not `Send`
...
87  |       listener.await
    |                ^^^^^ await occurs here, with `mut $listener` maybe used later
note: required by a bound in `tokio::spawn`
   --> /Users/mklein/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.36.0/src/task/spawn.rs:166:21
    |
164 |     pub fn spawn<F>(future: F) -> JoinHandle<F::Output>
    |            ----- required by a bound in this function
165 |     where
166 |         F: Future + Send + 'static,
    |                     ^^^^ required by this bound in `spawn`

I haven't dug into the guts but it seems like awaiting the future should somehow consume it such that this would not fail the Send check? Am I doing something wrong? Thank you.

notgull commented 9 months ago

Nope, this is a bug, StackSlot should be Send.

mattklein123 commented 9 months ago

Thanks for getting back to me. Do you want me to do a PR to fix or do you plan on fixing?

notgull commented 9 months ago

I would appreciate a PR