[vlc-devel] [PATCH 1/2] win32: Add a vlc_mta_holder

Hugo Beauzée-Luyssen hugo at beauzee.fr
Tue Jun 13 11:50:54 CEST 2017



On Tue, Jun 13, 2017, at 11:49 AM, Rémi Denis-Courmont wrote:
> On June 13, 2017 12:16:37 PM GMT+03:00, "Hugo Beauzée-Luyssen"
> <hugo at beauzee.fr> wrote:
> >This small helper will hold on to the MTA to ensure it won't be
> >destroyed between two calls to CoInitializeEx.
> >This will help us not leaking our TA configurations to
> >libvlc/libvlccore
> >threads
> >
> >--
> >Changes since previous patch #3:
> >- Fix a race condition causing the MTA to potentially not be acquired
> >by
> >  the time vlc_mta_acquire releases.
> >
> >Changes since previous patch #2:
> >- Don't use a VLC_OBJECT
> >- Use static inline functions instead of exported ones
> >
> >Changes since previous patch:
> >- Fix potential race condition
> >- Do not export the functions on !_WIN32
> >---
> > src/Makefile.am        |   2 +-
> >src/win32/mta_holder.h | 128
> >+++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 129 insertions(+), 1 deletion(-)
> > create mode 100644 src/win32/mta_holder.h
> >
> >diff --git a/src/Makefile.am b/src/Makefile.am
> >index 5f3726c3e6..51062532ca 100644
> >--- a/src/Makefile.am
> >+++ b/src/Makefile.am
> >@@ -467,7 +467,7 @@ libvlccore_la_LDFLAGS = \
> > libvlccore_la_DEPENDENCIES = libvlccore.sym
> > if HAVE_WIN32
> > libvlccore_la_DEPENDENCIES += libvlc_win32_rc.$(OBJEXT)
> >-libvlccore_la_LDFLAGS += -Wl,libvlc_win32_rc.$(OBJEXT) -avoid-version
> >-Wc,-static
> >+libvlccore_la_LDFLAGS += -Wl,libvlc_win32_rc.$(OBJEXT) -avoid-version
> >-Wc,-static $(LIBCOM)
> > endif
> > if HAVE_OS2
> > libvlccore_la_LDFLAGS += -avoid-version
> >diff --git a/src/win32/mta_holder.h b/src/win32/mta_holder.h
> >new file mode 100644
> >index 0000000000..e1a53c6632
> >--- /dev/null
> >+++ b/src/win32/mta_holder.h
> >@@ -0,0 +1,128 @@
> >+/*****************************************************************************
> >+ * mta_holder.c: Hold a MTA from another thread
> >+
> >*****************************************************************************
> >+ * Copyright (C) 2002-2017 the VideoLAN and AUTHORS
> >+ *
> >+ * Author: Hugo Beauzée-Luyssen <hugo at beauzee.fr>
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >modify
> >+ * it under the terms of the GNU General Public License as published
> >by
> >+ * the Free Software Foundation; either version 2 of the License, or
> >+ * (at your option) any later version.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA
> >02110-1301, USA.
> >+
> >*****************************************************************************/
> >+
> >+#ifndef MTA_HOLDER_H
> >+# define MTA_HOLDER_H
> >+
> >+#include <vlc_common.h>
> >+
> >+#include <assert.h>
> >+#include <windows.h>
> >+#include <objbase.h>
> >+
> >+typedef struct vlc_mta_holder
> >+{
> >+    vlc_thread_t thread;
> >+    vlc_cond_t   cond;
> >+    int          i_refcount;
> >+    bool         b_ready;
> >+} vlc_mta_holder;
> >+
> >+static vlc_mutex_t s_mutex = VLC_STATIC_MUTEX;
> >+
> >+static inline void* MtaMainLoop( void* opaque )
> >+{
> >+    vlc_mta_holder* p_mta = (vlc_mta_holder*)opaque;
> >+    CoInitializeEx( NULL, COINIT_MULTITHREADED );
> >+    vlc_mutex_lock( &s_mutex );
> >+
> >+    // Signal we now hold a reference to the MTA
> >+    p_mta->b_ready = true;
> >+    vlc_cond_signal( &p_mta->cond );
> >+
> >+    while( p_mta->i_refcount != 0 )
> >+    {
> >+        // And wait for all references to be released
> >+        vlc_cond_wait( &p_mta->cond, &s_mutex );
> >+    }
> >+    vlc_mutex_unlock( &s_mutex );
> >+    CoUninitialize();
> >+    return NULL;
> >+}
> >+
> >+/**
> >+ * Ensure an MTA context will be available until vlc_mta_release gets
> >called.
> >+ *
> >+ * In the background, this will create a thread that does nothing but
> >to keep the MTA
> >+ * refcount greater than 0.
> >+ *
> >+ * This is usefull in order not to commit a thread to a specific
> >concurrency model.
> >+ * This function is win32 specific.
> >+ */
> >+static inline vlc_mta_holder* vlc_mta_acquire( vlc_object_t *p_parent
> >)
> >+{
> >+    vlc_mutex_lock( &s_mutex );
> >+    vlc_mta_holder* p_mta = (vlc_mta_holder*)var_CreateGetAddress(
> >p_parent->obj.libvlc, "mta-holder" );
> >+    if ( p_mta == NULL )
> >+    {
> >+        p_mta = (vlc_mta_holder*)malloc( sizeof( *p_mta ) );
> >+        if ( unlikely( p_mta == NULL ) )
> >+        {
> >+            vlc_mutex_unlock( &s_mutex );
> >+            return NULL;
> >+        }
> >+        vlc_cond_init( &p_mta->cond );
> >+        p_mta->i_refcount = 1;
> >+        p_mta->b_ready = false;
> >+        if ( vlc_clone( &p_mta->thread, MtaMainLoop, p_mta,
> >VLC_THREAD_PRIORITY_LOW ) )
> >+        {
> >+            vlc_cond_destroy( &p_mta->cond );
> >+            free( p_mta );
> >+            p_mta = NULL;
> >+            vlc_mutex_unlock( &s_mutex );
> >+            return NULL;
> >+        }
> >+        var_SetAddress( p_parent->obj.libvlc, "mta-holder", p_mta );
> >+        while ( p_mta->b_ready == false )
> >+            vlc_cond_wait( &p_mta->cond, &s_mutex );
> >+    }
> >+    else
> >+        ++p_mta->i_refcount;
> >+    vlc_mutex_unlock( &s_mutex );
> >+    return p_mta;
> >+}
> >+
> >+/**
> >+ * Releases a reference to the MTA holder.
> >+ *
> >+ * When its refcount reaches 0, the thread created by
> >+ */
> >+static inline void vlc_mta_release( vlc_object_t* p_parent )
> >+{
> >+    vlc_mutex_lock( &s_mutex );
> >+    vlc_mta_holder *p_mta = (vlc_mta_holder*)var_InheritAddress(
> >p_parent->obj.libvlc, "mta-holder" );
> >+    assert( p_mta != NULL );
> >+    int i_refcount = --p_mta->i_refcount;
> >+    if ( i_refcount == 0 )
> >+        var_SetAddress( p_parent->obj.libvlc, "mta-holder", NULL );
> >+    vlc_mutex_unlock( &s_mutex );
> >+    if ( i_refcount == 0 )
> >+    {
> >+        vlc_cond_signal( &p_mta->cond );
> >+
> >+        vlc_join( p_mta->thread, NULL );
> >+
> >+        free( p_mta );
> >+    }
> >+}
> >+
> >+#endif
> >-- 
> >2.11.0
> >
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
> 
> Same condvar problem as the background meta worker, which was merged
> buggy.
> -- 
> Rémi Denis-Courmont

Could you clarify the race/issue here? I must say I don't see it.

Regards,

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr


More information about the vlc-devel mailing list