[pulseaudio-discuss] [PATCH] loopback: Fix crash if Max Request change at unload

[Tanu Kaskinen]

On Fri, 2012-03-16 at 08:25 +0200, Tanu Kaskinen wrote:
> On Fri, 2012-02-10 at 17:39 +0100, Frédéric Dalleau wrote:
> > Module-bluetooth-policy can load and unload module-loopback on demand.
> > Sometimes if there is an error, module-loopback can be unloaded early.
> > When module-loopback is loaded, it attaches a sink and the sink
> > calls sink_input_update_max_request for which there is a callback.
> > In this callback, module-loopback will queue a message in order to
> > process it from main thread. The message is queued in the sink
> > thread queue.
> > 
> > There is a possibility that this message is not processed if unload
> > has been requested. When this happens, the message
> > is still in the queue, and will eventually be processed. This triggers
> > segfault because the message carries a pointer to the no
> > longer existing sink input.
> > 
> > The fix is inspired by pa_thread_mq does for registering a thread specific
> > message queue. The idea is to create a module-specific queue. Any pending
> > message at unload will be flushed without processing.
> 
> I would like the main thread (and all other threads too) to make sure
> that it doesn't process messages that have an invalid receiver. The main
> thread should have a registry of valid receivers that each incoming
> message is checked against. Sink inputs would register themselves there
> automatically when they are created and unregister when they are
> unlinked. What do you think? What do Colin, Arun and David think?

No comments received for this suggestion. But that doesn't matter,
because the suggestion was bad anyway: it's not possible that a message
gets processed whose receiver has been freed. That's because the message
queue holds a reference to the receiver until the message gets
processed. So my suggestion would solve a non-existing problem.

Now I don't get this part of the problem description: "There is a
possibility that this message is not processed if unload has been
requested. When this happens, the message is still in the queue, and
will eventually be processed. This triggers segfault because the message
carries a pointer to the no longer existing sink input."

If I've understood correctly, the "no longer existing sink input" is the
message receiver. Did I understand correctly? If so, then the problem
isn't that the sink input doesn't exist anymore (unless the sink input
is incorrectly unreffed).

Would it fix the crash if you changed this line:

    if (u->adjust_time > 0)

in the SINK_INPUT_MESSAGE_MAX_REQUEST_CHANGED handler to this:

    if (PA_SINK_INPUT_IS_LINKED(PA_SINK_INPUT(obj)) && u->adjust_time > 0)

-- 
Tanu