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

2010-03-22 Thread Lennart Poettering
On Sun, 14.03.10 20:50, David Henningsson (launchpad@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.

Lennart

-- 
Lennart PoetteringRed Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/   GnuPG 0x1A015CC4
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


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

2010-03-22 Thread David Henningsson
Lennart Poettering wrote:
 On Sun, 14.03.10 20:50, David Henningsson (launchpad@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
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss