playlist: Never delete the playlist_item directly. We don't know who might need it.

We need to rework modules/playlist and implement refcounting or proper id management.

include/vlc_playlist.h

@@ -168,6 +168,8 @@ struct playlist_t
 
     playlist_item_array_t items; /**< Arrays of items */
     playlist_item_array_t all_items; /**< Array of items and nodes */
+    playlist_item_array_t items_to_delete; /**< Array of items and nodes to
+            delete... At the very end. This sucks. */
 
     playlist_item_array_t current; /**< Items currently being played */
     int                   i_current_index; /**< Index in current array */

src/playlist/engine.c

@@ -85,6 +85,7 @@ playlist_t * playlist_Create( vlc_object_t *p_parent )
 
     ARRAY_INIT( p_playlist->items );
     ARRAY_INIT( p_playlist->all_items );
+    ARRAY_INIT( p_playlist->items_to_delete );
     ARRAY_INIT( p_playlist->current );
 
     p_playlist->i_current_index = 0;
@@ -305,9 +306,8 @@ void set_current_status_item( playlist_t * p_playlist,
         p_playlist->status.p_item->i_flags & PLAYLIST_REMOVE_FLAG &&
         p_playlist->status.p_item != p_item )
     {
-         PL_DEBUG( "%s was marked for deletion, deleting",
-                         PLI_NAME( p_playlist->status.p_item  ) );
-         playlist_ItemDelete( p_playlist->status.p_item );
+        /* It's unsafe given current design to delete a playlist item :(
+        playlist_ItemDelete( p_playlist->status.p_item ); */
     }
     p_playlist->status.p_item = p_item;
 }
@@ -321,9 +321,8 @@ void set_current_status_node( playlist_t * p_playlist,
         p_playlist->status.p_node->i_flags & PLAYLIST_REMOVE_FLAG &&
         p_playlist->status.p_node != p_node )
     {
-         PL_DEBUG( "%s was marked for deletion, deleting",
-                         PLI_NAME( p_playlist->status.p_node  ) );
-         playlist_ItemDelete( p_playlist->status.p_node );
+        /* It's unsafe given current design to delete a playlist item :(
+        playlist_ItemDelete( p_playlist->status.p_node ); */
     }
     p_playlist->status.p_node = p_node;
 }
@@ -542,6 +541,12 @@ void playlist_LastLoop( playlist_t *p_playlist )
         free( p_del );
     FOREACH_END();
     ARRAY_RESET( p_playlist->all_items );
+    FOREACH_ARRAY( playlist_item_t *p_del, p_playlist->items_to_delete )
+        free( p_del->pp_children );
+        vlc_gc_decref( p_del->p_input );
+        free( p_del );
+    FOREACH_END();
+    ARRAY_RESET( p_playlist->items_to_delete );
 
     ARRAY_RESET( p_playlist->items );
     ARRAY_RESET( p_playlist->current );

src/playlist/item.c

@@ -195,18 +195,21 @@ playlist_item_t * playlist_ItemNewWithType( playlist_t *p_playlist,
  ***************************************************************************/
 
 /**
- * Delete item
+ * Release an item
  *
- * Delete a playlist item and detach its input item
  * \param p_item item to delete
  * \return VLC_SUCCESS
 */
-int playlist_ItemDelete( playlist_item_t *p_item )
+int playlist_ItemRelease( playlist_item_t *p_item )
 {
-    uninstall_input_item_observer( p_item );
-
-    vlc_gc_decref( p_item->p_input );
-    free( p_item );
+    /* Surprise, we can't actually do more because we
+     * don't do refcounting, or eauivalent.
+     * Because item are not only accessed by their id
+     * using playlist_item outside the PL_LOCK isn't safe.
+     * Most of the modules does that.
+     *
+     * Who wants to add proper memory management? */
+    ARRAY_APPEND( p_item->p_playlist->items_to_delete, p_item);
     return VLC_SUCCESS;
 }
 
@@ -863,7 +866,6 @@ static int DeleteInner( playlist_t * p_playlist, playlist_item_t *p_item,
 {
     int i;
     int i_id = p_item->i_id;
-    bool b_delay_deletion = false;
 
     if( p_item->i_children > -1 )
     {
@@ -893,7 +895,6 @@ static int DeleteInner( playlist_t * p_playlist, playlist_item_t *p_item,
             msg_Info( p_playlist, "stopping playback" );
             vlc_object_signal_maybe( VLC_OBJECT(p_playlist) );
         }
-        b_delay_deletion = true;
     }
 
     PL_DEBUG( "deleting item `%s'", p_item->p_input->psz_name );
@@ -901,13 +902,7 @@ static int DeleteInner( playlist_t * p_playlist, playlist_item_t *p_item,
     /* Remove the item from its parent */
     playlist_NodeRemoveItem( p_playlist, p_item, p_item->p_parent );
 
-    if( !b_delay_deletion )
-        playlist_ItemDelete( p_item );
-    else
-    {
-        PL_DEBUG( "marking %s for further deletion", PLI_NAME( p_item ) );
-        p_item->i_flags |= PLAYLIST_REMOVE_FLAG;
-    }
+    playlist_ItemRelease( p_item );
 
     return VLC_SUCCESS;
 }

src/playlist/playlist_internal.h

@@ -108,7 +108,7 @@ playlist_item_t *playlist_ItemFindFromInputAndRoot( playlist_t *p_playlist,
 
 int playlist_DeleteFromInputInParent( playlist_t *, int, playlist_item_t *, bool );
 int playlist_DeleteFromItemId( playlist_t*, int );
-int playlist_ItemDelete ( playlist_item_t * );
+int playlist_ItemRelease( playlist_item_t * );
 
 void playlist_release_current_input( playlist_t * p_playlist );
 void playlist_set_current_input(

src/playlist/tree.c

@@ -173,24 +173,7 @@ int playlist_NodeDelete( playlist_t *p_playlist, playlist_item_t *p_root,
         if( p_root->p_parent )
             playlist_NodeRemoveItem( p_playlist, p_root, p_root->p_parent );
 
-        /* Check if it is the current node */
-        if( p_playlist->status.p_node == p_root )
-        {
-            /* Hack we don't call playlist_Control for lock reasons */
-            p_playlist->request.i_status = PLAYLIST_STOPPED;
-            p_playlist->request.b_request = true;
-            p_playlist->request.p_item = NULL;
-            p_playlist->request.p_node = NULL;
-            msg_Info( p_playlist, "stopping playback" );
-            vlc_object_signal_maybe( VLC_OBJECT(p_playlist) );
-
-            PL_DEBUG( "marking %s for further deletion", PLI_NAME( p_root ) );
-            p_root->i_flags |= PLAYLIST_REMOVE_FLAG;
-        }
-        else
-            playlist_ItemDelete( p_root );
-
-
+        playlist_ItemRelease( p_root );
     }
     return VLC_SUCCESS;
 }