Skip to content

Commit

Permalink
sources.generic: don't unregister non-registered sources on drop
Browse files Browse the repository at this point in the history
  • Loading branch information
elinorbgr committed Sep 25, 2023
1 parent 92b42b1 commit 531aa78
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 4 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

#### Bugfixes

- Fix an issue where the `Generic` event source would try to unregister its contents from the event loop
after a failed registration.

## 0.12.1 -- 2023-09-19

#### Bugfixes
Expand Down
41 changes: 37 additions & 4 deletions src/sources/generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,7 @@ where
fn register(&mut self, poll: &mut Poll, token_factory: &mut TokenFactory) -> crate::Result<()> {
let token = token_factory.token();

// Make sure we can use the poller to deregister if need be.
self.poller = Some(poll.poller().clone());

// SAFETY: We've now ensured that we have a poller to deregister with.
// SAFETY: We ensure that we have a poller to deregister with (see below).
unsafe {
poll.register(
&self.file.as_ref().unwrap().0,
Expand All @@ -274,7 +271,13 @@ where
)?;
}

// Make sure we can use the poller to deregister if need be.
// But only if registration actually succeeded
// So that we don't try to unregister the FD on drop if it wasn't registered
// in the first place (for example if registration failed because of a duplicate insertion)
self.poller = Some(poll.poller().clone());
self.token = Some(token);

Ok(())
}

Expand Down Expand Up @@ -422,4 +425,34 @@ mod tests {
// the has now been properly dispatched
assert!(dispached);
}

#[test]
fn duplicate_insert() {
use std::os::unix::net::UnixStream;
let event_loop = crate::EventLoop::<()>::try_new().unwrap();

let handle = event_loop.handle();

let (_, rx) = UnixStream::pair().unwrap();

let rx = std::rc::Rc::new(rx);

let token = handle

Check failure on line 440 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied
.insert_source(

Check failure on line 441 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied
Generic::new(rx.clone(), Interest::READ, Mode::Level),

Check failure on line 442 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied
|_, _, _| Ok(PostAction::Continue),
)
.unwrap();

// inserting the same FD a second time should fail
let ret = handle.insert_source(
Generic::new(rx.clone(), Interest::READ, Mode::Level),

Check failure on line 449 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied

Check failure on line 449 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied
|_, _, _| Ok(PostAction::Continue),
);
assert!(ret.is_err());

Check failure on line 452 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied
std::mem::drop(ret);

Check failure on line 453 in src/sources/generic.rs

View workflow job for this annotation

GitHub Actions / CI (1.63.0)

the trait bound `Rc<UnixStream>: AsFd` is not satisfied

// but the original token is still registered
handle.update(&token).unwrap();
}
}

0 comments on commit 531aa78

Please sign in to comment.