Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when dropping removed token #155

Closed
wants to merge 3 commits into from
Closed

Fix crash when dropping removed token #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1108897
Fix crash when dropping removed token
chrisduerr Oct 3, 2023
631ac85
Add test
chrisduerr Oct 4, 2023
d4e9304
Add changelog entry
chrisduerr Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
186 changes: 105 additions & 81 deletions src/loop_logic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use slab::Slab;

use log::trace;
use log::{debug, trace, warn};

use crate::sources::{Dispatcher, EventSource, Idle, IdleDispatcher};
use crate::sys::{Notifier, PollEvent};
Expand Down Expand Up @@ -260,7 +260,7 @@
.borrow_mut(),
token,
) {
log::warn!(
warn!(

Check warning on line 263 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L263 

Added line #L263 was not covered by tests
"[calloop] Failed to unregister source from the polling system: {:?}",
e
);
Expand Down Expand Up @@ -419,98 +419,105 @@
.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: {:?}",

Check warning on line 426 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L425-L426 

Added lines #L425 - L426 were not covered by tests
registroken_token
);
continue;

Check warning on line 429 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L429 

Added line #L429 was not covered by tests
}
};

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 #{}",

Check warning on line 434 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L434 

Added line #L434 was not covered by tests
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 #{}",

Check warning on line 452 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L452 

Added line #L452 was not covered by tests
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 #{}",

Check warning on line 467 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L466-L467 

Added lines #L466 - L467 were not covered by tests
registroken_token
);
disp.unregister(
&mut self.handle.inner.poll.borrow_mut(),

Check warning on line 471 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L470-L471 

Added lines #L470 - L471 were not covered by tests
&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
)?;

Check warning on line 478 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L478 

Added line #L478 was not covered by tests
}
PostAction::Remove => {
trace!(
"[calloop] Postaction remove for source #{}",

Check warning on line 482 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L482 

Added line #L482 was not covered by tests
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",

Check warning on line 490 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L490 

Added line #L490 was not covered by tests
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: {:?}",

Check warning on line 517 in src/loop_logic.rs

View check run for this annotation

Codecov / codecov/patch

src/loop_logic.rs#L516-L517 

Added lines #L516 - L517 were not covered by tests
e
);
}
}
}

Expand Down Expand Up @@ -1097,6 +1104,23 @@
assert!(ret.is_err());
}

#[test]
fn remove_during_callback() {
use crate::sources::timer::{TimeoutAction, Timer};

let mut event_loop = EventLoop::<RegistrationToken>::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;
Expand Down
Loading