Skip to content

Commit 7038141

Browse files
moimartmoimart
committed
fix(system_tray): dispatch tray updates asynchronously to avoid blocking callers (#4199)
update_tray_*() functions call tray_update(), which on Linux blocks the caller inside pthread_cond_wait until the tray library's GTK main-loop callback runs. That callback calls libayatana-appindicator and libnotify synchronously. If the desktop notification daemon is unresponsive — a common failure mode on Wayland compositors (swaync/dunst/Quickshell) during desktop transitions, lock screens, or daemon restarts — the callback blocks indefinitely and every caller of update_tray_* blocks with it. One of those callers is stream::session::join(), which arms a 10-second NVENC-deadlock watchdog. When the tray update doesn't return in time, the watchdog fires debug_trap() and the entire sunshine process dies with SIGTRAP. Reported as #4199. Fix: spawn a detached worker thread in each update_tray_* entry point. The worker holds a tray_async_mutex (to serialize mutation of the shared tray struct) and then runs the original update body. The caller returns immediately. If the notification daemon is hung, the worker stays blocked until the daemon recovers or the process exits, but the session teardown — and all other callers of update_tray_* — complete promptly. Also replaces the 'static std::string msg = std::format(...)' pattern with file-scope buffers (playing_msg_buf etc.) that are reassigned on each update; the prior static-local-initialization idiom captured only the first app_name on first call and then reused it forever, a latent bug unrelated to the SIGTRAP but worth fixing while we're here. Fixes #4199
1 parent 5cf5e8c commit 7038141

1 file changed

Lines changed: 154 additions & 73 deletions

File tree

‎src/system_tray.cpp‎

Lines changed: 154 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@
3333
#include <chrono>
3434
#include <csignal>
3535
#include <format>
36+
#include <functional>
37+
#include <mutex>
3638
#include <string>
3739
#include <thread>
40+
#include <utility>
3841

3942
// lib includes
4043
#include <boost/filesystem.hpp>
@@ -55,6 +58,88 @@ using namespace std::literals;
5558
namespace system_tray {
5659
static std::atomic tray_initialized = false;
5760

61+
namespace detail {
62+
// Holds the shared state used by the async tray update workers. Packaged
63+
// in a Meyers-singleton accessor below so the mutex and the persistent
64+
// string buffers are function-local statics rather than file-scope
65+
// globals.
66+
struct tray_async_state_t {
67+
// Serializes mutation of the `tray` struct across the detached workers
68+
// spawned by update_tray_*(). tray_update() itself is internally
69+
// serialized on Linux/macOS via the tray library's own main-loop
70+
// dispatch, but we can still race on the fields of `tray` without this.
71+
std::mutex mutex;
72+
73+
// Persistent string storage for notification text. tray_update() on
74+
// Linux stores pointers into these strings and needs them to stay
75+
// valid until the next update, so they cannot be local to the worker.
76+
// Accessed only while holding `mutex`.
77+
std::string playing_msg;
78+
std::string pausing_msg;
79+
std::string stopped_msg;
80+
};
81+
82+
static tray_async_state_t &tray_async_state() {
83+
static tray_async_state_t state;
84+
return state;
85+
}
86+
87+
// Runs `fn` under the tray-async lock with exception containment. Pulled
88+
// out of the worker lambda so the lambda body is a single try/catch.
89+
static void run_tray_update_locked(const std::function<void()> &fn) {
90+
auto &state = tray_async_state();
91+
std::lock_guard lk(state.mutex);
92+
if (!tray_initialized) {
93+
return;
94+
}
95+
fn();
96+
}
97+
} // namespace detail
98+
99+
// Spawn a detached worker that performs a tray update.
100+
//
101+
// Rationale: on Linux, tray_update() synchronously waits for the GTK
102+
// main loop (i.e. the tray thread) to run the update callback — which
103+
// in turn calls libnotify and libayatana-appindicator. If the active
104+
// notification daemon is unresponsive (a common failure mode on
105+
// Wayland compositors during desktop transitions or when the daemon
106+
// crashes), the callback blocks indefinitely and the caller blocks
107+
// with it.
108+
//
109+
// The caller is frequently stream::session::join(), which arms a
110+
// 10-second NVENC-deadlock watchdog that triggers debug_trap() on
111+
// timeout. A hung notification daemon therefore ends up terminating
112+
// the entire sunshine process with SIGTRAP. See #4199.
113+
//
114+
// Running the update on a detached thread decouples the caller from
115+
// the tray subsystem: session teardown completes promptly, while
116+
// the worker stays blocked on tray_update() until the notification
117+
// daemon eventually responds (or the process exits).
118+
static void run_tray_async(std::function<void()> fn) {
119+
if (!tray_initialized) {
120+
return;
121+
}
122+
try {
123+
// std::jthread is used for its RAII guarantees; the thread is
124+
// immediately detached because tray_update() is allowed to block
125+
// indefinitely on a hung notification daemon and must not delay
126+
// process shutdown or the caller's critical path.
127+
std::jthread worker([fn = std::move(fn)]() mutable {
128+
try {
129+
detail::run_tray_update_locked(fn);
130+
} catch (const std::exception &e) {
131+
BOOST_LOG(warning) << "Tray update threw: "sv << e.what();
132+
}
133+
});
134+
worker.detach();
135+
} catch (const std::system_error &e) {
136+
// std::jthread construction can fail with std::system_error if the
137+
// OS is out of resources; in that case we drop the update rather
138+
// than surface the failure to the caller's critical path.
139+
BOOST_LOG(warning) << "Failed to spawn tray update thread: "sv << e.what();
140+
}
141+
}
142+
58143
void tray_open_ui_cb([[maybe_unused]] struct tray_menu *item) {
59144
BOOST_LOG(info) << "Opening UI from system tray"sv;
60145
launch_ui();
@@ -283,88 +368,84 @@ namespace system_tray {
283368
}
284369

285370
void update_tray_playing(std::string app_name) {
286-
if (!tray_initialized) {
287-
return;
288-
}
289-
290-
tray.notification_title = nullptr;
291-
tray.notification_text = nullptr;
292-
tray.notification_cb = nullptr;
293-
tray.notification_icon = nullptr;
294-
tray.icon = TRAY_ICON_PLAYING;
295-
tray_update(&tray);
296-
tray.icon = TRAY_ICON_PLAYING;
297-
tray.notification_title = "Stream Started";
298-
299-
static std::string msg = std::format("Streaming started for {}", app_name);
300-
tray.notification_text = msg.c_str();
301-
tray.tooltip = msg.c_str();
302-
tray.notification_icon = TRAY_ICON_PLAYING;
303-
tray_update(&tray);
371+
run_tray_async([name = std::move(app_name)]() {
372+
auto &state = detail::tray_async_state();
373+
tray.notification_title = nullptr;
374+
tray.notification_text = nullptr;
375+
tray.notification_cb = nullptr;
376+
tray.notification_icon = nullptr;
377+
tray.icon = TRAY_ICON_PLAYING;
378+
tray_update(&tray);
379+
380+
state.playing_msg = std::format("Streaming started for {}", name);
381+
tray.icon = TRAY_ICON_PLAYING;
382+
tray.notification_title = "Stream Started";
383+
tray.notification_text = state.playing_msg.c_str();
384+
tray.tooltip = state.playing_msg.c_str();
385+
tray.notification_icon = TRAY_ICON_PLAYING;
386+
tray_update(&tray);
387+
});
304388
}
305389

306390
void update_tray_pausing(std::string app_name) {
307-
if (!tray_initialized) {
308-
return;
309-
}
310-
311-
tray.notification_title = nullptr;
312-
tray.notification_text = nullptr;
313-
tray.notification_cb = nullptr;
314-
tray.notification_icon = nullptr;
315-
tray.icon = TRAY_ICON_PAUSING;
316-
tray_update(&tray);
317-
318-
static std::string msg = std::format("Streaming paused for {}", app_name);
319-
tray.icon = TRAY_ICON_PAUSING;
320-
tray.notification_title = "Stream Paused";
321-
tray.notification_text = msg.c_str();
322-
tray.tooltip = msg.c_str();
323-
tray.notification_icon = TRAY_ICON_PAUSING;
324-
tray_update(&tray);
391+
run_tray_async([name = std::move(app_name)]() {
392+
auto &state = detail::tray_async_state();
393+
tray.notification_title = nullptr;
394+
tray.notification_text = nullptr;
395+
tray.notification_cb = nullptr;
396+
tray.notification_icon = nullptr;
397+
tray.icon = TRAY_ICON_PAUSING;
398+
tray_update(&tray);
399+
400+
state.pausing_msg = std::format("Streaming paused for {}", name);
401+
tray.icon = TRAY_ICON_PAUSING;
402+
tray.notification_title = "Stream Paused";
403+
tray.notification_text = state.pausing_msg.c_str();
404+
tray.tooltip = state.pausing_msg.c_str();
405+
tray.notification_icon = TRAY_ICON_PAUSING;
406+
tray_update(&tray);
407+
});
325408
}
326409

327410
void update_tray_stopped(std::string app_name) {
328-
if (!tray_initialized) {
329-
return;
330-
}
331-
332-
tray.notification_title = nullptr;
333-
tray.notification_text = nullptr;
334-
tray.notification_cb = nullptr;
335-
tray.notification_icon = nullptr;
336-
tray.icon = TRAY_ICON;
337-
tray_update(&tray);
338-
339-
static std::string msg = std::format("Application {} successfully stopped", app_name);
340-
tray.icon = TRAY_ICON;
341-
tray.notification_icon = TRAY_ICON;
342-
tray.notification_title = "Application Stopped";
343-
tray.notification_text = msg.c_str();
344-
tray.tooltip = PROJECT_NAME;
345-
tray_update(&tray);
411+
run_tray_async([name = std::move(app_name)]() {
412+
auto &state = detail::tray_async_state();
413+
tray.notification_title = nullptr;
414+
tray.notification_text = nullptr;
415+
tray.notification_cb = nullptr;
416+
tray.notification_icon = nullptr;
417+
tray.icon = TRAY_ICON;
418+
tray_update(&tray);
419+
420+
state.stopped_msg = std::format("Application {} successfully stopped", name);
421+
tray.icon = TRAY_ICON;
422+
tray.notification_icon = TRAY_ICON;
423+
tray.notification_title = "Application Stopped";
424+
tray.notification_text = state.stopped_msg.c_str();
425+
tray.tooltip = PROJECT_NAME;
426+
tray_update(&tray);
427+
});
346428
}
347429

348430
void update_tray_require_pin() {
349-
if (!tray_initialized) {
350-
return;
351-
}
352-
353-
tray.notification_title = nullptr;
354-
tray.notification_text = nullptr;
355-
tray.notification_cb = nullptr;
356-
tray.notification_icon = nullptr;
357-
tray.icon = TRAY_ICON;
358-
tray_update(&tray);
359-
tray.icon = TRAY_ICON;
360-
tray.notification_title = "Incoming Pairing Request";
361-
tray.notification_text = "Click here to complete the pairing process";
362-
tray.notification_icon = TRAY_ICON_LOCKED;
363-
tray.tooltip = PROJECT_NAME;
364-
tray.notification_cb = []() {
365-
launch_ui("/pin");
366-
};
367-
tray_update(&tray);
431+
run_tray_async([]() {
432+
tray.notification_title = nullptr;
433+
tray.notification_text = nullptr;
434+
tray.notification_cb = nullptr;
435+
tray.notification_icon = nullptr;
436+
tray.icon = TRAY_ICON;
437+
tray_update(&tray);
438+
439+
tray.icon = TRAY_ICON;
440+
tray.notification_title = "Incoming Pairing Request";
441+
tray.notification_text = "Click here to complete the pairing process";
442+
tray.notification_icon = TRAY_ICON_LOCKED;
443+
tray.tooltip = PROJECT_NAME;
444+
tray.notification_cb = []() {
445+
launch_ui("/pin");
446+
};
447+
tray_update(&tray);
448+
});
368449
}
369450

370451
// Threading functions available on all platforms

0 commit comments

Comments
 (0)