diff --git a/CHANGELOG.md b/CHANGELOG.md index 8efaeb74..146374db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Fix an issue, where id-reuse could execute a PostAction on a newly registered event source + ## 0.12.2 -- 2023-09-25 #### Bugfixes diff --git a/src/io.rs b/src/io.rs index bc10b252..e8130069 100644 --- a/src/io.rs +++ b/src/io.rs @@ -62,7 +62,7 @@ impl<'l, F: AsFd> Async<'l, F> { interest: Interest::EMPTY, last_readiness: Readiness::EMPTY, })); - let key = inner.sources.borrow_mut().insert(dispatcher.clone()); + let key = inner.sources.borrow_mut().insert(Some(dispatcher.clone())); dispatcher.borrow_mut().token = Some(Token { key }); // SAFETY: We are sure to deregister on drop. diff --git a/src/loop_logic.rs b/src/loop_logic.rs index 22868db0..2b30d715 100644 --- a/src/loop_logic.rs +++ b/src/loop_logic.rs @@ -69,7 +69,7 @@ impl RegistrationToken { pub(crate) struct LoopInner<'l, Data> { pub(crate) poll: RefCell, - pub(crate) sources: RefCell + 'l>>>, + pub(crate) sources: RefCell + 'l>>>>, pub(crate) sources_with_additional_lifecycle_events: RefCell, idles: RefCell>>, pending_action: Cell, @@ -149,9 +149,9 @@ impl<'l, Data> LoopHandle<'l, Data> { ))); } - let key = sources.insert(dispatcher.clone_as_event_dispatcher()); + let key = sources.insert(Some(dispatcher.clone_as_event_dispatcher())); trace!("[calloop] Inserting new source #{}", key); - let ret = sources.get(key).unwrap().register( + let ret = sources.get(key).unwrap().as_ref().unwrap().register( &mut poll, &mut self .inner @@ -189,7 +189,7 @@ impl<'l, Data> LoopHandle<'l, Data> { /// /// **Note:** this cannot be done from within the source callback. pub fn enable(&self, token: &RegistrationToken) -> crate::Result<()> { - if let Some(source) = self.inner.sources.borrow().get(token.key) { + if let Some(Some(source)) = self.inner.sources.borrow().get(token.key) { trace!("[calloop] Registering source #{}", token.key); source.register( &mut self.inner.poll.borrow_mut(), @@ -208,7 +208,7 @@ impl<'l, Data> LoopHandle<'l, Data> { /// If after accessing the source you changed its parameters in a way that requires /// updating its registration. pub fn update(&self, token: &RegistrationToken) -> crate::Result<()> { - if let Some(source) = self.inner.sources.borrow().get(token.key) { + if let Some(Some(source)) = self.inner.sources.borrow().get(token.key) { trace!("[calloop] Updating registration of source #{}", token.key); if !source.reregister( &mut self.inner.poll.borrow_mut(), @@ -230,7 +230,7 @@ impl<'l, Data> LoopHandle<'l, Data> { /// /// The source remains in the event loop, but it'll no longer generate events pub fn disable(&self, token: &RegistrationToken) -> crate::Result<()> { - if let Some(source) = self.inner.sources.borrow().get(token.key) { + if let Some(Some(source)) = self.inner.sources.borrow().get(token.key) { trace!("[calloop] Unregistering source #{}", token.key); if !source.unregister( &mut self.inner.poll.borrow_mut(), @@ -250,20 +250,22 @@ impl<'l, Data> LoopHandle<'l, Data> { /// Removes this source from the event loop. pub fn remove(&self, token: RegistrationToken) { - if let Some(source) = self.inner.sources.borrow_mut().try_remove(token.key) { - trace!("[calloop] Removing source #{}", token.key); - if let Err(e) = source.unregister( - &mut self.inner.poll.borrow_mut(), - &mut self - .inner - .sources_with_additional_lifecycle_events - .borrow_mut(), - token, - ) { - log::warn!( - "[calloop] Failed to unregister source from the polling system: {:?}", - e - ); + if let Some(source) = self.inner.sources.borrow_mut().get_mut(token.key) { + if let Some(source) = source.take() { + trace!("[calloop] Removing source #{}", token.key); + if let Err(e) = source.unregister( + &mut self.inner.poll.borrow_mut(), + &mut self + .inner + .sources_with_additional_lifecycle_events + .borrow_mut(), + token, + ) { + log::warn!( + "[calloop] Failed to unregister source from the polling system: {:?}", + e + ); + } } } } @@ -353,7 +355,7 @@ impl<'l, Data> EventLoop<'l, Data> { .borrow_mut(); let sources = &self.handle.inner.sources.borrow(); for source in &mut *extra_lifecycle_sources.values { - if let Some(disp) = sources.get(source.key) { + if let Some(Some(disp)) = sources.get(source.key) { if let Some((readiness, token)) = disp.before_sleep()? { // Wake up instantly after polling if we recieved an event timeout = Some(Duration::ZERO); @@ -394,7 +396,7 @@ impl<'l, Data> EventLoop<'l, Data> { .borrow_mut(); if !extra_lifecycle_sources.values.is_empty() { for source in &mut *extra_lifecycle_sources.values { - if let Some(disp) = self.handle.inner.sources.borrow().get(source.key) { + if let Some(Some(disp)) = self.handle.inner.sources.borrow().get(source.key) { let iter = EventIterator { inner: events.iter(), registration_token: *source, @@ -419,7 +421,7 @@ impl<'l, Data> EventLoop<'l, Data> { .get(registroken_token) .cloned(); - if let Some(disp) = opt_disp { + if let Some(Some(disp)) = opt_disp { trace!( "[calloop] Dispatching events for source #{}", registroken_token @@ -482,12 +484,14 @@ impl<'l, Data> EventLoop<'l, Data> { PostAction::Continue => {} } - if !self + if self .handle .inner .sources .borrow() - .contains(registroken_token) + .get(registroken_token) + .unwrap_or(&None) + .is_none() { // the source has been removed from within its callback, unregister it let mut poll = self.handle.inner.poll.borrow_mut(); @@ -514,6 +518,13 @@ impl<'l, Data> EventLoop<'l, Data> { } } + // cleanup zombies + self.handle + .inner + .sources + .borrow_mut() + .retain(|_, opt_disp| opt_disp.is_some()); + Ok(()) } @@ -1457,6 +1468,44 @@ mod tests { assert_eq!(data, 22); } + #[test] + fn reuse() { + use crate::sources::timer; + use std::sync::{Arc, Mutex}; + use std::time::{Duration, Instant}; + + let mut evl = EventLoop::::try_new().unwrap(); + let handle = evl.handle(); + + let data = Arc::new(Mutex::new(1)); + let data_cloned = data.clone(); + + let timer_source = timer::Timer::from_duration(Duration::from_secs(1)); + let mut first_timer_token = evl + .handle() + .insert_source(timer_source, move |_, _, own_token| { + handle.remove(*own_token); + let data_cloned = data_cloned.clone(); + let _ = handle.insert_source(timer::Timer::immediate(), move |_, _, _| { + *data_cloned.lock().unwrap() = 2; + timer::TimeoutAction::Drop + }); + timer::TimeoutAction::Drop + }) + .unwrap(); + + let now = Instant::now(); + loop { + evl.dispatch(Some(Duration::from_secs(3)), &mut first_timer_token) + .unwrap(); + if Instant::now().duration_since(now) > Duration::from_secs(3) { + break; + } + } + + assert_eq!(*data.lock().unwrap(), 2); + } + #[test] fn drop_of_subsource() { struct WithSubSource {