Address review comments

CHANGELOG.md

@@ -6,8 +6,7 @@
 
 - Bump MSRV to 1.63
 - Make signals an optional feature under the `signals` features.
-- Replace the `nix` crate with standard library I/O errors and the `rustix` 
-crate.s
+- Replace the `nix` crate with standard library I/O errors and the `rustix` crate.
 - `pre_run` and `post_run` on `EventSource` have been replaced with `before_sleep` and `before_handle_events`, respectively.
   These are now opt-in through the `NEEDS_EXTRA_LIFECYCLE_EVENTS` associated constant, and occur at slightly different times to
   the methods they are replacing. This allows greater compatibility with Wayland based event sources.

Cargo.toml

@@ -24,7 +24,7 @@ bitflags = "1.2"
 futures-io = { version = "0.3.5", optional = true }
 io-lifetimes = "1.0.3"
 log = "0.4"
-nix = { version = "0.26", default-features = false, features = ["signal",], optional = true }
+nix = { version = "0.26", default-features = false, features = ["signal"], optional = true }
 pin-utils = { version = "0.1.0", optional = true }
 polling = "2.6.0"
 rustix = { version = "0.38", default-features = false, features = ["event", "fs", "pipe", "std"] }

src/io.rs

@@ -18,6 +18,7 @@ use rustix::fs::{fcntl_getfl, fcntl_setfl, OFlags};
 #[cfg(feature = "futures-io")]
 use futures_io::{AsyncRead, AsyncWrite, IoSlice, IoSliceMut};
 
