Waybar: Certain songs crash waybar with the new mpd module

[cole-h]

On waybar version Waybar v0.5.1-33-ga14b933 (Apr 18 2019, branch 'master'), sway version sway version 1.0-rc1-243-g35ddd9aa (Apr 18 2019, branch 'master'). You can find my config and stylesheet here: https://gist.github.com/cole-h/33d75033b027ae59f177c2a0dd419330

This only occurs on a handful of songs for me. I've only gone through a few looking for this behavior, but the songs so far are "Puzzle" by Beats Antique, "Everything In Its Right Place" by Radiohead, any song by Sabaton (so far). They all exhibited the exact same backtraces:

#0  0x00007ffff5f056e5 in __strlen_avx2 () at /usr/lib/libc.so.6
#1  0x00005555555b2754 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign(char const*) ()
#2  0x00005555555bfd25 in waybar::modules::MPD::setLabel() ()
#3  0x00005555555c0002 in waybar::modules::MPD::update() ()
#4  0x000055555559a169 in sigc::internal::slot_call0<waybar::Bar::getModules(waybar::Factory const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)::{lambda()#1}, void>::call_it(sigc::internal::slot_rep*) ()
#5  0x00007ffff751edea in Glib::DispatchNotifier::pipe_io_handler(Glib::IOCondition) () at /usr/lib/libglibmm-2.4.so.1
#6  0x00007ffff7520ecc in Glib::IOSource::dispatch(sigc::slot_base*) () at /usr/lib/libglibmm-2.4.so.1
#7  0x00007ffff75210cf in Glib::Source::dispatch_vfunc(_GSource*, int (*)(void*), void*) () at /usr/lib/libglibmm-2.4.so.1
#8  0x00007ffff69f8661 in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#9  0x00007ffff69fa739 in  () at /usr/lib/libglib-2.0.so.0
#10 0x00007ffff69fa77e in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#11 0x00007ffff6bb2f5e in g_application_run () at /usr/lib/libgio-2.0.so.0
#12 0x000055555557acac in waybar::Client::main(int, char**) ()
#13 0x00005555555798cc in main ()

---

STR

1. Enable and configure the new mpd module in ~/.config/waybar/config
2. Start up waybar while music is stopped or an affected (?) song isn't playing
3. Bar acts predictably
4. Play affected song (such as "Everything In Its Right Place") by either queuing it up or switching directly to it from an unaffected song
5. Observe segfault

Commenting out the mpd module lets the bar function normally.

[Alexays]

@minijackson Can you take a look?

[minijackson]

@cole-h Could you post your configuration of the MPD module, and the result of ffprobe <music-file>?

Edit: Sorry, I didn't see the Gist ^^

[cole-h]

Attached are the ffprobes of two songs that I can reproduce the crash with 100% of the time.

Beats Antique - Puzzle

ffprobe version n4.1.3 Copyright (c) 2007-2019 the FFmpeg developers
  built with gcc 8.2.1 (GCC) 20181127
  configuration: --prefix=/usr --disable-debug --disable-static --disable-stripping --enable-fontconfig --enable-gmp --enable-gnutls --enable-gpl --enable-ladspa --enable-libaom --enable-libass --enable-libbluray --enable-libdrm --enable-libfreetype --enable-libfribidi --enable-libgsm --enable-libiec61883 --enable-libjack --enable-libmodplug --enable-libmp3lame --enable-libopencore_amrnb --enable-libopencore_amrwb --enable-libopenjpeg --enable-libopus --enable-libpulse --enable-libsoxr --enable-libspeex --enable-libssh --enable-libtheora --enable-libv4l2 --enable-libvidstab --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxcb --enable-libxml2 --enable-libxvid --enable-nvdec --enable-nvenc --enable-omx --enable-shared --enable-version3
  libavutil      56. 22.100 / 56. 22.100
  libavcodec     58. 35.100 / 58. 35.100
  libavformat    58. 20.100 / 58. 20.100
  libavdevice    58.  5.100 / 58.  5.100
  libavfilter     7. 40.101 /  7. 40.101
  libswscale      5.  3.100 /  5.  3.100
  libswresample   3.  3.100 /  3.  3.100
  libpostproc    55.  3.100 / 55.  3.100
