Podcast: thread safety fixes

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)).

diff --git a/modules/services_discovery/podcast.c b/modules/services_discovery/podcast.c
index 95d4ba6f68f36e059c4728848cbac884c999f97a..d6d6a7ac59fc6bd6a2482a357c6ac9c141c51dbd 100644 (file)
--- a/modules/services_discovery/podcast.c
+++ b/modules/services_discovery/podcast.c
@@ -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;
 }