Multiple CAVA processes are spawned by mpd_oled but are never killed

[supercrab]

Hi

Whilst testing I noticed that mpd_oled creates a pipe to a CAVA process in int main() but it never closes it.
When mpd_oled is started and stopped the CAVA processes run orphaned in the background.
If the program is called with a custom CAVA input like alsa,hw:0' then the second time it is run, the orphaned CAVA process still holds a reference to the input device andmpd_oled` will fail to start.

Cheers Mase

[antiprism]

Hi Mase

I believe this is fixed in the current code code. Near the start of main.cpp there is a signal handler function. Add a call to abort at the end of it.

void signal_handler(int sig)
{
  switch (sig) {
  case SIGINT:
  case SIGHUP:
  case SIGTERM:
    cleanup();
    break;
  }
  abort();
}

I think it was possible for the signal handler to be called twice, and the cleanup code was then being run twice, and this was causing mpd_oled to segfault, and the segfault then stopped the cava process being terminated (as part of a normal termination of mpd_oled).

Adrian.

[supercrab]

Hi

I've upgraded the mpd_oled to the new version and this has been resolved :)

I've also change the plugin to download mpd_oled from this repository again.

Cheers Mase

[supercrab]

Hi

I got round to adding the changing the signal_handler() method but it didn’t resolve the issue. I also noticed that mpd_oled Not only creates a CAVA’ process but ampd_oled_cava` process too. Low priority really. Just thought I’d let you know.

Cheeers Mase

[antiprism]

Hi Mase

If mpd_oled crashes the cava process is left running.

There was an issue of mpd_oled crashing when being terminated with a normal termination signal (like Ctrl-C), which would then leave the cava process running. The signal handler change fixes that crash.

The current installation instructions create a cava binary called mpd_oled_cava, which includes only the required features (to reduce dependencies), and will not conflict with any other install of cava. The current mpd_oled uses 'mpd_oled_cava' by default, but will use 'cava' if option -k is specified. If you have run the current mpd_oled and the old one too and they have both crashed then you will have mpd_oled_cava and cava processes still running.

Ideally, mpd_oled would never crash, and if it can crash then this should be fixed (but maybe it isn't crashing anymore after the signal handler change). However, I'll see if I can catch the segfault signal and terminate the cava child process to minimise the inconvenience if it does occur.

Adrian.

P.S. I put together a test version of mpd_oled that uses U8G2 library for the driver and graphics. It works well and looks good. Also, the mpd_oled binary uses approximately 9% CPU, instead of approximately 13% CPU, which will make all the difference when running on a Pi Zero.

[antiprism]

Hi Mase

I have tested some code and it is possible to kill the cava process that result from a segfault (using a custom version of popen()), but it would be better to track down if there is still a problem first, as the rogue processes will indicate this. Could you check that there are no cava or mpd_oled_cava processes running, and that you only use copies of mpd_oled with the signal handling fix applied (and you don't have any local changes, that might lead to segfaults), then do you still find that extra cava or mpd_oled_cava processes are running later? The only time I see them now is when I am testing code changes, and these changes cause a segfault.

Adrian.

[supercrab]

I just tested this and when I hit Ctrl + C the orphaned processes do actually get destroyed properly.

I'm not sure what I was doing the other day. I am on Volumio 3 - I can't see that making a difference...