Cleanup interaction-capable interface registration

 * Make registration thread-safe, and independent on vlc_list_find().
 * Fix use-after-free race condition when destroying the interface
   (interaction will be lost instead).

include/vlc_interface.h

@@ -67,9 +67,6 @@ struct intf_thread_t
     void ( *pf_show_dialog ) ( intf_thread_t *, int, int,
                                intf_dialog_args_t * );
 
-    /** Interaction stuff */
-    bool b_interaction;
-
     vlc_mutex_t  change_lock;
 };
 
@@ -108,6 +105,9 @@ VLC_EXPORT( void,              intf_StopThread, ( intf_thread_t * ) );
 #define intf_Eject(a,b) __intf_Eject(VLC_OBJECT(a),b)
 VLC_EXPORT( int, __intf_Eject, ( vlc_object_t *, const char * ) );
 
+VLC_EXPORT( int, interaction_Register, ( intf_thread_t * ) );
+VLC_EXPORT( int, interaction_Unregister, ( intf_thread_t * ) );
+
 /*@}*/
 
 /*****************************************************************************

modules/gui/macosx/intf.m

@@ -409,7 +409,7 @@ static VLCMain *_o_sharedMainInstance = nil;
  
     var_Create( p_intf, "interaction", VLC_VAR_ADDRESS );
     var_AddCallback( p_intf, "interaction", InteractCallback, self );
-    p_intf->b_interaction = true;
+    interaction_Register( p_intf );
 
     /* update the playmode stuff */
     p_intf->p_sys->b_playmode_update = true;
@@ -685,7 +685,7 @@ static VLCMain *_o_sharedMainInstance = nil;
         [o_extended savePrefs];
     }
  
-    p_intf->b_interaction = false;
+    interaction_Unregister( p_intf );
     var_DelCallback( p_intf, "interaction", InteractCallback, self );
 
     /* remove global observer watching for vout device changes correctly */

modules/gui/qt4/main_interface.cpp

@@ -186,7 +186,7 @@ MainInterface::MainInterface( intf_thread_t *_p_intf ) : QVLCMW( _p_intf )
      ************/
     var_Create( p_intf, "interaction", VLC_VAR_ADDRESS );
     var_AddCallback( p_intf, "interaction", InteractCallback, this );
-    p_intf->b_interaction = true;
+    interaction_Register( p_intf );
 
     var_AddCallback( p_intf->p_libvlc, "intf-show", IntfShowCB, p_intf );
 
@@ -267,7 +267,7 @@ MainInterface::~MainInterface()
     /* Unregister callback for the intf-popupmenu variable */
     var_DelCallback( p_intf->p_libvlc, "intf-popupmenu", PopupMenuCB, p_intf );
 
-    p_intf->b_interaction = false;
+    interaction_Unregister( p_intf );
     var_DelCallback( p_intf, "interaction", InteractCallback, this );
 
     p_intf->p_sys->p_mi = NULL;

modules/gui/skins2/src/vlcproc.cpp

@@ -164,7 +164,7 @@ VlcProc::VlcProc( intf_thread_t *pIntf ): SkinObject( pIntf ),
     // Called when we have an interaction dialog to display
     var_Create( pIntf, "interaction", VLC_VAR_ADDRESS );
     var_AddCallback( pIntf, "interaction", onInteraction, this );
-    pIntf->b_interaction = true;
+    interaction_Register( pIntf );
 
     getIntf()->p_sys->p_input = NULL;
 }
@@ -179,6 +179,8 @@ VlcProc::~VlcProc()
         vlc_object_release( getIntf()->p_sys->p_input );
     }
 
+    interaction_Unregister( getIntf() );
+
     var_DelCallback( getIntf()->p_sys->p_playlist, "intf-change",
                      onIntfChange, this );
     var_DelCallback( getIntf()->p_sys->p_playlist, "item-append",

src/interface/interaction.c

@@ -47,7 +47,7 @@
  * Local prototypes
  *****************************************************************************/
 static interaction_t *          InteractionGet( vlc_object_t * );
-static void                     InteractionSearchInterface( interaction_t * );
+static intf_thread_t *          SearchInterface( interaction_t * );
 static void*                    InteractionLoop( vlc_object_t * );
 static void                     InteractionManage( interaction_t * );
 
@@ -401,6 +401,39 @@ void interaction_Destroy( interaction_t *p_interaction )
     vlc_object_release( p_interaction );
 }
 