Input #0, mp3, from '/home/cole/Music/Beats Antique/A Thousand Faces - Act 2/02 Puzzle.mp3':
  Metadata:
    TYER            : 0
    track           : 2/9
    disc            : 0/0
    replaygain_album_gain: -8.11 dB
    replaygain_album_peak: 2.009017
    replaygain_track_gain: -9.13 dB
    replaygain_track_peak: 1.081100
    title           : Puzzle
    artist          : Beats Antique
    album_artist    : Beats Antique
    album           : A Thousand Faces - Act 2
  Duration: 00:04:21.17, start: 0.025056, bitrate: 324 kb/s
    Stream #0:0: Audio: mp3, 44100 Hz, stereo, fltp, 320 kb/s
    Metadata:
      encoder         : LAME3.99r
    Side data:
      replaygain: track gain - -9.130000, track peak - 0.000025, album gain - -8.110000, album peak - 0.000047, 
    Stream #0:1: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 512x512 [SAR 1:1 DAR 1:1], 90k tbr, 90k tbn, 90k tbc
    Metadata:
      comment         : Cover (front)

Radiohead - Everything In Its Right Place

ffprobe version n4.1.3 Copyright (c) 2007-2019 the FFmpeg developers
  built with gcc 8.2.1 (GCC) 20181127
  configuration: --prefix=/usr --disable-debug --disable-static --disable-stripping --enable-fontconfig --enable-gmp --enable-gnutls --enable-gpl --enable-ladspa --enable-libaom --enable-libass --enable-libbluray --enable-libdrm --enable-libfreetype --enable-libfribidi --enable-libgsm --enable-libiec61883 --enable-libjack --enable-libmodplug --enable-libmp3lame --enable-libopencore_amrnb --enable-libopencore_amrwb --enable-libopenjpeg --enable-libopus --enable-libpulse --enable-libsoxr --enable-libspeex --enable-libssh --enable-libtheora --enable-libv4l2 --enable-libvidstab --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxcb --enable-libxml2 --enable-libxvid --enable-nvdec --enable-nvenc --enable-omx --enable-shared --enable-version3
  libavutil      56. 22.100 / 56. 22.100
  libavcodec     58. 35.100 / 58. 35.100
  libavformat    58. 20.100 / 58. 20.100
  libavdevice    58.  5.100 / 58.  5.100
  libavfilter     7. 40.101 /  7. 40.101
  libswscale      5.  3.100 /  5.  3.100
  libswresample   3.  3.100 /  3.  3.100
  libpostproc    55.  3.100 / 55.  3.100
Input #0, mp3, from '/home/cole/Music/Radiohead/Kid A/1 - Everything In Its Right Place.mp3':
  Metadata:
    track           : 1/11
    replaygain_album_gain: -8.11 dB
    replaygain_album_peak: 2.009017
    replaygain_track_gain: -8.56 dB
    replaygain_track_peak: 1.005117
    title           : Everything In Its Right Place
    artist          : Radiohead
    album           : Kid A
    date            : 2000
  Duration: 00:04:11.45, start: 0.025056, bitrate: 325 kb/s
    Stream #0:0: Audio: mp3, 44100 Hz, stereo, fltp, 320 kb/s
    Metadata:
      encoder         : Lavc57.24
    Side data:
      replaygain: track gain - -8.560000, track peak - 0.000023, album gain - -8.110000, album peak - 0.000047, 
    Stream #0:1: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 800x800 [SAR 300:300 DAR 1:1], 90k tbr, 90k tbn, 90k tbc
    Metadata:
      comment         : Cover (front)

