Bug 95494

Created attachment 8728 [details]
Patch to cure this bug

I found out that the bug is caused by a broken pipe error on the alsa output
channel. This error condition is tested in several methods of the class
AudioIOALSA, also, there are methods available to cure the channel from this
condition. I therefore assume the broken condition of the output channel to be
something "given" and didn't try to find out the reason for it to occur.

Such a test is missing in at least one method, and, unfortunately, it's exactly
the place where a broken pipe would be detected after the modification of event
dispatching between versions 1.2 and 1.3:
After artsd wakes up from a select on the active file descriptors, the first
alsa snd_* function called is, via AudioIOALSA::getParam, snd_pcm_avail_update.
Its return value was not checked at all but directly run through
snd_pcm_frames_to_bytes. In error case, this returns a negative number (a
corresponding multiple of the true error code). The event processor only checks
for the returned value > 0 and returns from the event handling method
immediately. So, none of the methods that handle the broken pipe condition in
AudioIOALSA ever gets called.
As a result, the select statement of the event dispatcher immediately returns
because bytes could be written to output, the event dispatcher calles the
corresponding event handler which immediately returns because it assumes that
it can't put any bytes to output. This results in the described busy hang of
artsd, even a machine hang if artsd is run with "real time priority" settings.

The patch I appended consists of copying the usual failure recovery from the
broken pipe condition found e.g. in the playback methods AudioIOALSA to the
getParam method. In practise, these work well, i.e. the audio channel has
always been usable after returning from the method in all my tests.

Additionally, I put a test into audiosubsys to abort artsd in the case that the
event handling function for the "ready to write" event finds that no bytes are
free on the output channel - this appears to be an obviously inconsistency in
the sound system, no matter what the underlying implementation is. I did not
test this, however, with other implementations than AudioIOALSA.

Nice, but you cannot assume you can write just because ALSA has restarted you. Far too many drivers are buggy and just restarts the client every now and then.

Created attachment 8734 [details]
Restart pcm channels with PIPE error condition also from getParam method

I now put the error checking/restart/xrun code into a separate method to be
used from all methods where it may be relevant. The idea is that the
handleError method checks a supplied return code for error conditions it can
handle. If so, it will restart or xrun the relevant pcm channel and return
-EAGAIN to the caller. So, getParam wraps handleError around the called
function, and if the result is -EAGAIN, calles that function again.

It already safes a lot of code duplication, though still the solution is not
too elegant, as the caller still has to check for a certain error condition and
re-call the failed function. To cure this design flaw, I can only think of a
hierarchy of sound action objects that describe a specific alsa call each and
are supplied to an issueCall method or so, that handles error checking and
re-calling itself. But this again seems to be stupidly over-designed.

The problem is still there in Version 1.4.3

It's gone for a while. Probably with version 1.5.