From 1108897a8e7caf25303bad3b410c84039ae2f250 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Tue, 3 Oct 2023 22:07:05 +0200 Subject: [PATCH 1/3] Fix crash when dropping removed token This fixes an issue where calloop would crash internally when the `PostAction::Drop` was invoked after the user manually cancelled the source in the callback. --- src/loop_logic.rs | 169 ++++++++++++++++++++++++---------------------- 1 file changed, 88 insertions(+), 81 deletions(-) diff --git a/src/loop_logic.rs b/src/loop_logic.rs index 22868db0..f3b8a60f 100644 --- a/src/loop_logic.rs +++ b/src/loop_logic.rs @@ -12,7 +12,7 @@ use std::future::Future; use slab::Slab; -use log::trace; +use log::{debug, trace, warn}; use crate::sources::{Dispatcher, EventSource, Idle, IdleDispatcher}; use crate::sys::{Notifier, PollEvent}; @@ -260,7 +260,7 @@ impl<'l, Data> LoopHandle<'l, Data> { .borrow_mut(), token, ) { - log::warn!( + warn!( "[calloop] Failed to unregister source from the polling system: {:?}", e ); @@ -419,98 +419,105 @@ impl<'l, Data> EventLoop<'l, Data> { .get(registroken_token) .cloned(); - if let Some(disp) = opt_disp { - trace!( - "[calloop] Dispatching events for source #{}", - registroken_token - ); - let mut ret = disp.process_events(event.readiness, event.token, data)?; - - // if the returned PostAction is Continue, it may be overwritten by an user-specified pending action - let pending_action = self - .handle - .inner - .pending_action - .replace(PostAction::Continue); - if let PostAction::Continue = ret { - ret = pending_action; + let disp = match opt_disp { + Some(disp) => disp, + None => { + debug!( + "[calloop] Received an event for non-existent source: {:?}", + registroken_token + ); + continue; } + }; - match ret { - PostAction::Reregister => { - trace!( - "[calloop] Postaction reregister for source #{}", - registroken_token - ); - disp.reregister( - &mut self.handle.inner.poll.borrow_mut(), - &mut self - .handle - .inner - .sources_with_additional_lifecycle_events - .borrow_mut(), - &mut TokenFactory::new(registroken_token), - )?; - } - PostAction::Disable => { - trace!( - "[calloop] Postaction unregister for source #{}", - registroken_token - ); - disp.unregister( - &mut self.handle.inner.poll.borrow_mut(), - &mut self - .handle - .inner - .sources_with_additional_lifecycle_events - .borrow_mut(), - RegistrationToken::new(registroken_token), - )?; - } - PostAction::Remove => { - trace!( - "[calloop] Postaction remove for source #{}", - registroken_token - ); - // delete the source from the list, it'll be cleaned up with the if just below - self.handle + trace!( + "[calloop] Dispatching events for source #{}", + registroken_token + ); + let mut ret = disp.process_events(event.readiness, event.token, data)?; + + // if the returned PostAction is Continue, it may be overwritten by an user-specified pending action + let pending_action = self + .handle + .inner + .pending_action + .replace(PostAction::Continue); + if let PostAction::Continue = ret { + ret = pending_action; + } + + match ret { + PostAction::Reregister => { + trace!( + "[calloop] Postaction reregister for source #{}", + registroken_token + ); + disp.reregister( + &mut self.handle.inner.poll.borrow_mut(), + &mut self + .handle .inner - .sources - .borrow_mut() - .remove(registroken_token); - } - PostAction::Continue => {} + .sources_with_additional_lifecycle_events + .borrow_mut(), + &mut TokenFactory::new(registroken_token), + )?; } - - if !self - .handle - .inner - .sources - .borrow() - .contains(registroken_token) - { - // the source has been removed from within its callback, unregister it - let mut poll = self.handle.inner.poll.borrow_mut(); - if let Err(e) = disp.unregister( - &mut poll, + PostAction::Disable => { + trace!( + "[calloop] Postaction unregister for source #{}", + registroken_token + ); + disp.unregister( + &mut self.handle.inner.poll.borrow_mut(), &mut self .handle .inner .sources_with_additional_lifecycle_events .borrow_mut(), RegistrationToken::new(registroken_token), - ) { - log::warn!( - "[calloop] Failed to unregister source from the polling system: {:?}", - e + )?; + } + PostAction::Remove => { + trace!( + "[calloop] Postaction remove for source #{}", + registroken_token + ); + + // Delete the source from the list, unregister is performed separately. + let mut sources = self.handle.inner.sources.borrow_mut(); + if sources.try_remove(registroken_token).is_none() { + debug!( + "[calloop] could not remove event source #{}: token does not exist", + registroken_token ); } } - } else { - log::warn!( - "[calloop] Received an event for non-existence source: {:?}", - registroken_token - ); + PostAction::Continue => {} + } + + if !self + .handle + .inner + .sources + .borrow() + .contains(registroken_token) + { + // the source has been removed from within its callback, unregister it + let mut poll = self.handle.inner.poll.borrow_mut(); + if let Err(e) = disp.unregister( + &mut poll, + &mut self + .handle + .inner + .sources_with_additional_lifecycle_events + .borrow_mut(), + RegistrationToken::new(registroken_token), + ) { + warn!( + "[calloop] Failed to unregister source from the polling system: {:?}", + e + ); + } } } From 631ac8550dd5b2ca325cf202a7adee26eee58de1 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Wed, 4 Oct 2023 02:19:37 +0200 Subject: [PATCH 2/3] Add test --- src/loop_logic.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/loop_logic.rs b/src/loop_logic.rs index f3b8a60f..6d7edc4d 100644 --- a/src/loop_logic.rs +++ b/src/loop_logic.rs @@ -1104,6 +1104,23 @@ mod tests { assert!(ret.is_err()); } + #[test] + fn remove_during_callback() { + use crate::sources::timer::{TimeoutAction, Timer}; + + let mut event_loop = EventLoop::::try_new().unwrap(); + let handle = event_loop.handle(); + let mut token = event_loop + .handle() + .insert_source(Timer::immediate(), move |_, _, token| { + handle.remove(*token); + TimeoutAction::Drop + }) + .unwrap(); + + event_loop.dispatch(Duration::ZERO, &mut token).unwrap(); + } + #[test] fn insert_source_no_interest() { use rustix::pipe::pipe; From d4e930446c0404583b467b770db02b1f1f6e484d Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Wed, 4 Oct 2023 16:50:15 +0200 Subject: [PATCH 3/3] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8efaeb74..80a4302c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +#### Bugfixes + +- Fixed a crash when removing a token in a callback with a `PostAction::Remove` + ## 0.12.2 -- 2023-09-25 #### Bugfixes