Podcast: thread safety fixes
authorRémi Denis-Courmont <rdenis@simphalempin.com>
Sun, 7 Sep 2008 11:06:14 +0000 (14:06 +0300)
committerRémi Denis-Courmont <rdenis@simphalempin.com>
Sun, 7 Sep 2008 11:06:14 +0000 (14:06 +0300)
Variables callback can be (and usually are) invoked from other threads.
They cannot access the object without locking, and must be unregistered
before the object is destroyed (var_DelCallback() -> free(p_sys)).

modules/services_discovery/podcast.c

index 95d4ba6..d6d6a7a 100644 (file)
@@ -33,6 +33,7 @@
 #include <vlc_plugin.h>
 #include <vlc_playlist.h>
 #include <vlc_network.h>
+#include <assert.h>
 
 #include <errno.h>                                                 /* ENOMEM */
 
@@ -88,6 +89,8 @@ struct services_discovery_sys_t
     char **ppsz_urls;
     int i_urls;
 
+    vlc_mutex_t lock;
+    vlc_cond_t  wait;
     bool b_update;
 };
 
@@ -114,6 +117,8 @@ static int Open( vlc_object_t *p_this )
     p_sys->ppsz_urls = NULL;
     p_sys->i_input = 0;
     p_sys->pp_input = NULL;
+    vlc_mutex_init( &p_sys->lock );
+    vlc_cond_init( &p_sys->wait );
     p_sys->b_update = true;
 
     p_sd->pf_run = Run;
@@ -122,6 +127,10 @@ static int Open( vlc_object_t *p_this )
     /* Give us a name */
     services_discovery_SetLocalizedName( p_sd, _("Podcasts") );
 
+    /* Launch the callback associated with this variable */
+    var_Create( p_sd, "podcast-urls", VLC_VAR_STRING | VLC_VAR_DOINHERIT );
+    var_AddCallback( p_sd, "podcast-urls", UrlsChange, p_sys );
+
     return VLC_SUCCESS;
 }
 
@@ -133,6 +142,11 @@ static void Close( vlc_object_t *p_this )
     services_discovery_t *p_sd = ( services_discovery_t* )p_this;
     services_discovery_sys_t *p_sys  = p_sd->p_sys;
     int i;
+
+    var_DelCallback( p_sd, "podcast-urls", UrlsChange, p_sys );
+    vlc_cond_destroy( &p_sys->wait );
+    vlc_mutex_destroy( &p_sys->lock );
+
     for( i = 0; i < p_sys->i_input; i++ )
     {
         if( p_sd->p_sys->pp_input[i] )
@@ -155,25 +169,20 @@ static void Run( services_discovery_t *p_sd )
 {
     services_discovery_sys_t *p_sys  = p_sd->p_sys;
 
-    /* Launch the callback associated with this variable */
-    var_Create( p_sd, "podcast-urls", VLC_VAR_STRING | VLC_VAR_DOINHERIT );
-    var_AddCallback( p_sd, "podcast-urls", UrlsChange, p_sys );
-
+    vlc_mutex_lock( &p_sys->lock );
+    mutex_cleanup_push( &p_sys->lock );
     for( ;; )
     {
-        /* FIXME: That's 2000 wake up per seconds too many. */
-        msleep( 500 );
+        while( !p_sys->b_update )
+            vlc_cond_wait( &p_sys->wait, &p_sys->lock );
 
-        int canc = vlc_savecancel (); /* <- FIXME: should not be needed */
-        if( p_sys->b_update == true )
-        {
-            msg_Dbg( p_sd, "Update required" );
-            char* psz_urls = var_GetNonEmptyString( p_sd, "podcast-urls" );
-            if( psz_urls != NULL )
-                ParseUrls( p_sd, psz_urls );
-            free( psz_urls );
-            p_sys->b_update = false;
-        }
+        int canc = vlc_savecancel ();
+        msg_Dbg( p_sd, "Update required" );
+        char* psz_urls = var_GetNonEmptyString( p_sd, "podcast-urls" );
+        if( psz_urls != NULL )
+            ParseUrls( p_sd, psz_urls );
+        free( psz_urls );
+        p_sys->b_update = false;
 
         for( int i = 0; i < p_sd->p_sys->i_input; i++ )
         {
@@ -189,6 +198,8 @@ static void Run( services_discovery_t *p_sd )
         }
         vlc_restorecancel (canc);
     }
+    vlc_cleanup_pop();
+    assert(0); /* dead code */
 }
 
 static int UrlsChange( vlc_object_t *p_this, char const *psz_var,
@@ -198,7 +209,11 @@ static int UrlsChange( vlc_object_t *p_this, char const *psz_var,
     VLC_UNUSED(p_this); VLC_UNUSED(psz_var); VLC_UNUSED(oldval);
     VLC_UNUSED(newval);
     services_discovery_sys_t *p_sys  = (services_discovery_sys_t *)p_data;
+
+    vlc_mutex_lock( &p_sys->lock );
     p_sys->b_update = true;
+    vlc_cond_signal( &p_sys->wait );
+    vlc_mutex_unlock( &p_sys->lock );
     return VLC_SUCCESS;
 }