Zander671/Waybar
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mpris): disconnect GLib signals before destroying objects

Browse Source 
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)
This commit is contained in:
Neale Swinnerton
2026-03-03 16:02:04 +00:00
parent fd086d0f33
commit a816218637
1 changed files with 17 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
src/modules/mpris/mpris.cpp
View File

@@ -161,8 +161,7 @@ Mpris::Mpris(const std::string& id, const Json::Value& config)
if (player) { if (player) {
g_object_connect(player, "signal::play", G_CALLBACK(onPlayerPlay), this, "signal::pause", g_object_connect(player, "signal::play", G_CALLBACK(onPlayerPlay), this, "signal::pause",
G_CALLBACK(onPlayerPause), this, "signal::stop", G_CALLBACK(onPlayerStop), G_CALLBACK(onPlayerPause), this, "signal::stop", G_CALLBACK(onPlayerStop),
this, "signal::stop", G_CALLBACK(onPlayerStop), this, "signal::metadata", this, "signal::metadata", G_CALLBACK(onPlayerMetadata), this, NULL);
G_CALLBACK(onPlayerMetadata), this, NULL);
} }
// allow setting an interval count that triggers periodic refreshes // 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() { Mpris::~Mpris() {
if (last_active_player_ && last_active_player_ != player) g_object_unref(last_active_player_); if (manager != nullptr) {
if (manager != nullptr) g_object_unref(manager); g_signal_handlers_disconnect_by_data(manager, this);
if (player != nullptr) g_object_unref(player); }
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 { auto Mpris::getIconFromJson(const Json::Value& icons, const std::string& key) -> std::string {
@@ -411,11 +418,14 @@ auto Mpris::onPlayerNameAppeared(PlayerctlPlayerManager* manager, PlayerctlPlaye
return; 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); mpris->player = playerctl_player_new_from_name(player_name, nullptr);
g_object_connect(mpris->player, "signal::play", G_CALLBACK(onPlayerPlay), mpris, "signal::pause", g_object_connect(mpris->player, "signal::play", G_CALLBACK(onPlayerPlay), mpris, "signal::pause",
G_CALLBACK(onPlayerPause), mpris, "signal::stop", G_CALLBACK(onPlayerStop), G_CALLBACK(onPlayerPause), mpris, "signal::stop", G_CALLBACK(onPlayerStop),
mpris, "signal::stop", G_CALLBACK(onPlayerStop), mpris, "signal::metadata", mpris, "signal::metadata", G_CALLBACK(onPlayerMetadata), mpris, NULL);
G_CALLBACK(onPlayerMetadata), mpris, NULL);
mpris->dp.emit(); mpris->dp.emit();
} }