* include/vlc_threads.h,include/main.h,src/misc/threads.c,src/misc/win32_specific.c:
reverted Win32 pthread implementation to the old code. Fixed vlc_cond_broadcast() for
WinNT/2K/XP. Additional vlc_cond_* implementations for Win9x.
* src/libvlc.h: renamed --fast_pthread option into --fast-mutex. Added a
--win9x-cv-method option to choose which vlc_cond_* implementation we want on Win9x.

include/main.h

@@ -3,7 +3,7 @@
  * Declaration and extern access to global program object.
  *****************************************************************************
  * Copyright (C) 1999, 2000, 2001, 2002 VideoLAN
- * $Id: main.h,v 1.40 2002/07/20 18:01:41 sam Exp $
+ * $Id: main.h,v 1.41 2002/07/29 19:05:47 gbazin Exp $
  *
  * Authors: Vincent Seguin <seguin@via.ecp.fr>
  *
@@ -83,7 +83,8 @@ struct vlc_t
     vlc_object_t *         p_appthread;
 #elif defined( WIN32 )
     SIGNALOBJECTANDWAIT    SignalObjectAndWait;
-    vlc_bool_t             b_fast_pthread;
+    vlc_bool_t             b_fast_mutex;
+    int                    i_win9x_cv;
 #endif
 };
 

include/vlc_threads.h

@@ -3,7 +3,7 @@
  * This header provides a portable threads implementation.
  *****************************************************************************
  * Copyright (C) 1999, 2000 VideoLAN
- * $Id: vlc_threads.h,v 1.5 2002/07/16 21:29:10 sam Exp $
+ * $Id: vlc_threads.h,v 1.6 2002/07/29 19:05:47 gbazin Exp $
  *
  * Authors: Jean-Marc Dressler <polux@via.ecp.fr>
  *          Samuel Hocevar <sam@via.ecp.fr>
@@ -116,14 +116,14 @@ typedef struct
 
 typedef struct
 {
-    int                 i_waiting_threads;
+    volatile int        i_waiting_threads;
     /* WinNT/2K/XP implementation */
-    HANDLE              semaphore;
-    HANDLE              signal;
-    vlc_bool_t          b_broadcast;
+    HANDLE              event;
     SIGNALOBJECTANDWAIT SignalObjectAndWait;
     /* Win95/98/ME implementation */
-    HANDLE              p_events[2];
+    HANDLE              semaphore;
+    CRITICAL_SECTION    csection;
+    int                 i_win9x_cv;
 } vlc_cond_t;
 
 #elif defined( PTHREAD_COND_T_IN_PTHREAD_H )
@@ -361,15 +361,39 @@ static inline int vlc_cond_signal( vlc_cond_t *p_condvar )
     /* Release one waiting thread if one is available. */
     /* For this trick to work properly, the vlc_cond_signal must be surrounded
      * by a mutex. This will prevent another thread from stealing the signal */
-    if( p_condvar->i_waiting_threads )
+    if( !p_condvar->semaphore )
     {
-        if( p_condvar->signal )
+        PulseEvent( p_condvar->event );
+    }
+    else if( p_condvar->i_win9x_cv == 1 )
+    {
+        /* Wait for the gate to be open */
+        WaitForSingleObject( p_condvar->event, INFINITE );
+
+        if( p_condvar->i_waiting_threads )
         {
+            /* Using a semaphore exposes us to a race condition. It is
+             * possible for another thread to start waiting on the semaphore
+             * just after we signaled it and thus steal the signal.
+             * We have to prevent new threads from entering the cond_wait(). */
+            ResetEvent( p_condvar->event );
+
+            /* A semaphore is used here because Win9x doesn't have
+             * SignalObjectAndWait() and thus a race condition exists
+             * during the time we release the mutex and the time we start
+             * waiting on the event (more precisely, the signal can sometimes
+             * be missed by the waiting thread if we use PulseEvent()). */
             ReleaseSemaphore( p_condvar->semaphore, 1, 0 );
         }
-        else
+    }
+    else
+    {
+        if( p_condvar->i_waiting_threads )
         {
-            SetEvent( p_condvar->p_events[0/*signal*/] );
+            ReleaseSemaphore( p_condvar->semaphore, 1, 0 );
+
+            /* Wait for the last thread to be awakened */
+            WaitForSingleObject( p_condvar->event, INFINITE );
         }
     }
     return 0;
