fix mpd module hang/crash after suspend #3793
Draft
+37
−13
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been having this annoying issue for a while, in which after waking up from a long suspend the MPD module is not updating anymore.
What I think is happening is that once the client goes to sleep the server TCP connection to the client expires (based on the values in
/proc/sys/net/ipv4/tcp_keepalive_*
), the server sends aRST
but the client never receives it because it is asleep.How to reproduce
This can be reproduced by killing the socket(s) on the server using
ss
like soWhen the client wakes, the MPD module will not update anymore.
Second bug
While debugging this, I realized that killing the socket on the client (or just having the connection naturally expire) actually causes waybar to go on an infinite loop and eventually crash:
Results in waybar realizing that the idle connection has died, but reconnection fails, clicking on the mpd icon in waybar causes the crash (last line):
Third bug (somehow unrelated?)
If there are any extra definitions in the waybar config, such as
on-click-middle
, single/right clicking on the mpd icon in waybar has no effect.Fix proposal
I haven't found a way to actually make the client immediately realize that the mpd connection is dead on wake (short of setting a period Idle timer to check the connection?)
unique_connection
if there are any errors when usingIDLE_RUN_NOIDLE_AND_CMD
unique_connection
on errors inIdle::on_io
(e.g. when the connection is killed like shown above (fixes second bug)handlePlayPause
withhandleToggle
(fixes third bug)Reason for the third bug
The reason for the third bug is that when a
waybar::AModule::AModule
is instantiated, depending on the initialization values and the config, it connects a few slots, for example when a module has actions and/or some custom user actions are defined in the configuration,Gdk::BUTTON_PRESS_MASK
is added to the event box and
AModule::handleToggle(GdkEventButton* const& e)
is connected toevent_box_.signal_button_press_event()
(see https://github.com/Alexays/Waybar/blob/master/src/AModule.cpp#L50-L51)handletoggle(e)
callshandleUserEvent(e)
(https://github.com/Alexays/Waybar/blob/master/src/AModule.cpp#L152, https://github.com/Alexays/Waybar/blob/master/src/AModule.cpp#L156), which does its processing and returnstrue
, stopping other handlers from being invoked for the button press event.In the current definition of
waybar::modules::MPD::MPD(...)
,MPD::handlePlayPause
is connected toevent_box_.signal_button_press_event()
(https://github.com/Alexays/Waybar/blob/master/src/modules/mpd/mpd.cpp#L43-L44), but thisis only ever called if there are no user-defined actions (e.g.
on-click-middle
) in the configuration. When there's no user-defined actions in the configuration, in that casehandleToggle()
is never connected inwaybar::AModule::AModule()
, event processing is not stopped by theAModule::handleToggle
call andMPD::handlePlayPause
is actually called.This PR replaces
handlePlayPause
withhandleToggle()
, which handles play/pause/stop and then calls the parenthandleToggle
method.