Skip to content

Commit ece243d

Browse files
authored
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
1 parent bdf7072 commit ece243d

File tree

5 files changed

+65
-31
lines changed

5 files changed

+65
-31
lines changed

Diff for: .changes/event-once.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"tauri": patch
3+
---
4+
5+
Prevent "once" events from being able to be called multiple times.
6+
* `Window::trigger(/*...*/)` is now properly `pub` instead of `pub(crate)`.
7+
* `Manager::once_global(/*...*/)` now returns an `EventHandler`.
8+
* `Window::once(/*...*/)` now returns an `EventHandler`.
9+
* (internal) `event::Listeners::trigger(/*...*/)` now handles removing "once" events.

Diff for: core/tauri/src/event.rs

+52-27
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,20 @@ impl Event {
4141
}
4242
}
4343

44+
/// What happens after the handler is called?
45+
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
46+
enum AfterHandle {
47+
/// The handler is removed (once).
48+
Remove,
49+
50+
/// Nothing is done (regular).
51+
DoNothing,
52+
}
53+
4454
/// Stored in [`Listeners`] to be called upon when the event that stored it is triggered.
4555
struct Handler<Window: Tag> {
4656
window: Option<Window>,
47-
callback: Box<dyn Fn(Event) + Send>,
57+
callback: Box<dyn Fn(Event) -> AfterHandle + Send>,
4858
}
4959

5060
/// A collection of handlers. Multiple handlers can represent the same event.
@@ -85,44 +95,51 @@ impl<E: Tag, L: Tag> Listeners<E, L> {
8595
self.queue_object_name.to_string()
8696
}
8797

88-
/// Adds an event listener for JS events.
89-
pub(crate) fn listen<F: Fn(Event) + Send + 'static>(
90-
&self,
91-
event: E,
92-
window: Option<L>,
93-
handler: F,
94-
) -> EventHandler {
98+
fn listen_internal<F>(&self, event: E, window: Option<L>, handler: F) -> EventHandler
99+
where
100+
F: Fn(Event) -> AfterHandle + Send + 'static,
101+
{
95102
let id = EventHandler(Uuid::new_v4());
96-
let handler = Handler {
97-
window,
98-
callback: Box::new(handler),
99-
};
103+
100104
self
101105
.inner
102106
.lock()
103107
.expect("poisoned event mutex")
104108
.entry(event)
105109
.or_default()
106-
.insert(id, handler);
110+
.insert(
111+
id,
112+
Handler {
113+
window,
114+
callback: Box::new(handler),
115+
},
116+
);
117+
107118
id
108119
}
109120

121+
/// Adds an event listener for JS events.
122+
pub(crate) fn listen<F>(&self, event: E, window: Option<L>, handler: F) -> EventHandler
123+
where
124+
F: Fn(Event) + Send + 'static,
125+
{
126+
self.listen_internal(event, window, move |event| {
127+
handler(event);
128+
AfterHandle::DoNothing
129+
})
130+
}
131+
110132
/// Listen to a JS event and immediately unlisten.
111133
pub(crate) fn once<F: Fn(Event) + Send + 'static>(
112134
&self,
113135
event: E,
114136
window: Option<L>,
115137
handler: F,
116-
) {
117-
let self_ = self.clone();
118-
self.listen(event, window, move |e| {
119-
let self_ = self_.clone();
120-
let id = e.id;
121-
crate::async_runtime::spawn(async move {
122-
self_.unlisten(id);
123-
});
124-
handler(e);
125-
});
138+
) -> EventHandler {
139+
self.listen_internal(event, window, move |event| {
140+
handler(event);
141+
AfterHandle::Remove
142+
})
126143
}
127144

128145
/// Removes an event listener.
@@ -139,14 +156,22 @@ impl<E: Tag, L: Tag> Listeners<E, L> {
139156

140157
/// Triggers the given global event with its payload.
141158
pub(crate) fn trigger(&self, event: E, window: Option<L>, data: Option<String>) {
142-
if let Some(handlers) = self.inner.lock().expect("poisoned event mutex").get(&event) {
143-
for (&id, handler) in handlers {
159+
if let Some(handlers) = self
160+
.inner
161+
.lock()
162+
.expect("poisoned event mutex")
163+
.get_mut(&event)
164+
{
165+
handlers.retain(|&id, handler| {
144166
if window.is_none() || window == handler.window {
145167
let data = data.clone();
146168
let payload = Event { id, data };
147-
(handler.callback)(payload)
169+
(handler.callback)(payload) != AfterHandle::Remove
170+
} else {
171+
// skip and retain all handlers specifying a different window
172+
true
148173
}
149-
}
174+
})
150175
}
151176
}
152177
}

Diff for: core/tauri/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ pub trait Manager<M: Params>: sealed::ManagerBase<M> {
173173
}
174174

175175
/// Listen to a global event only once.
176-
fn once_global<F>(&self, event: M::Event, handler: F)
176+
fn once_global<F>(&self, event: M::Event, handler: F) -> EventHandler
177177
where
178178
F: Fn(Event) + Send + 'static,
179179
{

Diff for: core/tauri/src/runtime/manager.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ impl<P: Params> WindowManager<P> {
516516
event: P::Event,
517517
window: Option<P::Label>,
518518
handler: F,
519-
) {
519+
) -> EventHandler {
520520
self.inner.listeners.once(event, window, handler)
521521
}
522522
pub fn event_listeners_object_name(&self) -> String {

Diff for: core/tauri/src/runtime/window.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ pub(crate) mod export {
256256
}
257257

258258
/// Listen to a an event on this window a single time.
259-
pub fn once<F>(&self, event: P::Event, handler: F)
259+
pub fn once<F>(&self, event: P::Event, handler: F) -> EventHandler
260260
where
261261
F: Fn(Event) + Send + 'static,
262262
{
@@ -265,7 +265,7 @@ pub(crate) mod export {
265265
}
266266

267267
/// Triggers an event on this window.
268-
pub(crate) fn trigger(&self, event: P::Event, data: Option<String>) {
268+
pub fn trigger(&self, event: P::Event, data: Option<String>) {
269269
let label = self.window.label.clone();
270270
self.manager.trigger(event, Some(label), data)
271271
}

0 commit comments

Comments
 (0)