@@ -442,23 +466,36 @@ static inline int vlc_cond_broadcast( vlc_cond_t *p_condvar )
 
 #elif defined( WIN32 )
     /* Release all waiting threads. */
-    if( p_condvar->i_waiting_threads )
+    int i;
+
+    if( !p_condvar->semaphore )
+        for( i = p_condvar->i_waiting_threads; i > 0; i-- )
+            PulseEvent( p_condvar->event );
+    else if( p_condvar->i_win9x_cv == 1 )
     {
-        if( p_condvar->signal )
+        /* Wait for the gate to be open */
+        WaitForSingleObject( p_condvar->event, INFINITE );
+
+        if( p_condvar->i_waiting_threads )
         {
-            p_condvar->b_broadcast = 1;
-            /* This call is atomic */
+            /* close gate */
+            ResetEvent( p_condvar->event );
+
             ReleaseSemaphore( p_condvar->semaphore,
                               p_condvar->i_waiting_threads, 0 );
-            /* Wait for all threads to get the semaphore */
-            WaitForSingleObject( p_condvar->signal, INFINITE );
-            p_condvar->b_broadcast = 0;
         }
-        else
+    }
+    else
+    {
+        if( p_condvar->i_waiting_threads )
         {
-            SetEvent( p_condvar->p_events[1/*broadcast*/] );
+            ReleaseSemaphore( p_condvar->semaphore,
+                              p_condvar->i_waiting_threads, 0 );
+            /* Wait for the last thread to be awakened */
+            WaitForSingleObject( p_condvar->event, INFINITE );
         }
     }
+
     return 0;
 
 #elif defined( PTHREAD_COND_T_IN_PTHREAD_H )
