don't remove once listener on new thread (#1506)

Spawning an async task to remove the once listener caused it to be able to be
called multiple times before being removed. This design choice was previously
made due to deadlock happening when removing the event from inside `fn once`.
That was because the listeners were already locked inside the trigger when
asked to be removed. `fn trigger` now handles removing once handlers
This commit is contained in:
chip 2021-04-14 23:11:42 -07:00 committed by GitHub
parent bdf707285e
commit ece243d17c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 65 additions and 31 deletions

9
.changes/event-once.md Normal file
View File

@ -0,0 +1,9 @@
---
"tauri": patch
---
Prevent "once" events from being able to be called multiple times.
* `Window::trigger(/*...*/)` is now properly `pub` instead of `pub(crate)`.
* `Manager::once_global(/*...*/)` now returns an `EventHandler`.
* `Window::once(/*...*/)` now returns an `EventHandler`.
* (internal) `event::Listeners::trigger(/*...*/)` now handles removing "once" events.

View File

@ -41,10 +41,20 @@ impl Event {
}
}
/// What happens after the handler is called?
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
enum AfterHandle {
/// The handler is removed (once).
Remove,
/// Nothing is done (regular).
DoNothing,
}
/// Stored in [`Listeners`] to be called upon when the event that stored it is triggered.
struct Handler<Window: Tag> {
window: Option<Window>,
callback: Box<dyn Fn(Event) + Send>,
callback: Box<dyn Fn(Event) -> AfterHandle + Send>,
}
/// A collection of handlers. Multiple handlers can represent the same event.
@ -85,44 +95,51 @@ impl<E: Tag, L: Tag> Listeners<E, L> {
self.queue_object_name.to_string()
}
/// Adds an event listener for JS events.
pub(crate) fn listen<F: Fn(Event) + Send + 'static>(
&self,
event: E,
window: Option<L>,
handler: F,
) -> EventHandler {
fn listen_internal<F>(&self, event: E, window: Option<L>, handler: F) -> EventHandler
where
F: Fn(Event) -> AfterHandle + Send + 'static,
{
let id = EventHandler(Uuid::new_v4());
let handler = Handler {
window,
callback: Box::new(handler),
};
self
.inner
.lock()
.expect("poisoned event mutex")
.entry(event)
.or_default()
.insert(id, handler);
.insert(
id,
Handler {
window,
callback: Box::new(handler),
},
);
id
}
/// Adds an event listener for JS events.
pub(crate) fn listen<F>(&self, event: E, window: Option<L>, handler: F) -> EventHandler
where
F: Fn(Event) + Send + 'static,
{
self.listen_internal(event, window, move |event| {
handler(event);
AfterHandle::DoNothing
})
}
/// Listen to a JS event and immediately unlisten.
pub(crate) fn once<F: Fn(Event) + Send + 'static>(
&self,
event: E,
window: Option<L>,
handler: F,
) {
let self_ = self.clone();
self.listen(event, window, move |e| {
let self_ = self_.clone();
let id = e.id;
crate::async_runtime::spawn(async move {
self_.unlisten(id);
});
handler(e);
});
) -> EventHandler {
self.listen_internal(event, window, move |event| {
handler(event);
AfterHandle::Remove
})
}
/// Removes an event listener.
@ -139,14 +156,22 @@ impl<E: Tag, L: Tag> Listeners<E, L> {
/// Triggers the given global event with its payload.
pub(crate) fn trigger(&self, event: E, window: Option<L>, data: Option<String>) {
if let Some(handlers) = self.inner.lock().expect("poisoned event mutex").get(&event) {
for (&id, handler) in handlers {
if let Some(handlers) = self
.inner
.lock()
.expect("poisoned event mutex")
.get_mut(&event)
{
handlers.retain(|&id, handler| {
if window.is_none() || window == handler.window {
let data = data.clone();
let payload = Event { id, data };
(handler.callback)(payload)
(handler.callback)(payload) != AfterHandle::Remove
} else {
// skip and retain all handlers specifying a different window
true
}
}
})
}
}
}

View File

@ -173,7 +173,7 @@ pub trait Manager<M: Params>: sealed::ManagerBase<M> {
}
/// Listen to a global event only once.
fn once_global<F>(&self, event: M::Event, handler: F)
fn once_global<F>(&self, event: M::Event, handler: F) -> EventHandler
where
F: Fn(Event) + Send + 'static,
{

View File

@ -516,7 +516,7 @@ impl<P: Params> WindowManager<P> {
event: P::Event,
window: Option<P::Label>,
handler: F,
) {
) -> EventHandler {
self.inner.listeners.once(event, window, handler)
}
pub fn event_listeners_object_name(&self) -> String {

View File

@ -256,7 +256,7 @@ pub(crate) mod export {
}
/// Listen to a an event on this window a single time.
pub fn once<F>(&self, event: P::Event, handler: F)
pub fn once<F>(&self, event: P::Event, handler: F) -> EventHandler
where
F: Fn(Event) + Send + 'static,
{
@ -265,7 +265,7 @@ pub(crate) mod export {
}
/// Triggers an event on this window.
pub(crate) fn trigger(&self, event: P::Event, data: Option<String>) {
pub fn trigger(&self, event: P::Event, data: Option<String>) {
let label = self.window.label.clone();
self.manager.trigger(event, Some(label), data)
}