[vlc-devel] [PATCH 1/2] oldrc: lock the player only once while processing commands

Thomas Guillem thomas at gllm.fr
Tue May 28 10:15:10 CEST 2019


And avoid racy states due to successive lock/unlock

Refs #22328
---
 modules/control/oldrc.c | 45 ++++++++++++++---------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/modules/control/oldrc.c b/modules/control/oldrc.c
index bcf9210c3f..4c0b9a0845 100644
--- a/modules/control/oldrc.c
+++ b/modules/control/oldrc.c
@@ -487,12 +487,11 @@ static void *Run( void *data )
         b_complete = ReadCommand( p_intf, p_buffer, &i_size );
         canc = vlc_savecancel( );
 
+        vlc_player_Lock(player);
         /* Manage the input part */
         if( item == NULL )
         {
-            vlc_player_Lock(player);
             item = vlc_player_HoldCurrentMedia(player);
-            vlc_player_Unlock(player);
             /* New input has been registered */
             if( item )
             {
@@ -502,7 +501,6 @@ static void *Run( void *data )
             }
         }
 
-        vlc_player_Lock(player);
         if( !vlc_player_IsStarted( player ) )
         {
             if (item)
@@ -514,13 +512,10 @@ static void *Run( void *data )
             p_sys->last_state = VLC_PLAYER_STATE_STOPPED;
             msg_rc( STATUS_CHANGE "( stop state: 0 )" );
         }
-        vlc_player_Unlock(player);
 
         if( item != NULL )
         {
-            vlc_player_Lock(player);
             enum vlc_player_state state = vlc_player_GetState(player);
-            vlc_player_Unlock(player);
 
             if (p_sys->last_state != state)
             {
@@ -545,9 +540,7 @@ static void *Run( void *data )
 
         if( item && b_showpos )
         {
-            vlc_player_Lock(player);
             i_newpos = 100 * vlc_player_GetPosition( player );
-            vlc_player_Unlock(player);
             if( i_oldpos != i_newpos )
             {
                 i_oldpos = i_newpos;
@@ -556,7 +549,11 @@ static void *Run( void *data )
         }
 
         /* Is there something to do? */
-        if( !b_complete ) continue;
+        if( !b_complete )
+        {
+            vlc_player_Unlock(player);
+            continue;
+        }
 
         /* Skip heading spaces */
         psz_cmd = p_buffer;
@@ -582,7 +579,10 @@ static void *Run( void *data )
         }
 
         if( !strcmp( psz_cmd, "quit" ) )
+        {
+            vlc_player_Unlock(player);
             libvlc_Quit( vlc_object_instance(p_intf) );
+        }
 
 #define VOID(name, func) \
         if (strcmp(psz_cmd, name) == 0) { \
@@ -703,26 +703,20 @@ static void *Run( void *data )
         }
         else if( !strcmp( psz_cmd, "get_time" ) )
         {
-            vlc_player_Lock(player);
             vlc_tick_t t = vlc_player_GetTime(player);
-            vlc_player_Unlock(player);
             if (t != VLC_TICK_INVALID)
                 msg_rc("%"PRIu64, SEC_FROM_VLC_TICK(t));
         }
         else if( !strcmp( psz_cmd, "get_length" ) )
         {
-            vlc_player_Lock(player);
             vlc_tick_t l = vlc_player_GetLength(player);
-            vlc_player_Unlock(player);
             if (l != VLC_TICK_INVALID)
                 msg_rc("%"PRIu64, SEC_FROM_VLC_TICK(l));
         }
         else if( !strcmp( psz_cmd, "get_title" ) )
         {
-            vlc_player_Lock(player);
             struct vlc_player_title const *title =
                 vlc_player_GetSelectedTitle(player);
-            vlc_player_Unlock(player);
             msg_rc("%s", title ? title->name : "");
         }
         else if( !strcmp( psz_cmd, "longhelp" ) || !strncmp( psz_cmd, "h", 1 )
@@ -765,6 +759,7 @@ static void *Run( void *data )
 
         /* Command processed */
         i_size = 0; p_buffer[0] = 0;
+        vlc_player_Unlock(player);
     }
 
     msg_rc( STATUS_CHANGE "( stop state: 0 )" );
@@ -918,7 +913,6 @@ static void Input( vlc_object_t *p_this, char const *psz_cmd,
     intf_thread_t *p_intf = (intf_thread_t*)p_this;
     vlc_player_t *player = vlc_playlist_GetPlayer(p_intf->p_sys->playlist);
 
-    vlc_player_Lock(player);
     if( vlc_player_IsPaused(player) &&
         ( strcmp( psz_cmd, "pause" ) != 0 ) && (strcmp( psz_cmd,"frame") != 0 ) )
         msg_rc( "%s", _("Press pause to continue.") );
@@ -1046,14 +1040,14 @@ static void Input( vlc_object_t *p_this, char const *psz_cmd,
         {
             int idx = atoi(newval.psz_string);
             if (idx < 0)
-                goto out;
+                return;
             size_t track_count = vlc_player_GetTrackCount(player, cat);
             if ((unsigned)idx >= track_count)
-                goto out;
+                return;
             struct vlc_player_track const *track =
                 vlc_player_GetTrackAt(player, cat, (size_t)idx);
             if (!track)
-                goto out;
+                return;
             vlc_player_SelectTrack(player, track->es_id);
         }
         else
@@ -1073,8 +1067,6 @@ static void Input( vlc_object_t *p_this, char const *psz_cmd,
             msg_rc("+----[ end of %s ]", name);
         }
     }
-out:
-    vlc_player_Unlock(player);
 }
 
 static void print_playlist(intf_thread_t *p_intf, vlc_playlist_t *playlist)
@@ -1103,13 +1095,11 @@ static void Playlist( vlc_object_t *p_this, char const *psz_cmd,
     vlc_playlist_t *playlist = p_intf->p_sys->playlist;
     vlc_player_t *player = vlc_playlist_GetPlayer(playlist);
 
-    vlc_playlist_Lock(playlist);
-
     if (vlc_playlist_GetCurrentIndex(playlist) != -1 &&
         vlc_player_IsPaused(player))
     {
         msg_rc("%s", _("Type 'pause' to continue."));
-        goto end;
+        return;
     }
 
     /* Parse commands that require a playlist */
@@ -1231,7 +1221,7 @@ static void Playlist( vlc_object_t *p_this, char const *psz_cmd,
             int ret = vlc_playlist_InsertOne(playlist, count, p_item);
             input_item_Release(p_item);
             if (ret != VLC_SUCCESS)
-                goto end;
+                return;
 
             if (!strcmp(psz_cmd, "add"))
                 vlc_playlist_PlayAt(playlist, count);
@@ -1295,9 +1285,6 @@ static void Playlist( vlc_object_t *p_this, char const *psz_cmd,
      */
     else
         msg_rc( "unknown command!" );
-
-end:
-    vlc_playlist_Unlock(playlist);
 }
 
 static void Intf( vlc_object_t *p_this, char const *psz_cmd,
@@ -1539,9 +1526,7 @@ out:
 static void Statistics( intf_thread_t *p_intf )
 {
     vlc_player_t *player = vlc_playlist_GetPlayer(p_intf->p_sys->playlist);
-    vlc_player_Lock(player);
     input_item_t *p_item = vlc_player_GetCurrentMedia(player);
-    vlc_player_Unlock(player);
 
     if (p_item == NULL)
         return;
-- 
2.20.1



More information about the vlc-devel mailing list