Re: Race condition causes crash in AVTransport's dispose

[Jens Georg <mail jensge org>]

hi,

> Hello,
>
>   I tried to discuss this also on #gupnp, but sending an e-mail here
> would be probably more appropriate. We use gupnp and (slightly older)

Yeah, sorry. I don't have access to IRC for half the day. Mail is better 
;)

> rygel and (pretty recent) gupnp to implement upnp DMR functionality.
> Sometimes, it is necesary to restart the whole rygel-renderer/gupnp
> stack (without restarting process).

I see. Nice to hear that it's got adapted outside of zombiesque projects 
:D
Which probably means I should stop breaking API/ABI, even if its 
versioned breaks...

>   When restarting Rygel after playback start, we sometimes see crash
> in gupnp_service_dispose (and in gupnp_service_infoget_control_url).
> We traced the issue, came up with following scenarion causing the
> crash:

That crash sounds familiar although I could not find it in bugzilla.

>   1) SetAVTransportURI is used to set new playback URI;
> AVTransport.handle_new_transport_uri is called and a soup message is
> created in order to get DLNA content features from a DMS
>   2) Rygel.MediaRenderer is unreffed, and so are all its services;
> Soup message, however, holds a reference to the AVTransport instance,
> so it is not freed
>   3) Soup message handling is completed, AVTransport instance is 
> unreffed
>   4) Destructors are called; GUPnPService destructor fails, because it
> attempts to access contents of an 'element' property, which was freed
> when destroying MediaRenderer
>
>   We currently don't see a clear way to fix this (nor know whether
> this is an issue of gupnp or rygel), here are few ideas:
>
>   1) The 'document' property of gupnp service is currently initialized
> to NULL. Unfortunately, there is no documentation about what kind of
> XML document it should contain, thus I don't know how exactly is it
> meant to be used

It is propagated from the root device through self-inspection (IIRC), 
but it should be refcounted. Not sure why it is set to NULL.

>   2) Destroy the message if the MediaRenderer is being destroyed.

That's what I currently would lean to. Actually that should also happen 
if the message is still running while a new SetAVTransportURI request is 
issued, that might be racy as well and cause unpleasant side effects.

>   That's current state of our analysis of this problem; if you have
> any recommendations on what would be an acceptable way to continue,
> please let me know. We will gladly implement/test/contribute fix for
> this, but currently, we are stuck in above mentioned state.
>
>   Best regards,
>     Milan Plzik
> _______________________________________________
> rygel-list mailing list
> rygel-list gnome org
> https://mail.gnome.org/mailman/listinfo/rygel-list