+static vlc_mutex_t intf_lock = VLC_STATIC_MUTEX;
+
+int interaction_Register( intf_thread_t *intf )
+{
+    libvlc_priv_t *priv = libvlc_priv( intf->p_libvlc );
+    int ret = VLC_EGENERIC;
+
+    vlc_mutex_lock( &intf_lock );
+    if( priv->p_interaction_intf == NULL )
+    {   /* Since the interface is responsible for unregistering itself before
+         * it terminates, an object reference is not needed. */
+        priv->p_interaction_intf = intf;
+        ret = VLC_SUCCESS;
+    }
+    vlc_mutex_unlock( &intf_lock );
+    return ret;
+}
+
+int interaction_Unregister( intf_thread_t *intf )
+{
+    libvlc_priv_t *priv = libvlc_priv( intf->p_libvlc );
+    int ret = VLC_EGENERIC;
+
+    vlc_mutex_lock( &intf_lock );
+    if( priv->p_interaction_intf == intf )
+    {
+        priv->p_interaction_intf = NULL;
+        ret = VLC_SUCCESS;
+    }
+    vlc_mutex_unlock( &intf_lock );
+    return ret;
+}
+
 /**********************************************************************
  * The following functions are local
  **********************************************************************/
@@ -415,32 +448,19 @@ static interaction_t * InteractionGet( vlc_object_t *p_this )
 }
 
 
-/* Look for an interface suitable for interaction */
-static void InteractionSearchInterface( interaction_t *p_interaction )
+/* Look for an interface suitable for interaction, and hold it. */
+static intf_thread_t *SearchInterface( interaction_t *p_interaction )
 {
-    vlc_list_t  *p_list;
-    int          i_index;
+    libvlc_priv_t *priv = libvlc_priv( p_interaction->p_libvlc );
+    intf_thread_t *intf;
 
-    p_interaction->p_intf = NULL;
-
-    p_list = vlc_list_find( p_interaction, VLC_OBJECT_INTF, FIND_ANYWHERE );
-    if( !p_list )
-    {
-        msg_Err( p_interaction, "unable to create module list" );
-        return;
-    }
+    vlc_mutex_lock( &intf_lock );
+    intf = priv->p_interaction_intf;
+    if( intf != NULL )
+        vlc_object_hold( intf );
+    vlc_mutex_unlock( &intf_lock );
 
-    for( i_index = 0; i_index < p_list->i_count; i_index ++ )
-    {
-        intf_thread_t *p_intf = (intf_thread_t *)
-                                        p_list->p_values[i_index].p_object;
-        if( p_intf->b_interaction )
-        {
-            p_interaction->p_intf = p_intf;
-            break;
-        }
-    }
-    vlc_list_release ( p_list );
+    return intf;
 }
 
 /* Find an interaction dialog by its id */
@@ -595,7 +615,7 @@ static void InteractionManage( interaction_t *p_interaction )
     /* Nothing to do */
     if( p_interaction->i_dialogs == 0 ) return;
 
-    InteractionSearchInterface( p_interaction );
+    p_interaction->p_intf = SearchInterface( p_interaction );
     if( !p_interaction->p_intf )
     {
         /* We mark all dialogs as answered with their "default" answer */
@@ -611,8 +631,6 @@ static void InteractionManage( interaction_t *p_interaction )
                 p_dialog->i_status = HIDING_DIALOG;
         }
     }
-    else
-        vlc_object_hold( p_interaction->p_intf );
 
     for( i_index = 0 ; i_index < p_interaction->i_dialogs; i_index ++ )
     {

src/interface/interface.c

@@ -89,7 +89,6 @@ intf_thread_t* __intf_Create( vlc_object_t *p_this, const char *psz_module )
     p_intf = vlc_object_create( p_this, VLC_OBJECT_INTF );
     if( !p_intf )
         return NULL;
-    p_intf->b_interaction = false;
 #if defined( __APPLE__ ) || defined( WIN32 )
     p_intf->b_should_run_on_first_thread = false;
 #endif

src/libvlc.h

@@ -226,6 +226,7 @@ typedef struct libvlc_priv_t
     playlist_t        *p_playlist; //< the playlist singleton
     vlm_t             *p_vlm;  ///< the VLM singleton (or NULL)
     interaction_t     *p_interaction;    ///< interface interaction object
+    intf_thread_t     *p_interaction_intf; ///< XXX interface for interaction
     httpd_t           *p_httpd; ///< HTTP daemon (src/network/httpd.c)
 #ifdef ENABLE_SOUT
     sap_handler_t     *p_sap; ///< SAP SDP advertiser

src/libvlccore.sym

@@ -188,6 +188,8 @@ __input_Read
 input_SplitMRL
 input_StopThread
 input_vaControl
+interaction_Register
+interaction_Unregister
 __intf_Create
 __intf_Eject
 __intf_Progress