I can also reproduce on songs by a friend's band, so I'll ask if they wouldn't mind me sending one of you a music file to see if it's truly the song's fault or if it's just my environment. If the above information does not help, let me know and I'll do just that.

[cole-h]

warning, wall of text below, some things might be unrelated to the issue at hand

I think I've narrowed down the problem. Setting up the build in debug mode let me see that the segfault was happening when calling strlen on an empty album_artist at mpd.cpp:127. Sure enough, all the problematic songs had empty album artist tags. Commenting out that line lets the bar function as expected on all previously-problematic songs. Uncommenting and rebuilding has the problem immediately rear its ugly head again.

That's the extent of my abilities, however. I hope this helps you track down why this happens and resolve it.

EDIT: It seems ANY tag being empty causes a segfault. I used youtube-dl to download a song just now, played it through MPD, and got a segfault. I also reproduced on a song with an empty date tag.

EDIT2: Cobbled together this patch after looking at this libmpdclient example. I don't plan on sending it in because there is probably a better way to do this. Updated the below patch to be nicer looking (some inspiration from ncmpcpp). Handles NULL checking in new function so it doesn't look like spaghetti. Defaults to "N/A" because I want to see more than just -- when tags aren't present. Feel free to use this code if it's acceptable.

diff --git a/include/modules/mpd.hpp b/include/modules/mpd.hpp
index b3134f4..0574d8d 100644
--- a/include/modules/mpd.hpp
+++ b/include/modules/mpd.hpp
@@ -15,6 +15,7 @@ class MPD : public ALabel {

  private:
   std::thread periodic_updater();
+  std::string getTag(mpd_tag_type type, unsigned idx = 0);
   void        setLabel();
   std::string getStateIcon();
   std::string getOptionIcon(std::string optionName, bool activated);
diff --git a/src/modules/mpd.cpp b/src/modules/mpd.cpp
index bd9b095..c838bd0 100644
--- a/src/modules/mpd.cpp
+++ b/src/modules/mpd.cpp
@@ -75,6 +75,15 @@ std::thread waybar::modules::MPD::periodic_updater() {
   });
 }

