[pulseaudio-discuss] [PATCH] Fix crash on jack server shutdown

[David Henningsson]

Lennart Poettering wrote:
> On Sun, 14.03.10 20:50, David Henningsson (launchpad.web at epost.diwic.se) wrote:
> 
>> On sink unlinking, existing sink inputs are moved, which in turn calls
>> a get latency callback, which references the jack client. Therefore,
>> make sure the sink is unlinked before the client is closed. Failure to
>> do so might lead to SIGSEGV.
>>
>> This patch simply moves the call to pa_sink_unlink above
>> jack_client_close, which fixes the problem.
> 
> Uh. I don't think this really fixes the problem. This just moves things
> around and replaces one race by another. 
> 
> The old race: the PA IO loop might end up calling into libjack with an
> invalid jack client context after the context got destructed but the PA
> sink didn't get shutdown yet.
> 
> The new race: the jack IO loop might end up calling (and blocking) into
> PA code when the PA sink is unlinked (though not yet destructed) but the
> jack code is not yet.
> 
> What is missing is that the jack loop does not depend on the PA sink to
> be around resp. the PA IO loop doesn't call into jack when it is
> dead. If either of that is implemented (and then destruction order
> matches this) the problem is fixed.
> 
> Also, what applies to m-jack-sink needs fixing in m-jack-source, too.

Good points. I admit to not having checked properly whether
pa_sink_unlink and pa_sink_render_full can be called in parallell, I see
now that they shouldn't.

Since appropriate stream moving is good, and stream moving (apparently)
needs to do a get-latency call to do its job properly, I would say that
this was a step in the right direction.

So, let us also send a message to the thread to stop rendering (i e stop
calling pa_sink_xxx), before the pa_sink_unlink call? If this sounds
like a good idea to you, I'll prepare a git patch.

// David