@@ -538,56 +575,76 @@ static inline int __vlc_cond_wait( char * psz_file, int i_line,
     return i_ret;
 
 #elif defined( WIN32 )
-    /* Increase our wait count */
-    p_condvar->i_waiting_threads++;
-
-    if( p_condvar->signal )
+    if( !p_condvar->semaphore )
     {
+        /* Increase our wait count */
+        p_condvar->i_waiting_threads++;
+
+        if( p_condvar->SignalObjectAndWait )
         /* It is only possible to atomically release the mutex and initiate the
          * waiting on WinNT/2K/XP. Win9x doesn't have SignalObjectAndWait(). */
-        p_condvar->SignalObjectAndWait( p_mutex->mutex, p_condvar->semaphore,
-                                        INFINITE, FALSE );
-        /* XXX: we should protect i_waiting_threads with a mutex, but
-         * is it really worth it ? */
-        p_condvar->i_waiting_threads--;
-
-        if( p_condvar->b_broadcast
-             && p_condvar->i_waiting_threads == 0 )
-        {
-            p_condvar->SignalObjectAndWait( p_condvar->signal, p_mutex->mutex,
+            p_condvar->SignalObjectAndWait( p_mutex->mutex,
+                                            p_condvar->event,
                                             INFINITE, FALSE );
-        }
         else
         {
-            /* Just take back the lock */
-            WaitForSingleObject( p_mutex->mutex, INFINITE );
+            LeaveCriticalSection( &p_mutex->csection );
+            WaitForSingleObject( p_condvar->event, INFINITE );
         }
-        return 0;
+    }
+    else if( p_condvar->i_win9x_cv == 1 )
+    {
+        int i_waiting_threads;
+
+        /* Wait for the gate to be open */
+        WaitForSingleObject( p_condvar->event, INFINITE );
+
+        /* Increase our wait count */
+        p_condvar->i_waiting_threads++;
+
+        LeaveCriticalSection( &p_mutex->csection );
+        WaitForSingleObject( p_condvar->semaphore, INFINITE );
+
+        /* Decrement and test must be atomic */
+        EnterCriticalSection( &p_condvar->csection );
+
+        /* Decrease our wait count */
+        i_waiting_threads = --p_condvar->i_waiting_threads;
+
+        LeaveCriticalSection( &p_condvar->csection );
+
+        /* Reopen the gate if we were the last waiting thread */
+        if( !i_waiting_threads )
+            SetEvent( p_condvar->event );
     }
     else
     {
-        int i_ret;
+        int i_waiting_threads;
+
+        /* Increase our wait count */
+        p_condvar->i_waiting_threads++;
 
-        /* Release the mutex, wait, and reacquire. */
         LeaveCriticalSection( &p_mutex->csection );
-        i_ret = WaitForMultipleObjects( 2, p_condvar->p_events,
-                                        FALSE, INFINITE );
-        EnterCriticalSection( &p_mutex->csection );
+        WaitForSingleObject( p_condvar->semaphore, INFINITE );
+
+        /* Decrement and test must be atomic */
+        EnterCriticalSection( &p_condvar->csection );
 
         /* Decrease our wait count */
-        p_condvar->i_waiting_threads--;
+        i_waiting_threads = --p_condvar->i_waiting_threads;
 
-        /* If we are the last waiter and it was a broadcast signal, reset
-         * the broadcast event. */
-        if( i_ret == WAIT_OBJECT_0 + 1/*broadcast*/
-             && p_condvar->i_waiting_threads == 0 )
-        {
-            ResetEvent( p_condvar->p_events[1/*broadcast*/] );
-        }
+        LeaveCriticalSection( &p_condvar->csection );
 
-        return( i_ret == WAIT_FAILED );
+        /* Signal that the last waiting thread just went through */
+        if( !i_waiting_threads )
+            SetEvent( p_condvar->event );
     }
 
+    /* Reacquire the mutex before returning. */
+    vlc_mutex_lock( p_mutex );
+
+    return 0;
+
 #elif defined( PTHREAD_COND_T_IN_PTHREAD_H )
 
 #   ifdef DEBUG

src/libvlc.h

@@ -2,7 +2,7 @@
  * libvlc.h: main libvlc header
  *****************************************************************************
  * Copyright (C) 1998-2002 VideoLAN
- * $Id: libvlc.h,v 1.7 2002/07/23 00:39:17 sam Exp $
+ * $Id: libvlc.h,v 1.8 2002/07/29 19:05:47 gbazin Exp $
  *
  * Authors: Vincent Seguin <seguin@via.ecp.fr>
  *          Samuel Hocevar <sam@zoy.org>
@@ -280,11 +280,23 @@
 #define DEMUX_LONGTEXT N_( \
     "This is a legacy entry to let you configure demux modules")
 
-#define FAST_PTHREAD_TEXT N_("fast pthread on NT/2K/XP (developpers only)")
-#define FAST_PTHREAD_LONGTEXT N_( \
-    "On Windows NT/2K/XP we use a slow but correct pthread implementation, " \
-    "you can also use this faster implementation but you might experience " \
-    "problems with it.")
+#define FAST_MUTEX_TEXT N_("fast mutex on NT/2K/XP (developpers only)")
+#define FAST_MUTEX_LONGTEXT N_( \
+    "On Windows NT/2K/XP we use a slow mutex implementation but which " \
+    "allows us to correctely implement condition variables. " \
+    "You can also use the faster Win9x implementation but you might " \
+    "experience problems with it.")
+
+#define WIN9X_CV_TEXT N_("Condition variables implementation for Win9x " \
+    "(developpers only)")
+#define WIN9X_CV_LONGTEXT N_( \
+    "On Windows 9x/Me we use a fast but not correct condition variables " \
+    "implementation (more precisely there is a possibility for a race " \
+    "condition to happen). " \
+    "However it is possible to use slower alternatives which should be more " \
+    "robust. " \
+    "Currently you can choose between implementation 0 (which is the " \
+    "default and the fastest), 1 and 2.")
 
 #define PLAYLIST_USAGE N_("\nPlaylist items:" \
     "\n  *.mpg, *.vob                   plain MPEG-1/2 files" \