+std::string waybar::modules::MPD::getTag(mpd_tag_type type, unsigned idx) {
+  std::string result = "N/A";
+  const char* tag = mpd_song_get_tag(song_.get(), type, idx);
+  // mpd_song_get_tag can return NULL, so make sure it's valid before setting
+  if (tag)
+    result = tag;
+  return result;
+}
+
 void waybar::modules::MPD::setLabel() {
   if (connection_ == nullptr) {
     label_.get_style_context()->add_class("disconnected");
@@ -123,11 +132,11 @@ void waybar::modules::MPD::setLabel() {

     stateIcon = getStateIcon();

-    artist = mpd_song_get_tag(song_.get(), MPD_TAG_ARTIST, 0);
-    album_artist = mpd_song_get_tag(song_.get(), MPD_TAG_ALBUM_ARTIST, 0);
-    album = mpd_song_get_tag(song_.get(), MPD_TAG_ALBUM, 0);
-    title = mpd_song_get_tag(song_.get(), MPD_TAG_TITLE, 0);
-    date = mpd_song_get_tag(song_.get(), MPD_TAG_DATE, 0);
+    artist = getTag(MPD_TAG_ARTIST);
+    album_artist = getTag(MPD_TAG_ALBUM_ARTIST);
+    album = getTag(MPD_TAG_ALBUM);
+    title = getTag(MPD_TAG_TITLE);
+    date = getTag(MPD_TAG_DATE);
     elapsedTime = std::chrono::seconds(mpd_status_get_elapsed_time(status_.get()));
     totalTime = std::chrono::seconds(mpd_status_get_total_time(status_.get()));
   }
@@ -234,7 +243,7 @@ void waybar::modules::MPD::tryConnect() {

   try {
     checkErrors(connection_.get());
-  } catch (std::runtime_error e) {
+  } catch (std::runtime_error &e) {
     std::cerr << module_name_ << ": Failed to connect to MPD: " << e.what() << std::endl;
     connection_.reset();
     alternate_connection_.reset();

(changing runtime_error to a reference fixed a compile-time warning; not necessary for this issue)

EDIT3: Might be related: I am also getting segfaults at mpd_status_get_state after leaving music paused for a while. The only two times I think I've witnessed this issue are after letting swayidle dpms off my screens and then waking them back up, entering my password for swaylock, and pressing play, only to find a lovely segfault with the following trace. Line numbers might not be accurate because I've been playing around with all kinds of different stuff. Tomorrow I'll try setting swayidle to a shorter timeout (currently at 25m for dpms off) and see if this is actually reproducible.

SIGSEGV

#0  0x00007ffff62c3680 in mpd_status_get_state () at /usr/lib/libmpdclient.so.2
#1  0x00005555555f8bed in waybar::modules::MPD::fetchState() (this=0x555555753e20) at ../src/modules/mpd.cpp:276
#2  0x00005555555f5f7e in waybar::modules::MPD::update() (this=0x555555753e20) at ../src/modules/mpd.cpp:37
#3  0x00005555555b08b1 in waybar::Bar::<lambda()>::operator()(void) const (__closure=0x555555ab54d0) at ../src/bar.cpp:194
#4  0x00005555555b16ee in sigc::adaptor_functor<waybar::Bar::getModules(const waybar::Factory&, const string&)::<lambda()> >::operator()(void) const (this=0x555555ab54d0) at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:256
#5  0x00005555555b1595 in sigc::internal::slot_call0<waybar::Bar::getModules(const waybar::Factory&, const string&)::<lambda()>, void>::call_it(sigc::internal::slot_rep *) (rep=0x555555ab54a0) at /usr/include/sigc++-2.0/sigc++/functors/slo
t.h:136
#6  0x00007ffff751edea in Glib::DispatchNotifier::pipe_io_handler(Glib::IOCondition) () at /usr/lib/libglibmm-2.4.so.1
#7  0x00007ffff7520ecc in Glib::IOSource::dispatch(sigc::slot_base*) () at /usr/lib/libglibmm-2.4.so.1
#8  0x00007ffff75210cf in Glib::Source::dispatch_vfunc(_GSource*, int (*)(void*), void*) () at /usr/lib/libglibmm-2.4.so.1
#9  0x00007ffff69f8661 in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#10 0x00007ffff69fa739 in  () at /usr/lib/libglib-2.0.so.0
#11 0x00007ffff69fa77e in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#12 0x00007ffff6bb2f5e in g_application_run () at /usr/lib/libgio-2.0.so.0
#13 0x00005555555b61fd in waybar::Client::main(int, char**) (this=0x5555556bb8a0, argc=1, argv=0x7fffffffe188) at ../src/client.cpp:258
#14 0x00005555555add1f in main(int, char**) (argc=1, argv=0x7fffffffe188) at ../src/main.cpp:22

[minijackson]

Your patch looks very nice! I think you could go ahead and make a pull request for those sweet internet points ^^

I think I've also encountered the segfault after leaving a music paused for a while, but never managed to reproduce it (I thought it was random, but now you gave me a lead! So I'll also look into it)

[cole-h]

Update on the mpd_status_get_state segfault: it occurs due to state_.get() occasionally being nullptr. I don't know of a nice way to handle this because my rudimentary modifications (just checking if (state_.get() != nullptr) before get state) would update the module to say "Disconnected", but wouldn't ever re-set state. Just mpd: Connection to MPD closed in the logs.

STR (on my system, at least)

1. Start up the bar
2. Play music for a second or two
3. Pause it
4. Do something for a minute or so (for me, always happens after 1 minute, exactly)
5. Play music again
6. Observe segfault

N.B. Everywhere else that uses state_.get() has a chance to segfault -- I saw a few in the elapsedTime and totalTime variables, as well as the icons just below that while messing around.