+use crate::loop_logic::EventIterator;
 use crate::{
     loop_logic::{LoopInner, MAX_SOURCES_MASK},
     sources::EventDispatcher,
@@ -274,7 +275,7 @@ impl<Data> EventDispatcher<Data> for RefCell<IoDispatcher> {
     fn before_sleep(&self) -> crate::Result<Option<(Readiness, Token)>> {
         Ok(None)
     }
-    fn before_handle_events(&self, _: bool) {}
+    fn before_handle_events(&self, _: EventIterator<'_>) {}
 }
 
 /*

src/loop_logic.rs

@@ -1,10 +1,11 @@
 use std::cell::{Cell, RefCell};
 use std::fmt::Debug;
-use std::io;
+use std::iter::Chain;
 use std::rc::Rc;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
+use std::{io, slice};
 
 #[cfg(feature = "block_on")]
 use std::future::Future;
@@ -15,7 +16,8 @@ use slab::Slab;
 use crate::sources::{Dispatcher, EventSource, Idle, IdleDispatcher};
 use crate::sys::{Notifier, PollEvent};
 use crate::{
-    AdditionalLifecycleEventsSet, EventDispatcher, InsertError, Poll, PostAction, TokenFactory,
+    AdditionalLifecycleEventsSet, EventDispatcher, InsertError, Poll, PostAction, Readiness, Token,
+    TokenFactory,
 };
 
 type IdleCallback<'i, Data> = Rc<RefCell<dyn IdleDispatcher<Data> + 'i>>;
@@ -342,8 +344,7 @@ impl<'l, Data> EventLoop<'l, Data> {
                 .sources_with_additional_lifecycle_events
                 .borrow_mut();
             let sources = &self.handle.inner.sources.borrow();
-            for (source, has_event) in &mut *extra_lifecycle_sources.values {
-                *has_event = false;
+            for source in &mut *extra_lifecycle_sources.values {
                 if let Some(disp) = sources.get(source.key) {
                     if let Some((readiness, token)) = disp.before_sleep()? {
                         // Wake up instantly after polling if we recieved an event
@@ -384,19 +385,18 @@ impl<'l, Data> EventLoop<'l, Data> {
                 .sources_with_additional_lifecycle_events
                 .borrow_mut();
             if !extra_lifecycle_sources.values.is_empty() {
-                for (source, has_event) in &mut *extra_lifecycle_sources.values {
-                    for event in &events {
-                        *has_event |= (event.token.key & MAX_SOURCES_MASK) == source.key;
+                for source in &mut *extra_lifecycle_sources.values {
+                    if let Some(disp) = self.handle.inner.sources.borrow().get(source.key) {
+                        let iter = EventIterator {
+                            inner: self.synthetic_events.iter().chain(&events),
+                            registration_token: *source,
+                        };
+                        disp.before_handle_events(iter);
+                    } else {
+                        unreachable!()
                     }
                 }
             }
-            for (source, has_event) in &*extra_lifecycle_sources.values {
-                if let Some(disp) = self.handle.inner.sources.borrow().get(source.key) {
-                    disp.before_handle_events(*has_event);
-                } else {
-                    unreachable!()
-                }
-            }
         }
 
         for event in self.synthetic_events.drain(..).chain(events) {
@@ -626,6 +626,31 @@ impl<'l, Data> EventLoop<'l, Data> {
     }
 }
 
+#[derive(Clone, Debug)]
+/// The EventIterator is an `Iterator` over the events relevant to a particular source
+/// This type is used in the [`EventSource::before_handle_events`] methods for
+/// two main reasons:
+/// - To avoid dynamic dispatch overhead
+/// - Secondly, it is to allow this type to be `Clone`, which is not
+/// possible with dynamic dispatch
+pub struct EventIterator<'a> {
+    inner: Chain<slice::Iter<'a, PollEvent>, slice::Iter<'a, PollEvent>>,
+    registration_token: RegistrationToken,
+}
+
+impl<'a> Iterator for EventIterator<'a> {
+    type Item = (Readiness, Token);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        while let Some(next) = self.inner.next() {
+            if next.token.key & MAX_SOURCES_MASK == self.registration_token.key {
+                return Some((next.readiness, next.token));
+            }
+        }
+        None
+    }
+}
+
 /// A signal that can be shared between thread to stop or wakeup a running
 /// event loop
 #[derive(Clone)]
@@ -671,8 +696,8 @@ mod tests {
         channel::{channel, Channel},
         generic::Generic,
         ping::*,
-        Dispatcher, EventSource, Interest, Mode, Poll, PostAction, Readiness, RegistrationToken,
-        Token, TokenFactory,
+        Dispatcher, EventIterator, EventSource, Interest, Mode, Poll, PostAction, Readiness,
+        RegistrationToken, Token, TokenFactory,
     };
 
     use super::EventLoop;
@@ -859,7 +884,7 @@ mod tests {
                 Ok(None)
             }
 
-            fn before_handle_events(&mut self, _: bool) {
+            fn before_handle_events(&mut self, _: EventIterator) {
                 self.lock.unlock();
             }
         }
@@ -1015,7 +1040,7 @@ mod tests {
                 Ok(Some((Readiness::EMPTY, self.token.unwrap())))
             }
 
-            fn before_handle_events(&mut self, _: bool) {
+            fn before_handle_events(&mut self, _: EventIterator) {
                 self.lock.unlock();
             }
         }

src/sources/mod.rs

@@ -4,6 +4,7 @@ use std::{
     rc::Rc,
 };
 
+pub use crate::loop_logic::EventIterator;
 use crate::{sys::TokenFactory, Poll, Readiness, RegistrationToken, Token};
 
 pub mod channel;
@@ -168,24 +169,34 @@ pub trait EventSource {
     /// and [`EventSource::before_handle_events`] notifications. These are opt-in because
     /// they require more expensive checks, and almost all sources will not need these notifications
     const NEEDS_EXTRA_LIFECYCLE_EVENTS: bool = false;
-    /// Notification that a single `poll` is about to begin.
-    /// If this returns Ok(Some), polling will shortcircuit, and
-    /// your event handler will be called with the returned Token and Readiness
+    /// Notification that a single `poll` is about to begin
     ///
     /// Use this to perform operations which must be done before polling,
     /// but which may conflict with other event handlers. For example,
-    /// if polling requires a lock to be taken.
+    /// if polling requires a lock to be taken
+    ///
+    /// If this returns Ok(Some), this will be treated as an event arriving in polling, and
+    /// your event handler will be called with the returned `Token` and `Readiness`.
+    /// Polling will however still occur, so additional events from this or other sources
+    /// may also be handled in the same loop.
+    /// The returned `Token` must belong to this source
+    // If you need to return multiple synthetic events from this notification, please
+    // open an issue
     fn before_sleep(&mut self) -> crate::Result<Option<(Readiness, Token)>> {
         Ok(None)
     }
-    /// Notification that polling is completed, and the resulting events are
-    /// going to be executed.
+    /// Notification that polling is complete, and [`EventSource::process_events`] will
+    /// be called with the given events for this source. The iterator may be empty,
+    /// which indicates that no events were generated for this source
+    ///
+    /// Please note, the iterator will also include any synthetic event returned from
+    /// [`EventSource::before_sleep`]
     ///
     /// Use this to perform a cleanup before event handlers with arbitrary
     /// code may run. This could be used to drop a lock obtained in
     /// [`EventSource::before_sleep`]
     #[allow(unused_variables)]
-    fn before_handle_events(&mut self, was_awoken: bool) {}
+    fn before_handle_events(&mut self, events: EventIterator<'_>) {}
 }
 
 /// Blanket implementation for boxed event sources. [`EventSource`] is not an
@@ -230,8 +241,8 @@ impl<T: EventSource> EventSource for Box<T> {
         T::before_sleep(&mut **self)
     }
 
-    fn before_handle_events(&mut self, was_awoken: bool) {
-        T::before_handle_events(&mut **self, was_awoken)
+    fn before_handle_events(&mut self, events: EventIterator) {
+        T::before_handle_events(&mut **self, events)
     }
 }
 
@@ -278,8 +289,8 @@ impl<T: EventSource> EventSource for &mut T {
         T::before_sleep(&mut **self)
     }
 
-    fn before_handle_events(&mut self, was_awoken: bool) {
-        T::before_handle_events(&mut **self, was_awoken)
+    fn before_handle_events(&mut self, events: EventIterator) {
+        T::before_handle_events(&mut **self, events)
     }
 }
 
@@ -365,10 +376,10 @@ where
         source.before_sleep()
     }
 
-    fn before_handle_events(&self, was_awoken: bool) {
+    fn before_handle_events(&self, events: EventIterator<'_>) {
         let mut disp = self.borrow_mut();
         let DispatcherInner { ref mut source, .. } = *disp;
-        source.before_handle_events(was_awoken);
+        source.before_handle_events(events);
     }
 }
 
@@ -402,26 +413,23 @@ pub(crate) trait EventDispatcher<Data> {
     ) -> crate::Result<bool>;
 
     fn before_sleep(&self) -> crate::Result<Option<(Readiness, Token)>>;
-    fn before_handle_events(&self, was_awoken: bool);
+    fn before_handle_events(&self, events: EventIterator<'_>);
 }
 
 #[derive(Default)]
 /// The list of events
 pub(crate) struct AdditionalLifecycleEventsSet {
-    /// The list of events. The boolean indicates whether the source had an event
-    /// - this is stored in this list because we need to know this value for each
-    /// item at once - that is, to avoid allocating in the hot loop
-    pub(crate) values: Vec<(RegistrationToken, bool)>,
+    /// The list of sources
+    pub(crate) values: Vec<RegistrationToken>,
 }
 
 impl AdditionalLifecycleEventsSet {
     fn register(&mut self, token: RegistrationToken) {
-        self.values.push((token, false))
+        self.values.push(token)
     }
 
     fn unregister(&mut self, token: RegistrationToken) {
-        self.values
-            .retain(|it: &(RegistrationToken, bool)| it.0 != token)
+        self.values.retain(|it| it != &token)
     }
 }