From a81621863790ce42cff243b215a01c229cef9142 Mon Sep 17 00:00:00 2001 From: Neale Swinnerton Date: Tue, 3 Mar 2026 16:02:04 +0000 Subject: [PATCH] fix(mpris): disconnect GLib signals before destroying objects Waybar SEGVs in Glib::DispatchNotifier::pipe_io_handler when the MPRIS module is enabled. The crash is intermittent because it requires a race between signal emission and object destruction: a playerctl GLib signal callback (e.g. onPlayerPlay) calls dp.emit(), which writes a pointer to the Dispatcher into an internal pipe. If the Mpris object is destroyed before the GLib main loop reads that pipe entry, pipe_io_handler dereferences a dangling pointer. This typically occurs when a media player appears or vanishes on D-Bus (browser closing, player quitting) or during waybar shutdown/config reload. The root cause is that ~Mpris() calls g_object_unref() on the manager and player GObjects without first disconnecting the signal handlers that hold raw `this` pointers. If playerctl holds additional references to these GObjects, they survive the unref and can still fire signals targeting the already-destroyed Mpris instance. Adopt the same cleanup pattern used by the Wireplumber module: call g_signal_handlers_disconnect_by_data() to sever all signal connections referencing `this` before releasing the GObjects with g_clear_object(). This guarantees no callbacks can enqueue stale Dispatcher notifications after teardown begins. Additionally: - Clean up old player in onPlayerNameAppeared before replacing it, fixing a GObject leak and accumulation of dangling signal connections - Remove duplicate onPlayerStop signal registration (copy-paste bug) --- src/modules/mpris/mpris.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/modules/mpris/mpris.cpp b/src/modules/mpris/mpris.cpp index 1bdd7df6..5771a5ba 100644 --- a/src/modules/mpris/mpris.cpp +++ b/src/modules/mpris/mpris.cpp @@ -161,8 +161,7 @@ Mpris::Mpris(const std::string& id, const Json::Value& config) if (player) { g_object_connect(player, "signal::play", G_CALLBACK(onPlayerPlay), this, "signal::pause", G_CALLBACK(onPlayerPause), this, "signal::stop", G_CALLBACK(onPlayerStop), - this, "signal::stop", G_CALLBACK(onPlayerStop), this, "signal::metadata", - G_CALLBACK(onPlayerMetadata), this, NULL); + this, "signal::metadata", G_CALLBACK(onPlayerMetadata), this, NULL); } // allow setting an interval count that triggers periodic refreshes @@ -178,9 +177,17 @@ Mpris::Mpris(const std::string& id, const Json::Value& config) } Mpris::~Mpris() { - if (last_active_player_ && last_active_player_ != player) g_object_unref(last_active_player_); - if (manager != nullptr) g_object_unref(manager); - if (player != nullptr) g_object_unref(player); + if (manager != nullptr) { + g_signal_handlers_disconnect_by_data(manager, this); + } + if (player != nullptr) { + g_signal_handlers_disconnect_by_data(player, this); + } + if (last_active_player_ != nullptr && last_active_player_ != player) { + g_object_unref(last_active_player_); + } + g_clear_object(&manager); + g_clear_object(&player); } auto Mpris::getIconFromJson(const Json::Value& icons, const std::string& key) -> std::string { @@ -411,11 +418,14 @@ auto Mpris::onPlayerNameAppeared(PlayerctlPlayerManager* manager, PlayerctlPlaye return; } + if (mpris->player != nullptr) { + g_signal_handlers_disconnect_by_data(mpris->player, mpris); + g_clear_object(&mpris->player); + } mpris->player = playerctl_player_new_from_name(player_name, nullptr); g_object_connect(mpris->player, "signal::play", G_CALLBACK(onPlayerPlay), mpris, "signal::pause", G_CALLBACK(onPlayerPause), mpris, "signal::stop", G_CALLBACK(onPlayerStop), - mpris, "signal::stop", G_CALLBACK(onPlayerStop), mpris, "signal::metadata", - G_CALLBACK(onPlayerMetadata), mpris, NULL); + mpris, "signal::metadata", G_CALLBACK(onPlayerMetadata), mpris, NULL); mpris->dp.emit(); }