[vlc-devel] [PATCH 2/3] win32: Add a vlc_mta_holder
Rémi Denis-Courmont
remi at remlab.net
Wed Jun 14 16:33:37 CEST 2017
On June 14, 2017 3:34:24 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 #5:
>- Use VLC_VAR_COMPARE_EXCHANGE to fix invalid static mutex
>
>Changes since previous patch #4:
>- Use a semaphore instead of a wait cond to signal the MTA has been
>acquired
>- Fix mta_holder structure cleanup
>
>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 | 164
>+++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 165 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..d818dc2d43
>--- /dev/null
>+++ b/src/win32/mta_holder.h
>@@ -0,0 +1,164 @@
>+/*****************************************************************************
>+ * 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 <vlc_atomic.h>
>+
>+#include <assert.h>
>+#include <windows.h>
>+#include <objbase.h>
>+
>+#ifdef __cplusplus
>+using std::atomic_uint;
>+#endif
>+
>+typedef struct vlc_mta_holder
>+{
>+ vlc_thread_t thread;
>+ vlc_mutex_t lock;
>+ vlc_sem_t sem;
>+ vlc_cond_t ready_cond;
>+ bool b_ready;
>+ atomic_uint i_refcount;
>+} vlc_mta_holder;
>+
>+static inline void* MtaMainLoop( void* opaque )
>+{
>+ vlc_mta_holder* p_mta = (vlc_mta_holder*)opaque;
>+ CoInitializeEx( NULL, COINIT_MULTITHREADED );
>+
>+ vlc_mutex_lock( &p_mta->lock );
>+ p_mta->b_ready = true;
>+ vlc_cond_signal( &p_mta->ready_cond );
>+ vlc_mutex_unlock( &p_mta->lock );
>+
>+ vlc_sem_wait( &p_mta->sem );
>+
>+ 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_mta_holder* p_mta = (vlc_mta_holder*)var_CreateGetAddress(
>p_parent->obj.libvlc, "mta-holder" );
>+
>+ bool force_var_change = false;
>+ if ( p_mta != NULL )
>+ {
>+ if( atomic_fetch_add( &p_mta->i_refcount, 1u ) == 0 )
>+ {
>+ /* The mta struct is being destroyed but the var is still
>present */
>+ force_var_change = true;
>+ }
>+ else
>+ goto wait_ready;
>+ }
>+
>+ p_mta = (vlc_mta_holder*) malloc( sizeof( *p_mta ) );
>+ if ( unlikely( p_mta == NULL ) )
>+ return NULL;
>+
>+ vlc_mutex_init( &p_mta->lock );
>+ vlc_sem_init( &p_mta->sem, 0 );
>+ vlc_cond_init( &p_mta->ready_cond );
>+ atomic_init( &p_mta->i_refcount, 1u );
>+ p_mta->b_ready = false;
>+
>+ if( force_var_change )
>+ var_SetAddress( p_parent->obj.libvlc, "mta-holder", p_mta );
>+ else
>+ {
>+ vlc_value_t newval = { .p_address = p_mta }, oldval = {
>.p_address = NULL };
>+
>+ if( var_Change( p_parent->obj.libvlc, "mta-holder",
>VLC_VAR_COMPARE_EXCHANGE,
>+ &oldval, &newval) != VLC_SUCCESS )
>+ {
>+ /* Worse case scenario: mta is already being initialized
>by an
>+ * other thread, try again */
>+
>+ vlc_sem_destroy( &p_mta->sem );
>+ vlc_cond_destroy( &p_mta->ready_cond );
>+ vlc_mutex_destroy( &p_mta->lock );
>+ free( p_mta );
>+
>+ /* It's OK to do a recursive call here: the variable is
>now created
>+ * by an other thread, therefore the next call won't go
>through
>+ * this code path */
>+ return vlc_mta_acquire( p_parent );
>+ }
>+ }
>+ if ( vlc_clone( &p_mta->thread, MtaMainLoop, p_mta,
>VLC_THREAD_PRIORITY_LOW ) )
>+ {
>+ vlc_sem_destroy( &p_mta->sem );
>+ vlc_cond_destroy( &p_mta->ready_cond );
>+ vlc_mutex_destroy( &p_mta->lock );
>+ free( p_mta );
>+ return NULL;
>+ }
>+
>+wait_ready:
>+ vlc_mutex_lock( &p_mta->lock );
>+ while( !p_mta->b_ready )
>+ vlc_cond_wait( &p_mta->ready_cond, &p_mta->lock );
>+ vlc_mutex_unlock( &p_mta->lock );
>+
>+ 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_mta_holder *p_mta = (vlc_mta_holder*)var_GetAddress(
>p_parent->obj.libvlc, "mta-holder" );
>+ assert( p_mta != NULL );
>+ if ( atomic_fetch_sub( &p_mta->i_refcount, 1u ) == 1 )
>+ {
>+ vlc_value_t newval = { .p_address = NULL }, oldval = {
>.p_address = p_mta };
>+ var_Change( p_parent->obj.libvlc, "mta-holder",
>VLC_VAR_COMPARE_EXCHANGE,
>+ &oldval, &newval);
>+
>+ vlc_sem_post( &p_mta->sem );
>+ vlc_join( p_mta->thread, NULL );
>+
>+ vlc_sem_destroy( &p_mta->sem );
>+ vlc_cond_destroy( &p_mta->ready_cond );
>+ vlc_mutex_destroy( &p_mta->lock );
>+ 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
I do not believe in atomic pointers. As far as I am aware, a pointer can be atomic only if the pointee is statically allocated. (And then, an enumeration may be better than a pointer.)
In any other cases, atomic pointer implies bug.
--
Rémi Denis-Courmont
More information about the vlc-devel
mailing list