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

[Rémi Denis-Courmont]

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