[vlc-devel] commit: Podcast: thread safety fixes ( Rémi Denis-Courmont )

[git version control]

vlc | branch: master | Rémi Denis-Courmont <rdenis at simphalempin.com> | Sun Sep  7 14:06:14 2008 +0300| [20ad5a20371bd4b028c2fd8d6ae9a928878e5d06] | committer: Rémi Denis-Courmont 

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

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

 modules/services_discovery/podcast.c |   47 ++++++++++++++++++++++-----------
 1 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/modules/services_discovery/podcast.c b/modules/services_discovery/podcast.c
index 95d4ba6..d6d6a7a 100644
--- 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;
 }