[vlc-commits] upnp: correctly fix deadlock when calling UpnpUnRegisterClient

[Thomas Guillem]

vlc | branch: master | Thomas Guillem <thomas at gllm.fr> | Mon Feb  8 18:04:36 2016 +0100| [2d7589e4f45f97c91ef6f151dc76ef2c4810e842] | committer: 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.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=2d7589e4f45f97c91ef6f151dc76ef2c4810e842
---

 modules/services_discovery/upnp.cpp |    6 +++++-
 modules/services_discovery/upnp.hpp |    1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/modules/services_discovery/upnp.cpp b/modules/services_discovery/upnp.cpp
index 5e7c2a1..be57054 100644
--- a/modules/services_discovery/upnp.cpp
+++ b/modules/services_discovery/upnp.cpp
@@ -1067,12 +1067,14 @@ UpnpInstanceWrapper::UpnpInstanceWrapper()
     , callback_( NULL )
     , refcount_( 0 )
 {
+    vlc_mutex_init( &callback_lock_ );
 }
 
 UpnpInstanceWrapper::~UpnpInstanceWrapper()
 {
     UpnpUnRegisterClient( handle_ );
     UpnpFinish();
+    vlc_mutex_destroy( &callback_lock_ );
 }
 
 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
     // This assumes a single UPNP SD instance
     if (callback && opaque)
     {
+        vlc_mutex_locker lock( &s_instance->callback_lock_ );
         assert(!s_instance->callback_ && !s_instance->opaque_);
         s_instance->opaque_ = opaque;
         s_instance->callback_ = callback;
@@ -1139,6 +1142,7 @@ void UpnpInstanceWrapper::release(bool isSd)
     vlc_mutex_locker lock( &s_lock );
     if ( isSd )
     {
+        vlc_mutex_locker lock( &callback_lock_ );
         callback_ = NULL;
         opaque_ = NULL;
     }
@@ -1157,7 +1161,7 @@ UpnpClient_Handle UpnpInstanceWrapper::handle() const
 int UpnpInstanceWrapper::Callback(Upnp_EventType event_type, void *p_event, void *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_ )
         return 0;
     self->callback_( event_type, p_event, self->opaque_ );
diff --git a/modules/services_discovery/upnp.hpp b/modules/services_discovery/upnp.hpp
index 0cdbd8d..27c7338 100644
--- a/modules/services_discovery/upnp.hpp
+++ b/modules/services_discovery/upnp.hpp
@@ -72,6 +72,7 @@ private:
     static UpnpInstanceWrapper* s_instance;
     static vlc_mutex_t s_lock;
     UpnpClient_Handle handle_;
+    vlc_mutex_t callback_lock_; // protect opaque_ and callback_
     SD::MediaServerList* opaque_;
     Upnp_FunPtr callback_;
     int refcount_;