@@ -396,7 +408,8 @@ ADD_MODULE  ( "access", MODULE_CAPABILITY_ACCESS, NULL, NULL, ACCESS_TEXT, ACCES
 ADD_MODULE  ( "demux", MODULE_CAPABILITY_DEMUX, NULL, NULL, DEMUX_TEXT, DEMUX_LONGTEXT )
 
 #if defined(WIN32)
-ADD_BOOL ( "fast-pthread", 0, NULL, FAST_PTHREAD_TEXT, FAST_PTHREAD_LONGTEXT )
+ADD_BOOL ( "fast-mutex", 0, NULL, FAST_MUTEX_TEXT, FAST_MUTEX_LONGTEXT )
+ADD_INTEGER ( "win9x-cv-method", 0, NULL, WIN9X_CV_TEXT, WIN9X_CV_LONGTEXT )
 #endif
 
 /* Usage (mainly useful for cmd line stuff) */

src/misc/threads.c

@@ -2,7 +2,7 @@
  * threads.c : threads implementation for the VideoLAN client
  *****************************************************************************
  * Copyright (C) 1999, 2000, 2001, 2002 VideoLAN
- * $Id: threads.c,v 1.10 2002/07/20 18:01:43 sam Exp $
+ * $Id: threads.c,v 1.11 2002/07/29 19:05:47 gbazin Exp $
  *
  * Authors: Jean-Marc Dressler <polux@via.ecp.fr>
  *          Samuel Hocevar <sam@zoy.org>
@@ -230,7 +230,7 @@ int __vlc_mutex_init( vlc_object_t *p_this, vlc_mutex_t *p_mutex )
      * function and have a 100% correct vlc_cond_wait() implementation.
      * As this function is not available on Win9x, we can use the faster
      * CriticalSections */
-    if( (GetVersion() < 0x80000000) && !p_this->p_vlc->b_fast_pthread )
+    if( p_this->p_vlc->SignalObjectAndWait && !p_this->p_vlc->b_fast_mutex )
     {
         /* We are running on NT/2K/XP, we can use SignalObjectAndWait */
         p_mutex->mutex = CreateMutex( 0, FALSE, 0 );
@@ -348,28 +348,38 @@ int __vlc_cond_init( vlc_object_t *p_this, vlc_cond_t *p_condvar )
     /* Initialize counter */
     p_condvar->i_waiting_threads = 0;
 
-    if( (GetVersion() < 0x80000000) && !p_this->p_vlc->b_fast_pthread )
-    {
-        /* Create an auto-reset event and a semaphore. */
-        p_condvar->signal = CreateEvent( NULL, FALSE, FALSE, NULL );
-        p_condvar->semaphore = CreateSemaphore( NULL, 0, 0x7fffffff, NULL );
-
-        p_condvar->b_broadcast = 0;
+    /* Misc init */
+    p_condvar->i_win9x_cv = p_this->p_vlc->i_win9x_cv;
+    p_condvar->SignalObjectAndWait = p_this->p_vlc->SignalObjectAndWait;
 
-        /* We are running on NT/2K/XP, we can use SignalObjectAndWait */
-        p_condvar->SignalObjectAndWait = p_this->p_vlc->SignalObjectAndWait;
-
-        return !p_condvar->signal || !p_condvar->semaphore;
+    if( p_this->p_vlc->i_win9x_cv == 0 )
+    {
+        /* Create an auto-reset event. */
+        p_condvar->event = CreateEvent( NULL,   /* no security */
+                                        FALSE,  /* auto-reset event */
+                                        FALSE,  /* start non-signaled */
+                                        NULL ); /* unnamed */
+
+        p_condvar->semaphore = NULL;
+        return !p_condvar->event;
     }
     else
     {
-        p_condvar->signal = NULL;
+        p_condvar->semaphore = CreateSemaphore( NULL,       /* no security */
+                                                0,          /* initial count */
+                                                0x7fffffff, /* max count */
+                                                NULL );     /* unnamed */
+
+        if( p_this->p_vlc->i_win9x_cv == 1 )
+            /* Create a manual-reset event initially signaled. */
+            p_condvar->event = CreateEvent( NULL, TRUE, TRUE, NULL );
+        else
+            /* Create a auto-reset event. */
+            p_condvar->event = CreateEvent( NULL, FALSE, FALSE, NULL );
 
-        /* Create an auto-reset event and a manual-reset event. */
-        p_condvar->p_events[0] = CreateEvent( NULL, FALSE, FALSE, NULL );
-        p_condvar->p_events[1] = CreateEvent( NULL, TRUE, FALSE, NULL );
+        InitializeCriticalSection( &p_condvar->csection );
 
-        return !p_condvar->p_events[0] || !p_condvar->p_events[1];
+        return !p_condvar->semaphore || !p_condvar->event;
     }
 
 #elif defined( PTHREAD_COND_T_IN_PTHREAD_H )
@@ -414,16 +424,11 @@ int __vlc_cond_destroy( char * psz_file, int i_line, vlc_cond_t *p_condvar )
     return st_cond_destroy( *p_condvar );
 
 #elif defined( WIN32 )
-    if( p_condvar->signal )
-    {
-        return !CloseHandle( p_condvar->signal )
-                 || !CloseHandle( p_condvar->semaphore );
-    }
+    if( !p_condvar->semaphore )
+        return !CloseHandle( p_condvar->event );
     else
-    {
-        return !CloseHandle( p_condvar->p_events[0] )
-                 || !CloseHandle( p_condvar->p_events[1] );
-    }
+        return !CloseHandle( p_condvar->event )
+          || !CloseHandle( p_condvar->semaphore );
 
 #elif defined( PTHREAD_COND_T_IN_PTHREAD_H )
     int i_result = pthread_cond_destroy( p_condvar );

src/misc/win32_specific.c

@@ -2,7 +2,7 @@
  * win32_specific.c: Win32 specific features 
  *****************************************************************************
  * Copyright (C) 2001 VideoLAN
- * $Id: win32_specific.c,v 1.12 2002/06/08 14:08:46 sam Exp $
+ * $Id: win32_specific.c,v 1.13 2002/07/29 19:05:47 gbazin Exp $
  *
  * Authors: Samuel Hocevar <sam@zoy.org>
  *          Gildas Bazin <gbazin@netcourrier.com>
@@ -40,9 +40,16 @@ void system_Init( vlc_t *p_this, int *pi_argc, char *ppsz_argv[] )
     HINSTANCE hInstLib;
 
     /* dynamically get the address of SignalObjectAndWait */
-    hInstLib = LoadLibrary( "kernel32" );
-    p_this->p_vlc->SignalObjectAndWait =
-        (SIGNALOBJECTANDWAIT)GetProcAddress( hInstLib, "SignalObjectAndWait" );
+    if( (GetVersion() < 0x80000000) )
+    {
+        /* We are running on NT/2K/XP, we can use SignalObjectAndWait */
+        hInstLib = LoadLibrary( "kernel32" );
+        if( hInstLib)
+            p_this->p_vlc->SignalObjectAndWait =
+                (SIGNALOBJECTANDWAIT)GetProcAddress( hInstLib,
+                                                     "SignalObjectAndWait" );
+    }
+    else p_this->p_vlc->SignalObjectAndWait = NULL;
 
     /* WinSock Library Init. */
     i_err = WSAStartup( MAKEWORD( 1, 1 ), &Data );
@@ -60,7 +67,8 @@ void system_Init( vlc_t *p_this, int *pi_argc, char *ppsz_argv[] )
  *****************************************************************************/
 void system_Configure( vlc_t *p_this )
 {
-    p_this->p_vlc->b_fast_pthread = config_GetInt( p_this, "fast-pthread" );
+    p_this->p_vlc->b_fast_mutex = config_GetInt( p_this, "fast-mutex" );
+    p_this->p_vlc->i_win9x_cv = config_GetInt( p_this, "win9x-cv-method" );
 }
 
 /*****************************************************************************