Commit f193adfb authored by Rafaël Carré's avatar Rafaël Carré

dbus : fix a deadlock with p_events array

Mention that MPRIS is at version 1.0
Remove some trailing spaces
Remove unused argument 'lock' from MarshalStatus()
parent bad63db8
...@@ -32,7 +32,7 @@ ...@@ -32,7 +32,7 @@
* extract: * extract:
* "If you use this low-level API directly, you're signing up for some pain." * "If you use this low-level API directly, you're signing up for some pain."
* *
* MPRIS Specification (still drafting on Jan, 23 of 2008): * MPRIS Specification version 1.0
* http://wiki.xmms2.xmms.se/index.php/MPRIS * http://wiki.xmms2.xmms.se/index.php/MPRIS
*/ */
...@@ -73,7 +73,7 @@ static int TrackListChangeEmit( intf_thread_t *, int, int ); ...@@ -73,7 +73,7 @@ static int TrackListChangeEmit( intf_thread_t *, int, int );
static int AllCallback( vlc_object_t*, const char*, vlc_value_t, vlc_value_t, void* ); static int AllCallback( vlc_object_t*, const char*, vlc_value_t, vlc_value_t, void* );
static int GetInputMeta ( input_item_t *, DBusMessageIter * ); static int GetInputMeta ( input_item_t *, DBusMessageIter * );
static int MarshalStatus ( intf_thread_t *, DBusMessageIter *, bool ); static int MarshalStatus ( intf_thread_t *, DBusMessageIter * );
static int UpdateCaps( intf_thread_t* ); static int UpdateCaps( intf_thread_t* );
/* GetCaps() capabilities */ /* GetCaps() capabilities */
...@@ -311,7 +311,7 @@ DBUS_METHOD( GetStatus ) ...@@ -311,7 +311,7 @@ DBUS_METHOD( GetStatus )
REPLY_INIT; REPLY_INIT;
OUT_ARGUMENTS; OUT_ARGUMENTS;
MarshalStatus( p_this, &args, true ); MarshalStatus( p_this, &args );
REPLY_SEND; REPLY_SEND;
} }
...@@ -838,12 +838,28 @@ static void Run ( intf_thread_t *p_intf ) ...@@ -838,12 +838,28 @@ static void Run ( intf_thread_t *p_intf )
int canc = vlc_savecancel(); int canc = vlc_savecancel();
dbus_connection_read_write_dispatch( p_intf->p_sys->p_conn, 0 ); dbus_connection_read_write_dispatch( p_intf->p_sys->p_conn, 0 );
// Get the messages /* Get the list of events to process
*
* We can't keep the lock on p_intf->p_sys->p_events, else we risk a
* deadlock:
* The signal functions could lock mutex X while p_events is locked;
* While some other function in vlc (playlist) might lock mutex X
* and then set a variable which would call AllCallback(), which itself
* needs to lock p_events to add a new event.
*/
vlc_mutex_lock( &p_intf->p_sys->lock ); vlc_mutex_lock( &p_intf->p_sys->lock );
for( int i = vlc_array_count( p_intf->p_sys->p_events ) - 1; i >= 0; i-- ) int i_events = vlc_array_count( p_intf->p_sys->p_events );
callback_info_t* info[i_events];
for( int i = i_events - 1; i >= 0; i-- )
{ {
callback_info_t* info = vlc_array_item_at_index( p_intf->p_sys->p_events, i ); info[i] = vlc_array_item_at_index( p_intf->p_sys->p_events, i );
switch( info->signal ) vlc_array_remove( p_intf->p_sys->p_events, i );
}
vlc_mutex_unlock( &p_intf->p_sys->lock );
for( int i = 0; i < i_events; i++ )
{
switch( info[i]->signal )
{ {
case SIGNAL_ITEM_CURRENT: case SIGNAL_ITEM_CURRENT:
TrackChange( p_intf ); TrackChange( p_intf );
...@@ -851,7 +867,7 @@ static void Run ( intf_thread_t *p_intf ) ...@@ -851,7 +867,7 @@ static void Run ( intf_thread_t *p_intf )
case SIGNAL_INTF_CHANGE: case SIGNAL_INTF_CHANGE:
case SIGNAL_PLAYLIST_ITEM_APPEND: case SIGNAL_PLAYLIST_ITEM_APPEND:
case SIGNAL_PLAYLIST_ITEM_DELETED: case SIGNAL_PLAYLIST_ITEM_DELETED:
TrackListChangeEmit( p_intf, info->signal, info->i_node ); TrackListChangeEmit( p_intf, info[i]->signal, info[i]->i_node );
break; break;
case SIGNAL_RANDOM: case SIGNAL_RANDOM:
case SIGNAL_REPEAT: case SIGNAL_REPEAT:
...@@ -859,15 +875,13 @@ static void Run ( intf_thread_t *p_intf ) ...@@ -859,15 +875,13 @@ static void Run ( intf_thread_t *p_intf )
StatusChangeEmit( p_intf ); StatusChangeEmit( p_intf );
break; break;
case SIGNAL_STATE: case SIGNAL_STATE:
StateChange( p_intf, info->i_input_state ); StateChange( p_intf, info[i]->i_input_state );
break; break;
default: default:
assert(0); assert(0);
} }
free( info ); free( info[i] );
vlc_array_remove( p_intf->p_sys->p_events, i );
} }
vlc_mutex_unlock( &p_intf->p_sys->lock );
vlc_restorecancel( canc ); vlc_restorecancel( canc );
} }
} }
...@@ -1005,7 +1019,7 @@ DBUS_SIGNAL( StatusChangeSignal ) ...@@ -1005,7 +1019,7 @@ DBUS_SIGNAL( StatusChangeSignal )
/* we're called from a callback of input_thread_t, so it can not be /* we're called from a callback of input_thread_t, so it can not be
* destroyed before we return */ * destroyed before we return */
MarshalStatus( (intf_thread_t*) p_data, &args, false ); MarshalStatus( (intf_thread_t*) p_data, &args );
SIGNAL_SEND; SIGNAL_SEND;
} }
...@@ -1233,8 +1247,7 @@ static int GetInputMeta( input_item_t* p_input, ...@@ -1233,8 +1247,7 @@ static int GetInputMeta( input_item_t* p_input,
* MarshalStatus: Fill a DBusMessage with the current player status * MarshalStatus: Fill a DBusMessage with the current player status
*****************************************************************************/ *****************************************************************************/
static int MarshalStatus( intf_thread_t* p_intf, DBusMessageIter* args, static int MarshalStatus( intf_thread_t* p_intf, DBusMessageIter* args )
bool lock )
{ /* This is NOT the right way to do that, it would be better to sore { /* This is NOT the right way to do that, it would be better to sore
the status information in p_sys and update it on change, thus the status information in p_sys and update it on change, thus
avoiding a long lock */ avoiding a long lock */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment