Commit 2d7589e4 authored by Thomas Guillem's avatar Thomas Guillem

upnp: correctly fix deadlock when calling UpnpUnRegisterClient

UpnpInstanceWrapper::Callback() can be called while Upnp is unregistering via
UpnpUnRegisterClient(). Both functions locked the same mutex (s_lock) and this
resulted to a deadlock.

Add a new mutex to protect only the upnp callback.
parent 72c84ba4
...@@ -1067,12 +1067,14 @@ UpnpInstanceWrapper::UpnpInstanceWrapper() ...@@ -1067,12 +1067,14 @@ UpnpInstanceWrapper::UpnpInstanceWrapper()
, callback_( NULL ) , callback_( NULL )
, refcount_( 0 ) , refcount_( 0 )
{ {
vlc_mutex_init( &callback_lock_ );
} }
UpnpInstanceWrapper::~UpnpInstanceWrapper() UpnpInstanceWrapper::~UpnpInstanceWrapper()
{ {
UpnpUnRegisterClient( handle_ ); UpnpUnRegisterClient( handle_ );
UpnpFinish(); UpnpFinish();
vlc_mutex_destroy( &callback_lock_ );
} }
UpnpInstanceWrapper *UpnpInstanceWrapper::get(vlc_object_t *p_obj, Upnp_FunPtr callback, SD::MediaServerList *opaque) UpnpInstanceWrapper *UpnpInstanceWrapper::get(vlc_object_t *p_obj, Upnp_FunPtr callback, SD::MediaServerList *opaque)
...@@ -1127,6 +1129,7 @@ UpnpInstanceWrapper *UpnpInstanceWrapper::get(vlc_object_t *p_obj, Upnp_FunPtr c ...@@ -1127,6 +1129,7 @@ UpnpInstanceWrapper *UpnpInstanceWrapper::get(vlc_object_t *p_obj, Upnp_FunPtr c
// This assumes a single UPNP SD instance // This assumes a single UPNP SD instance
if (callback && opaque) if (callback && opaque)
{ {
vlc_mutex_locker lock( &s_instance->callback_lock_ );
assert(!s_instance->callback_ && !s_instance->opaque_); assert(!s_instance->callback_ && !s_instance->opaque_);
s_instance->opaque_ = opaque; s_instance->opaque_ = opaque;
s_instance->callback_ = callback; s_instance->callback_ = callback;
...@@ -1139,6 +1142,7 @@ void UpnpInstanceWrapper::release(bool isSd) ...@@ -1139,6 +1142,7 @@ void UpnpInstanceWrapper::release(bool isSd)
vlc_mutex_locker lock( &s_lock ); vlc_mutex_locker lock( &s_lock );
if ( isSd ) if ( isSd )
{ {
vlc_mutex_locker lock( &callback_lock_ );
callback_ = NULL; callback_ = NULL;
opaque_ = NULL; opaque_ = NULL;
} }
...@@ -1157,7 +1161,7 @@ UpnpClient_Handle UpnpInstanceWrapper::handle() const ...@@ -1157,7 +1161,7 @@ UpnpClient_Handle UpnpInstanceWrapper::handle() const
int UpnpInstanceWrapper::Callback(Upnp_EventType event_type, void *p_event, void *p_user_data) int UpnpInstanceWrapper::Callback(Upnp_EventType event_type, void *p_event, void *p_user_data)
{ {
UpnpInstanceWrapper* self = static_cast<UpnpInstanceWrapper*>( p_user_data ); UpnpInstanceWrapper* self = static_cast<UpnpInstanceWrapper*>( p_user_data );
vlc_mutex_locker lock( &self->s_lock ); vlc_mutex_locker lock( &self->callback_lock_ );
if ( !self->callback_ ) if ( !self->callback_ )
return 0; return 0;
self->callback_( event_type, p_event, self->opaque_ ); self->callback_( event_type, p_event, self->opaque_ );
......
...@@ -72,6 +72,7 @@ private: ...@@ -72,6 +72,7 @@ private:
static UpnpInstanceWrapper* s_instance; static UpnpInstanceWrapper* s_instance;
static vlc_mutex_t s_lock; static vlc_mutex_t s_lock;
UpnpClient_Handle handle_; UpnpClient_Handle handle_;
vlc_mutex_t callback_lock_; // protect opaque_ and callback_
SD::MediaServerList* opaque_; SD::MediaServerList* opaque_;
Upnp_FunPtr callback_; Upnp_FunPtr callback_;
int refcount_; int refcount_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment