[vlc-devel] [PATCH 2/9] dbus: Rework the main loop using a poll() based model
Mirsal Ennaime
mirsal at mirsal.fr
Tue Feb 8 22:35:57 CET 2011
From: Mirsal Ennaime <mirsal.ennaime at gmail.com>
---
modules/control/dbus/dbus.c | 437 ++++++++++++++++++++++++++++++++++-
modules/control/dbus/dbus_common.h | 4 +
2 files changed, 428 insertions(+), 13 deletions(-)
diff --git a/modules/control/dbus/dbus.c b/modules/control/dbus/dbus.c
index a16d920..45b47c9 100644
--- a/modules/control/dbus/dbus.c
+++ b/modules/control/dbus/dbus.c
@@ -56,8 +56,14 @@
#include <vlc_interface.h>
#include <vlc_playlist.h>
#include <vlc_meta.h>
+#include <vlc_mtime.h>
#include <assert.h>
+#include <string.h>
+
+#include <poll.h>
+#include <errno.h>
+#include <unistd.h>
/*****************************************************************************
* Local prototypes.
@@ -71,12 +77,33 @@ static int StateChange( intf_thread_t * );
static int TrackChange( intf_thread_t * );
static int AllCallback( vlc_object_t*, const char*, vlc_value_t, vlc_value_t, void* );
+static void dispatch_status_cb( DBusConnection *p_conn,
+ DBusDispatchStatus i_status,
+ void *p_data);
+
+static dbus_bool_t add_timeout ( DBusTimeout *p_timeout, void *p_data );
+static dbus_bool_t add_watch ( DBusWatch *p_watch, void *p_data );
+
+static void remove_timeout ( DBusTimeout *p_timeout, void *p_data );
+static void remove_watch ( DBusWatch *p_watch, void *p_data );
+
+static void timeout_toggled ( DBusTimeout *p_timeout, void *p_data );
+static void watch_toggled ( DBusWatch *p_watch, void *p_data );
+
+static void wakeup_main_loop( void *p_data );
+
typedef struct
{
int signal;
int i_node;
} callback_info_t;
+typedef struct
+{
+ mtime_t i_remaining;
+ DBusTimeout *p_timeout;
+} timeout_info_t;
+
/*****************************************************************************
* Module descriptor
*****************************************************************************/
@@ -112,12 +139,21 @@ static int Open( vlc_object_t *p_this )
if( !p_sys )
return VLC_ENOMEM;
+ dbus_threads_init_default();
+
p_sys->b_meta_read = false;
p_sys->i_caps = CAPS_NONE;
p_sys->b_dead = false;
p_sys->p_input = NULL;
p_sys->i_playing_state = -1;
+ if( pipe( &p_sys->i_pipe_fd_out ) )
+ {
+ free( p_sys );
+ msg_Err( p_intf, "Could not create pipe" );
+ return VLC_EGENERIC;
+ }
+
p_sys->b_unique = var_CreateGetBool( p_intf, "dbus-unique-service-id" );
if( p_sys->b_unique )
{
@@ -135,8 +171,10 @@ static int Open( vlc_object_t *p_this )
dbus_error_init( &error );
- /* connect to the session bus */
- p_conn = dbus_bus_get( DBUS_BUS_SESSION, &error );
+ /* connect privately to the session bus
+ * the connection will not be shared with other vlc modules which use dbus,
+ * thus avoiding a whole class of concurrency issues */
+ p_conn = dbus_bus_get_private( DBUS_BUS_SESSION, &error );
if( !p_conn )
{
msg_Err( p_this, "Failed to connect to the D-Bus session daemon: %s",
@@ -147,6 +185,8 @@ static int Open( vlc_object_t *p_this )
return VLC_EGENERIC;
}
+ dbus_connection_set_exit_on_disconnect( p_conn, FALSE );
+
/* register a well-known name on the bus */
dbus_bus_request_name( p_conn, psz_service_name, 0, &error );
if( dbus_error_is_set( &error ) )
@@ -175,6 +215,8 @@ static int Open( vlc_object_t *p_this )
p_intf->p_sys = p_sys;
p_sys->p_conn = p_conn;
p_sys->p_events = vlc_array_new();
+ p_sys->p_timeouts = vlc_array_new();
+ p_sys->p_watches = vlc_array_new();
vlc_mutex_init( &p_sys->lock );
p_playlist = pl_Get( p_intf );
@@ -188,6 +230,44 @@ static int Open( vlc_object_t *p_this )
var_AddCallback( p_playlist, "repeat", AllCallback, p_intf );
var_AddCallback( p_playlist, "loop", AllCallback, p_intf );
+ dbus_connection_set_dispatch_status_function( p_conn,
+ dispatch_status_cb,
+ p_intf, NULL );
+
+ if( !dbus_connection_set_timeout_functions( p_conn,
+ add_timeout,
+ remove_timeout,
+ timeout_toggled,
+ p_intf, NULL ) )
+ {
+ dbus_connection_set_dispatch_status_function( p_conn,
+ NULL, NULL, NULL );
+ dbus_connection_unref( p_conn );
+ free( psz_service_name );
+ free( p_sys );
+ return VLC_ENOMEM;
+ }
+
+ if( !dbus_connection_set_watch_functions( p_conn,
+ add_watch,
+ remove_watch,
+ watch_toggled,
+ p_intf, NULL ) )
+ {
+ dbus_connection_set_dispatch_status_function( p_conn,
+ NULL, NULL, NULL );
+ dbus_connection_set_timeout_functions( p_conn,
+ NULL, NULL, NULL, NULL, NULL );
+ dbus_connection_unref( p_conn );
+ free( psz_service_name );
+ free( p_sys );
+ return VLC_ENOMEM;
+ }
+
+/* dbus_connection_set_wakeup_main_function( p_conn,
+ wakeup_main_loop,
+ p_intf, NULL); */
+
UpdateCaps( p_intf );
return VLC_SUCCESS;
@@ -217,6 +297,9 @@ static void Close ( vlc_object_t *p_this )
vlc_object_release( p_sys->p_input );
}
+ /* The dbus connection is private, so we are responsible
+ * for closing it */
+ dbus_connection_close( p_sys->p_conn );
dbus_connection_unref( p_sys->p_conn );
// Free the events array
@@ -227,33 +310,253 @@ static void Close ( vlc_object_t *p_this )
}
vlc_mutex_destroy( &p_sys->lock );
vlc_array_destroy( p_sys->p_events );
+ vlc_array_destroy( p_sys->p_timeouts );
+ vlc_array_destroy( p_sys->p_watches );
free( p_sys );
}
+static void dispatch_status_cb( DBusConnection *p_conn,
+ DBusDispatchStatus i_status,
+ void *p_data)
+{
+ (void) p_conn;
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+
+ static const char *p_statuses[] = { "DATA_REMAINS",
+ "COMPLETE",
+ "NEED_MEMORY" };
+
+ msg_Dbg( p_intf,
+ "DBus dispatch status changed to %s.",
+ p_statuses[i_status]);
+}
+
+static dbus_bool_t add_timeout( DBusTimeout *p_timeout, void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+ intf_sys_t *p_sys = (intf_sys_t*) p_intf->p_sys;
+
+ timeout_info_t *p_info = calloc( sizeof( timeout_info_t ), 1 );
+ p_info->i_remaining = dbus_timeout_get_interval( p_timeout );
+ p_info->p_timeout = p_timeout;
+
+ dbus_timeout_set_data( p_timeout, p_info, free );
+
+ vlc_mutex_lock( &p_sys->lock );
+ vlc_array_append( p_sys->p_timeouts, p_timeout );
+ vlc_mutex_unlock( &p_sys->lock );
+
+ return TRUE;
+}
+
+static void remove_timeout( DBusTimeout *p_timeout, void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+ intf_sys_t *p_sys = (intf_sys_t*) p_intf->p_sys;
+
+ vlc_mutex_lock( &p_sys->lock );
+
+ vlc_array_remove( p_sys->p_timeouts,
+ vlc_array_index_of_item( p_sys->p_timeouts, p_timeout ) );
+
+ vlc_mutex_unlock( &p_sys->lock );
+}
+
+static void timeout_toggled( DBusTimeout *p_timeout, void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+
+ msg_Dbg( p_intf, "Toggling dbus timeout" );
+
+ if( dbus_timeout_get_enabled( p_timeout ) )
+ {
+ msg_Dbg( p_intf, "Timeout is enabled, main loop needs to wake up" );
+ wakeup_main_loop( p_intf );
+ }
+}
+
+static dbus_bool_t add_watch( DBusWatch *p_watch, void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+ intf_sys_t *p_sys = (intf_sys_t*) p_intf->p_sys;
+ int i_fd = dbus_watch_get_unix_fd( p_watch );
+
+ msg_Dbg( p_intf, "Adding dbus watch on fd %d", i_fd );
+
+ if( dbus_watch_get_flags( p_watch ) & DBUS_WATCH_READABLE )
+ msg_Dbg( p_intf, "Watching fd %d for readability", i_fd );
+
+ if( dbus_watch_get_flags( p_watch ) & DBUS_WATCH_WRITABLE )
+ msg_Dbg( p_intf, "Watching fd %d for writeability", i_fd );
+
+ vlc_mutex_lock( &p_sys->lock );
+ vlc_array_append( p_sys->p_watches, p_watch );
+ vlc_mutex_unlock( &p_sys->lock );
+
+ return TRUE;
+}
+
+static void remove_watch( DBusWatch *p_watch, void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+ intf_sys_t *p_sys = (intf_sys_t*) p_intf->p_sys;
+
+ msg_Dbg( p_intf, "Removing dbus watch on fd %d",
+ dbus_watch_get_unix_fd( p_watch ) );
+
+ vlc_mutex_lock( &p_sys->lock );
+
+ vlc_array_remove( p_sys->p_watches,
+ vlc_array_index_of_item( p_sys->p_watches, p_watch ) );
+
+ vlc_mutex_unlock( &p_sys->lock );
+}
+
+static void watch_toggled( DBusWatch *p_watch, void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+
+ msg_Dbg( p_intf, "Toggling dbus watch on fd %d",
+ dbus_watch_get_unix_fd( p_watch ) );
+
+ if( dbus_watch_get_enabled( p_watch ) )
+ {
+ msg_Dbg( p_intf,
+ "Watch on fd %d has been enabled, "
+ "the main loops needs to wake up",
+ dbus_watch_get_unix_fd( p_watch ) );
+
+ wakeup_main_loop( p_intf );
+ }
+}
+
/*****************************************************************************
* Run: main loop
*****************************************************************************/
static void Run ( intf_thread_t *p_intf )
{
+ intf_sys_t *p_sys = p_intf->p_sys;
+ mtime_t i_lastrun = mdate();
+ struct pollfd *p_fds = NULL;
+
for( ;; )
{
- if( dbus_connection_get_dispatch_status(p_intf->p_sys->p_conn)
- == DBUS_DISPATCH_COMPLETE )
- msleep( INTF_IDLE_SLEEP );
int canc = vlc_savecancel();
- dbus_connection_read_write_dispatch( p_intf->p_sys->p_conn, 0 );
+ vlc_mutex_lock( &p_sys->lock );
+
+ /* we build a pollfd array from:
+ * - the set of enabled dbus watches
+ * - the unix pipe which we use to manually wake up the main loop */
+ unsigned int i_watches = vlc_array_count( p_sys->p_watches );
+ p_fds = (struct pollfd*) calloc( sizeof( struct pollfd ), i_watches );
+
+ p_fds[0].fd = p_sys->i_pipe_fd_out;
+ p_fds[0].events = POLLIN | POLLPRI;
+
+ int i_fds = 1;
+ DBusWatch *p_watch = NULL;
+ for( unsigned int i = 0; i < i_watches; i++ )
+ {
+ p_watch = vlc_array_item_at_index( p_sys->p_watches, i );
+ if( !dbus_watch_get_enabled( p_watch ) )
+ continue;
+
+ p_fds[i_fds].fd = dbus_watch_get_unix_fd( p_watch );
+ int i_flags = dbus_watch_get_flags( p_watch );
+
+ if( i_flags & DBUS_WATCH_READABLE )
+ p_fds[i_fds].events |= POLLIN | POLLPRI;
+
+ if( i_flags & DBUS_WATCH_WRITABLE )
+ p_fds[i_fds].events |= POLLOUT;
+
+ i_fds++;
+ }
+
+ /* The correct poll timeout value is the shortest one
+ * in the dbus timeouts list */
+ mtime_t i_now = mdate(), i_next_timeout = LAST_MDATE;
+ msg_Dbg( p_intf, "%d µs elapsed since last wakeup", i_now - i_lastrun );
+ for( int i = 0; i < vlc_array_count( p_sys->p_timeouts ); i++ )
+ {
+ timeout_info_t *p_info = NULL;
+ DBusTimeout *p_timeout = NULL;
+ mtime_t i_interval = 0;
+
+ p_timeout = vlc_array_item_at_index( p_sys->p_timeouts, i );
+ i_interval = dbus_timeout_get_interval( p_timeout );
+ p_info = (timeout_info_t*) dbus_timeout_get_data( p_timeout );
+
+ p_info->i_remaining -= __MAX( 0, i_now - i_lastrun ) % i_interval;
+
+ if( !dbus_timeout_get_enabled( p_timeout ) )
+ continue;
- /* Get the list of events to process
+ i_next_timeout = __MIN( i_next_timeout,
+ __MAX( 0, p_info->i_remaining ) );
+ }
+
+ i_lastrun = i_now;
+ vlc_mutex_unlock( &p_sys->lock );
+
+ msg_Dbg( p_intf, "Sleeping until something happens, next timeout is in %d", i_next_timeout );
+
+ /* thread cancellation is allowed while the main loop sleeps */
+ vlc_restorecancel( canc );
+
+ int i_pollres = poll( p_fds, i_fds, i_next_timeout );
+ int i_errsv = errno;
+
+ canc = vlc_savecancel();
+
+ msg_Dbg( p_intf, "the main loop has been woken up" );
+
+ if( -1 == i_pollres )
+ { /* XXX: What should we do when poll() fails ? */
+ char buf[64];
+ msg_Err( p_intf, "poll() failed: %s", strerror_r( i_errsv, buf, 64 ) );
+ free( p_fds ); p_fds = NULL;
+ vlc_restorecancel( canc );
+ continue;
+ }
+
+ /* Was the main loop woken up manually ? */
+ if( 0 < i_pollres && ( p_fds[0].revents & POLLIN ) )
+ {
+ char buf;
+ msg_Dbg( p_intf, "Removing a byte from the self-pipe" );
+ read( p_fds[0].fd, &buf, 1 );
+ }
+
+ /* We need to lock the mutex while building lists of events,
+ * timeouts and watches to process but we can't keep the lock while
+ * processing them, or else we risk a deadlock:
*
- * 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 );
+
+ /* Get the list of timeouts to process */
+ unsigned int i_timeouts = vlc_array_count( p_sys->p_timeouts );
+ DBusTimeout **p_timeouts = calloc( sizeof( DBusTimeout* ), i_timeouts );
+ for( unsigned int i = 0; i < i_timeouts; i++ )
+ {
+ p_timeouts[i] = vlc_array_item_at_index( p_sys->p_timeouts, i );
+ }
+
+ /* Get the list of watches to process */
+ i_watches = vlc_array_count( p_sys->p_watches );
+ DBusWatch **p_watches = calloc( sizeof( DBusWatch* ), i_watches );
+ for( unsigned int i = 0; i < i_watches; i++ )
+ {
+ p_watches[i] = vlc_array_item_at_index( p_sys->p_watches, i );
+ }
+
+ /* Get the list of events to process */
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-- )
@@ -261,8 +564,11 @@ static void Run ( intf_thread_t *p_intf )
info[i] = vlc_array_item_at_index( p_intf->p_sys->p_events, i );
vlc_array_remove( p_intf->p_sys->p_events, i );
}
+
+ /* now we can release the lock and process what's pending */
vlc_mutex_unlock( &p_intf->p_sys->lock );
+ /* Process vlc events */
for( int i = 0; i < i_events; i++ )
{
switch( info[i]->signal )
@@ -288,10 +594,109 @@ static void Run ( intf_thread_t *p_intf )
}
free( info[i] );
}
+
+ /* Process watches */
+ for( unsigned int i = 0; i < i_watches; i++ )
+ {
+ DBusWatch *p_watch = p_watches[i];
+ if( !dbus_watch_get_enabled( p_watch ) )
+ continue;
+
+ for( int j = 0; j < i_fds; j++ )
+ {
+ if( p_fds[j].fd != dbus_watch_get_unix_fd( p_watch ) )
+ continue;
+
+ int i_flags = 0;
+ int i_revents = p_fds[j].revents;
+ int i_fd = p_fds[j].fd;
+
+ if( i_revents & POLLIN )
+ {
+ msg_Dbg( p_intf, "fd %d is ready for reading", i_fd );
+ i_flags |= DBUS_WATCH_READABLE;
+ }
+
+ if( i_revents & POLLOUT )
+ {
+ msg_Dbg( p_intf, "fd %d is ready for writing", i_fd );
+ i_flags |= DBUS_WATCH_WRITABLE;
+ }
+
+ if( i_revents & POLLERR )
+ {
+ msg_Dbg( p_intf, "error when polling fd %d", i_fd );
+ i_flags |= DBUS_WATCH_ERROR;
+ }
+
+ if( i_revents & POLLHUP )
+ {
+ msg_Dbg( p_intf, "Hangup signal on fd %d", i_fd );
+ i_flags |= DBUS_WATCH_HANGUP;
+ }
+
+ if( i_flags )
+ {
+ msg_Dbg( p_intf, "Handling dbus watch on fd %d", i_fd );
+ dbus_watch_handle( p_watch, i_flags );
+ }
+ else
+ msg_Dbg( p_intf, "Nothing happened on fd %d", i_fd );
+ }
+ }
+ free( p_fds ); p_fds = NULL;
+
+ /* Process timeouts */
+ for( unsigned int i = 0; i < i_timeouts; i++ )
+ {
+ timeout_info_t *p_info = NULL;
+
+ p_info = (timeout_info_t*) dbus_timeout_get_data( p_timeouts[i] );
+
+ if( !dbus_timeout_get_enabled( p_info->p_timeout ) )
+ continue;
+
+ if( p_info->i_remaining > 0 )
+ continue;
+
+ dbus_timeout_handle( p_info->p_timeout );
+ p_info->i_remaining = dbus_timeout_get_interval( p_info->p_timeout );
+ }
+
+ /* Dispatch incoming dbus messages */
+ DBusDispatchStatus status;
+ status = dbus_connection_get_dispatch_status( p_sys->p_conn );
+ while( status != DBUS_DISPATCH_COMPLETE )
+ {
+ msg_Dbg( p_intf, "Dispatching incoming dbus messages" );
+ dbus_connection_dispatch( p_sys->p_conn );
+ status = dbus_connection_get_dispatch_status( p_sys->p_conn );
+ }
+
+ /* Send outgoing data */
+ if( dbus_connection_has_messages_to_send( p_sys->p_conn ) )
+ {
+ msg_Dbg( p_intf, "Sending outgoing data" );
+ dbus_connection_flush( p_sys->p_conn );
+ }
+
vlc_restorecancel( canc );
}
}
+static void wakeup_main_loop( void *p_data )
+{
+ intf_thread_t *p_intf = (intf_thread_t*) p_data;
+
+ msg_Dbg( p_intf, "Sending wakeup signal to the main loop" );
+
+ if( !write( p_intf->p_sys->i_pipe_fd_in, "\0", 1 ) )
+ {
+ msg_Err( p_intf,
+ "Could not wake up the main loop: %s", strerror( errno ) );
+ }
+}
+
/*****************************************************************************
* UpdateCaps: update p_sys->i_caps
* This function have to be called with the playlist unlocked
@@ -343,6 +748,7 @@ static int AllCallback( vlc_object_t *p_this, const char *psz_var,
if( !info )
return VLC_ENOMEM;
+
vlc_mutex_lock( &p_intf->p_sys->lock );
// Wich event is it ?
@@ -370,8 +776,8 @@ static int AllCallback( vlc_object_t *p_this, const char *psz_var,
if( state == p_intf->p_sys->i_playing_state )
{
- free( info );
- goto end;
+ vlc_mutex_unlock( &p_intf->p_sys->lock );
+ return VLC_SUCCESS;
}
p_intf->p_sys->i_playing_state = state;
@@ -382,9 +788,14 @@ static int AllCallback( vlc_object_t *p_this, const char *psz_var,
// Append the event
vlc_array_append( p_intf->p_sys->p_events, info );
-
-end:
vlc_mutex_unlock( &p_intf->p_sys->lock );
+
+ msg_Dbg( p_intf,
+ "Got a VLC event on %s. The main loop needs to wake up "
+ "in order to process it", psz_var );
+
+ wakeup_main_loop( p_intf );
+
return VLC_SUCCESS;
}
diff --git a/modules/control/dbus/dbus_common.h b/modules/control/dbus/dbus_common.h
index e701a94..7a4c0de 100644
--- a/modules/control/dbus/dbus_common.h
+++ b/modules/control/dbus/dbus_common.h
@@ -89,6 +89,10 @@ struct intf_sys_t
dbus_int32_t i_playing_state;
bool b_dead;
vlc_array_t *p_events;
+ vlc_array_t *p_timeouts;
+ vlc_array_t *p_watches;
+ int i_pipe_fd_out;
+ int i_pipe_fd_in;
vlc_mutex_t lock;
input_thread_t *p_input;
bool b_unique;
--
1.7.1
More information about the vlc-devel
mailing list