[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