search

libxmp / xmp-cli

Command-line mod player using libxmp
GNU General Public License v2.0
75 stars 22 forks source link

Assertion failure with PulseAudio driver when hitting Ctrl-C #5

Closed eltoder closed 9 years ago

eltoder commented 9 years ago

When I abort the playback using Ctrl-C I get the following error:

Assertion 'pthread_mutex_destroy(&m->mutex) == 0' failed at pulsecore/mutex-posix.c:83, function pa_mutex_free(). Aborting.
Aborted (core dumped)

This happens because xmp registers a signal handler for SIGINT that calls sound->deinit(). For the PulseAudio driver this calls pa_simple_free(), which is apparently not happy to be called from a signal handler. Commenting out the call to deinit() fixes the problem, and seems to work well for both OSS and PulseAudio.

Generally speaking, calling deinit() from a signal handler is dangerous -- it's likely to call free() or similar, which is not signal safe. Is this necessary? If the idea is to have a graceful shutdown on Ctrl-C, the handler should set a flag or use longjmp() to exit the play loop in main(). Also, attempting a graceful shutdown for SIGFPE and especially SIGSEGV is probably not a good idea.

cmatsuoka commented 9 years ago

Indeed. I'll check that and report back.

cmatsuoka commented 9 years ago

I don't remember if there was a specific reason for the deinitialization to be there, but my guess is that there wasn't, and no problems were reported for the sound drivers that existed at that time so it just stayed there. So instead of a very elaborate solution to handle SIGTERM, I think we can just comment out the deinit() call. Most drivers only close a file descriptor, and I just tested it for OS X, which has more code there, and it worked well. If any problem appears in the less used OSes, we go for a better fix (but is there anyone out there playing mods on IRIX these days?) Thanks for reporting and testing!

eltoder commented 9 years ago

Thank you for fixing!