'moc --info' can fail randomly

Submitted by zealcharm on Sat, 2020-02-08 20:14

Forums:

MOC version: 
2.5.2

Hello,

After noticing that I was regularly seeing some core dumps in journalctl, I decided to find out why. After some investigation, I found the following:

* The crashes didn't come from just having MOC running but rather from the py3status's MOC plugin which adds some information about the track that is currently playing in the status bar. The way this works is that every N seconds, it runs `moc --info` to poll the currently playing track.

* After some tests, I found that I could reproduce the crash on the command line by simply running `moc --info` over and over a few dozen times. (It seems to happen way more often on certain files).

* After debugging a bit on the code, I found that the crash happens on `interface.c` --> `get_tags_no_iface` due to the call to `abort()`. The reason the call to `abort()` is triggered is because sometimes the event received is not `EV_FILE_TAGS`, but rather `EV_CTIME` which is generated by the server every second. So basically if you run `moc --info` enough times, eventually one of those periodic `EV_CTIME` events is going to "sneak in" before the `EV_FILE_TAGS` and cause the abort/crash.

This isn't exclusive of `EV_CTIME` but can also happen with other events, for example `EV_STATE`. Based on this, here's a standalone script that reproduces the problem. Simply run the script while a song is playing and you will quickly get a abort/crash:

#!/usr/bin/env bash set -euo pipefail trap "kill 0" EXIT # Constantly pauses and unpause to generate EV_STATE events (while true; do mocp --pause; sleep 0.05; mocp --unpause; sleep 0.05; done ) & # And at the same time request the track's information, which will # generate EV_FILE_TAGS events while true; do mocp --info; done # Our objective here is to sneak a EV_STATE event in interface.c get_tabs_no_iface, # in between the call from send_tags_request to get_int_from_srv. # If this happens, the call to mocp --info will call abort() and cause a core dump

Judging by the comment on `get_tags_no_iface` I'm not sure if this is a "known issue" or it was actually overlooked. But in either case it looks like this function (and also other places like `fill_tags`) need a mini event-loop that ignores those events.

This is my preliminary patch to make 'moc --info' more stable by ignoring non-critical events, but I haven't been able to exhaustively check yet that all kinds of events are handled correctly.

--- interface.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/interface.c b/interface.c index 1ba43bb..6fce4c7 100644 --- a/interface.c +++ b/interface.c @@ -3894,15 +3894,14 @@ static struct file_tags *get_tags_no_iface (const char *file, if (!strcmp(ev->file, file)) tags = tags_dup (ev->tags); - - free_tag_ev_data (ev); - } - else { - /* We can't handle other events, since this function - * is to be invoked without the interface. */ - logit ("Server sent an event which I didn't expect!"); - abort (); + } else if (type == EV_BUSY) { + interface_fatal ("The server is busy; " + "another client is connected!"); + } else if (type == EV_EXIT) { + interface_fatal ("The server exited!"); } + + free_event_data(type, data); } return tags; --

Regards.