[vlc-commits] playlist: merge interface and playlist locks as one

[Rémi Denis-Courmont]

vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Mon Jan  6 10:32:07 2014 +0200| [a5056a2d8fb05af732914c37e6bd15f56a97aa9d] | committer: Rémi Denis-Courmont

playlist: merge interface and playlist locks as one

This restores symmetry in the playlist API, but most importantly, it
is necessary to fix some corner case races (in later commits).

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

 src/interface/interface.c        |   43 ++++++++++++++++++++++++++++++++++----
 src/libvlc.c                     |    4 ++--
 src/libvlc.h                     |    2 +-
 src/playlist/engine.c            |   22 +------------------
 src/playlist/playlist_internal.h |    1 +
 5 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/src/interface/interface.c b/src/interface/interface.c
index b059956..b24cd69 100644
--- a/src/interface/interface.c
+++ b/src/interface/interface.c
@@ -43,15 +43,22 @@
 #include <vlc_common.h>
 #include <vlc_modules.h>
 #include <vlc_interface.h>
+#include <vlc_playlist.h>
 #include "libvlc.h"
-
-#ifdef __APPLE__
-#include "../lib/libvlc_internal.h"
-#endif
+#include "playlist/playlist_internal.h"
 
 static int AddIntfCallback( vlc_object_t *, char const *,
                             vlc_value_t , vlc_value_t , void * );
 
+/* This lock ensures that the playlist is created only once (per instance). It
+ * also protects the list of running interfaces against concurrent access,
+ * either to add or remove an interface.
+ *
+ * However, it does NOT protect from destruction of the playlist by
+ * intf_DestroyAll(). Instead, care must be taken that intf_Create() and any
+ * other function that depends on the playlist is only called BEFORE
+ * intf_DestroyAll() has the possibility to destroy all interfaces.
+ */
 static vlc_mutex_t lock = VLC_STATIC_MUTEX;
 
 #undef intf_Create
@@ -129,6 +136,34 @@ error:
     return VLC_EGENERIC;
 }
 
+/**
+ * Creates the playlist if necessary, and return a pointer to it.
+ * @note The playlist is not reference-counted. So the pointer is only valid
+ * until intf_DestroyAll() destroys interfaces.
+ */
+static playlist_t *intf_GetPlaylist(libvlc_int_t *libvlc)
+{
+    playlist_t *playlist;
+
+    vlc_mutex_lock(&lock);
+    playlist = libvlc_priv(libvlc)->playlist;
+    if (playlist == NULL)
+    {
+        playlist = playlist_Create(VLC_OBJECT(libvlc));
+        libvlc_priv(libvlc)->playlist = playlist;
+    }
+    vlc_mutex_unlock(&lock);
+
+    return playlist;
+}
+
+playlist_t *(pl_Get)(vlc_object_t *obj)
+{
+    playlist_t *pl = intf_GetPlaylist(obj->p_libvlc);
+    if (unlikely(pl == NULL))
+        abort();
+    return pl;
+}
 
 /**
  * Stops and destroys all interfaces
diff --git a/src/libvlc.c b/src/libvlc.c
index 7dbda91..d6c63ad 100644
--- a/src/libvlc.c
+++ b/src/libvlc.c
@@ -101,7 +101,7 @@ libvlc_int_t * libvlc_InternalCreate( void )
         return NULL;
 
     priv = libvlc_priv (p_libvlc);
-    priv->p_playlist = NULL;
+    priv->playlist = NULL;
     priv->p_dialog_provider = NULL;
     priv->p_vlm = NULL;
 
@@ -536,7 +536,7 @@ void libvlc_InternalCleanup( libvlc_int_t *p_libvlc )
 #endif
 
     /* Free playlist now, all threads are gone */
-    playlist_t *p_playlist = libvlc_priv (p_libvlc)->p_playlist;
+    playlist_t *p_playlist = libvlc_priv (p_libvlc)->playlist;
     if( p_playlist != NULL )
         playlist_Destroy( p_playlist );
 
diff --git a/src/libvlc.h b/src/libvlc.h
index a9c49da..d32976a 100644
--- a/src/libvlc.h
+++ b/src/libvlc.h
@@ -157,12 +157,12 @@ typedef struct libvlc_priv_t
     bool               b_stats;     ///< Whether to collect stats
 
     /* Singleton objects */
-    playlist_t        *p_playlist; ///< the playlist singleton
     vlm_t             *p_vlm;  ///< the VLM singleton (or NULL)
     vlc_object_t      *p_dialog_provider; ///< dialog provider
 #ifdef ENABLE_SOUT
     sap_handler_t     *p_sap; ///< SAP SDP advertiser
 #endif
+    struct playlist_t *playlist; ///< Playlist for interfaces
     struct playlist_preparser_t *parser; ///< Input item meta data handler
     struct vlc_actions *actions; ///< Hotkeys handler
 
diff --git a/src/playlist/engine.c b/src/playlist/engine.c
index 4861602..d9af6f0 100644
--- a/src/playlist/engine.c
+++ b/src/playlist/engine.c
@@ -195,7 +195,7 @@ static int VideoSplitterCallback( vlc_object_t *p_this, char const *psz_cmd,
  * \param p_parent the vlc object that is to be the parent of this playlist
  * \return a pointer to the created playlist, or NULL on error
  */
-static playlist_t *playlist_Create( vlc_object_t *p_parent )
+playlist_t *playlist_Create( vlc_object_t *p_parent )
 {
     playlist_t *p_playlist;
     playlist_private_t *p;
@@ -374,26 +374,6 @@ void playlist_Destroy( playlist_t *p_playlist )
     vlc_object_release( p_playlist );
 }
 
-#undef pl_Get
-playlist_t *pl_Get (vlc_object_t *obj)
-{
-    static vlc_mutex_t lock = VLC_STATIC_MUTEX;
-    libvlc_int_t *p_libvlc = obj->p_libvlc;
-    playlist_t *pl;
-
-    vlc_mutex_lock (&lock);
-    pl = libvlc_priv (p_libvlc)->p_playlist;
-    if (unlikely(pl == NULL))
-    {
-        pl = playlist_Create (VLC_OBJECT(p_libvlc));
-        if (unlikely(pl == NULL))
-            abort();
-        libvlc_priv (p_libvlc)->p_playlist = pl;
-    }
-    vlc_mutex_unlock (&lock);
-    return pl;
-}
-
 /** Get current playing input.
  */
 input_thread_t * playlist_CurrentInput( playlist_t * p_playlist )
diff --git a/src/playlist/playlist_internal.h b/src/playlist/playlist_internal.h
index 628fc56..2fbaaac 100644
--- a/src/playlist/playlist_internal.h
+++ b/src/playlist/playlist_internal.h
@@ -99,6 +99,7 @@ typedef struct playlist_private_t
  *****************************************************************************/
 
 /* Creation/Deletion */
+playlist_t *playlist_Create( vlc_object_t * );
 void playlist_Destroy( playlist_t * );
 void playlist_Activate( playlist_t * );