Skip to content

Commit

Permalink
Don't call deinitialization from signal handler
Browse files Browse the repository at this point in the history
Eugene Toder reports:

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.

Signed-off-by: Claudio Matsuoka <cmatsuoka@gmail.com>
  • Loading branch information
cmatsuoka committed Nov 25, 2014
1 parent ecc0528 commit ccfebb0
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
2 changes: 2 additions & 0 deletions Changelog
Expand Up @@ -3,6 +3,8 @@ Stable versions

4.0.10 ():
- Add AIFF file output driver (by Lorence Lombardo)
- Add command 'h' to display help message (by Eugene Toder)
- Fix sound driver deinitialization on signal (by Eugene Toder)
- Adjust CoreAudio driver latency
- Fix missing --all-sequences in help message

Expand Down
6 changes: 5 additions & 1 deletion src/main.c
Expand Up @@ -57,7 +57,11 @@ static void cleanup(int sig)
signal(SIGFPE, SIG_DFL);
signal(SIGSEGV, SIG_DFL);

sound->deinit();
/* Don't deinit from signal handler
* (https://github.com/cmatsuoka/xmp-cli/issues/5)
* sound->deinit();
*/

reset_tty();

signal(sig, SIG_DFL);
Expand Down

0 comments on commit ccfebb0

Please sign in to comment.