Threaded mainloop API steals signals unnecessarily

[jcornwall]

The following code in src/pulse/threaded-mainloop.c:

/* Make sure that signals are delivered to the main thread */
sigfillset(&mask);
pthread_sigmask(SIG_BLOCK, &mask, NULL);

Interferes badly with client applications that use signals for other purposes, including the Mono runtime environment. This functionality is only needed when pa_signal_init() has been called but is indiscriminately applied to all clients, when only a fraction will use this feature.

Attached is a patch to conditionally run this code when pa_signal_init() has been called. Because mainloop-signal.c is API-agnostic wiring the two together is tricky, so I opted to add an internal (exposed but not exported) API-agnostic function to act as a buffer. The patched library works with signal-dependent applications and does not alter functionality in other applications where pa_signal_init() is called, as it must before using signalling features.

[jcornwall]

Patch to conditionally call pthread_sigmask only once pa_signal_init has been called

[lennart]

Hmm, we explicitly block all signals in the threaded poll to make sure we don't interfere with normal signal delivery. (the reason for that is that signal handlers are process wide, and if a process receives a signal it is not defined which thread it will be deliered too. To make sure that the signals are only delivered to threads that are maintained by the application programmer himself we thus block signals in all threads we maintain.) This is for compatibility reasons. Now, as it turns out, Mono doesn't like that since it (mis)uses signals for debugging purposes. If that's the case however it probably is a better idea to temporarily enable signals when control is passed to mono code from the threaded event loop and block them again afterwards. i.e. each time you call into mono from one of the threaded callbacks do a:

sigfillset(&mask);
pthread_sigmask(SIG_UNBLOCK, &mask, &saved);
...
/* call into mono here */
...
pthread_sigmask(SIG_SETMASK, &saved, NULL);

Of course it would be more appropriate only to unblock the signals that mono really needs instead of all of them.

I don't think it would be a good idea to merge the patch you suggested. While it be good for compatibility with mono it will most likely create problems in other software. Also, blocking all signals feels more correct to me. And finally, changing behaviour in the middle of everything seems like the worst solution to me.

[jcornwall]

Thanks for the explanation. My assumption of pthread_sigmask was upside down; I had thought that SIG_BLOCK diverted signals to, rather than away from, that thread.

I had already tried intercepting callbacks to restore signals prior to making this bug report. It should have worked but I hadn't expected PulseAudio to invoke callbacks from multiple threads (the state callback is first called from the blocked pa_context_connect call and only afterwards from the mainloop thread) and had incorrectly optimised the pthread_sigmask call to occur only once. Now I simply check if the thread has been seen before and unblock signals if it hasn't, which works fine.

I will take this up with the Mono developers as it seems to be a bug to assume that any thread entering managed code receives the signals that the runtime requires. Quite what they do with those signals will be interesting to find out!

[lennart]

We do dispatch calls form multiple threads, however never "in parallel". i.e. if you lock the threaded main loop how you are supposed to do it all callbacks are properly serialized although not all called from the same thread.

Hmm, if you have a mono bugzilla id handy for this issue i'd be thankful if you could post it here, as i'd like to follow this issue too.

[jcornwall]

Been trying to elicit a response on the Mono mailing list to no avail so I've set up a bug report here:

https://bugzilla.novell.com/show_bug.cgi?id=463197