bugfix: Remove listener if one already exists
This commit makes it so EventListener::listen() does not overwrite existing listeners if they already exist. If the listener is already registered in an event listener list, it removes the listener from that list before registering the new listener. This soundness bug was missed during #94. This is a patch-compatible fix for #100. We may also want an API-level fix for #100 as well. This is up for further discussion. Signed-off-by: John Nunley <dev@notgull.net>
This commit is contained in:
parent
cc33cc5467
commit
c2d1ccb7ae
13
src/lib.rs
13
src/lib.rs
|
@ -810,13 +810,24 @@ impl<T> EventListener<T> {
|
|||
///
|
||||
/// This method can only be called after the listener has been pinned, and must be called before
|
||||
/// the listener is polled.
|
||||
///
|
||||
/// Notifications that exist when this function is called will be discarded.
|
||||
pub fn listen(mut self: Pin<&mut Self>, event: &Event<T>) {
|
||||
let inner = {
|
||||
let inner = event.inner();
|
||||
unsafe { Arc::clone(&ManuallyDrop::new(Arc::from_raw(inner))) }
|
||||
};
|
||||
|
||||
let ListenerProject { event, listener } = self.as_mut().project().listener.project();
|
||||
let ListenerProject {
|
||||
event,
|
||||
mut listener,
|
||||
} = self.as_mut().project().listener.project();
|
||||
|
||||
// If an event is already registered, make sure to remove it.
|
||||
if let Some(current_event) = event.as_ref() {
|
||||
current_event.remove(listener.as_mut(), false);
|
||||
}
|
||||
|
||||
let inner = event.insert(inner);
|
||||
inner.insert(listener);
|
||||
|
||||
|
|
|
@ -189,3 +189,16 @@ fn notify_all_fair() {
|
|||
.poll(&mut Context::from_waker(&waker3))
|
||||
.is_ready());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn more_than_one_event() {
|
||||
let event = Event::new();
|
||||
let event2 = Event::new();
|
||||
|
||||
let mut listener = Box::pin(EventListener::<()>::new());
|
||||
listener.as_mut().listen(&event);
|
||||
listener.as_mut().listen(&event2);
|
||||
|
||||
drop(listener);
|
||||
event.notify(1);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue