[vlc-devel] [PATCH 3/7] thread: use generic semaphores on all platforms

Rémi Denis-Courmont remi at remlab.net
Mon Feb 17 15:18:34 CET 2020


Hi,

There is not a specific goal beyond sharing the code here. It so happens to simplify implementing timed-wait, of course. It also happens to allow eliminating destroy calls. But the patch stands alone for the generic benefits if using a common implementation of any thing.

Le 17 février 2020 14:43:54 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>I was about to answer to your previous patch that we could use the OS
>semaphore for Android and Windows.
>
>But you are now removing all OS implementation. It deserves a git
>message explaining why.
>
>I'm curious to know the reason too. Is it for the timed_sem ?
>
>On Sun, Feb 16, 2020, at 18:50, Rémi Denis-Courmont wrote:
>> ---
>>  include/vlc_threads.h | 14 ------------
>>  src/darwin/thread.c   | 45 --------------------------------------
>>  src/misc/threads.c    |  3 ---
>>  src/posix/thread.c    | 44 -------------------------------------
>>  src/win32/thread.c    | 51
>-------------------------------------------
>>  5 files changed, 157 deletions(-)
>> 
>> diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>> index 688855ca72..584b6416b1 100644
>> --- a/include/vlc_threads.h
>> +++ b/include/vlc_threads.h
>> @@ -70,7 +70,6 @@ typedef struct
>>  } vlc_mutex_t;
>>  #define VLC_STATIC_MUTEX { false, { { false, 0 } } }
>>  #define LIBVLC_NEED_CONDVAR
>> -#define LIBVLC_NEED_SEMAPHORE
>>  #define LIBVLC_NEED_RWLOCK
>>  typedef INIT_ONCE vlc_once_t;
>>  #define VLC_STATIC_ONCE INIT_ONCE_STATIC_INIT
>> @@ -123,7 +122,6 @@ typedef struct
>>      unsigned signaled;
>>  } vlc_cond_t;
>>  #define VLC_STATIC_COND { NULLHANDLE, 0, NULLHANDLE, 0 }
>> -#define LIBVLC_NEED_SEMAPHORE
>>  #define LIBVLC_NEED_RWLOCK
>>  typedef struct
>>  {
>> @@ -172,7 +170,6 @@ static inline int vlc_poll (struct pollfd *fds, 
>> unsigned nfds, int timeout)
>>  # define LIBVLC_USE_PTHREAD_CLEANUP   1
>>  # define LIBVLC_NEED_SLEEP
>>  # define LIBVLC_NEED_CONDVAR
>> -# define LIBVLC_NEED_SEMAPHORE
>>  # define LIBVLC_NEED_RWLOCK
>>  
>>  typedef struct vlc_thread *vlc_thread_t;
>> @@ -226,7 +223,6 @@ typedef pthread_mutex_t vlc_mutex_t;
>>  #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER
>>  typedef pthread_cond_t vlc_cond_t;
>>  #define VLC_STATIC_COND PTHREAD_COND_INITIALIZER
>> -typedef semaphore_t     vlc_sem_t;
>>  typedef pthread_rwlock_t vlc_rwlock_t;
>>  #define VLC_STATIC_RWLOCK PTHREAD_RWLOCK_INITIALIZER
>>  typedef pthread_once_t  vlc_once_t;
>> @@ -244,7 +240,6 @@ typedef struct vlc_timer *vlc_timer_t;
>>  #else /* POSIX threads */
>>  # include <unistd.h> /* _POSIX_SPIN_LOCKS */
>>  # include <pthread.h>
>> -# include <semaphore.h>
>>  
>>  /**
>>   * Whether LibVLC threads are based on POSIX threads.
>> @@ -299,13 +294,6 @@ typedef pthread_cond_t  vlc_cond_t;
>>   */
>>  #define VLC_STATIC_COND  PTHREAD_COND_INITIALIZER
>>  
>> -/**
>> - * Semaphore.
>> - *
>> - * Storage space for a thread-safe semaphore.
>> - */
>> -typedef sem_t           vlc_sem_t;
>> -
>>  /**
>>   * Read/write lock.
>>   *
>> @@ -367,7 +355,6 @@ typedef struct
>>  # define VLC_STATIC_COND { 0 }
>>  #endif
>>  
>> -#ifdef LIBVLC_NEED_SEMAPHORE
>>  typedef struct vlc_sem
>>  {
>>      union {
>> @@ -377,7 +364,6 @@ typedef struct vlc_sem
>>          int dummy;
>>     };
>>  } vlc_sem_t;
>> -#endif
>>  
>>  #ifdef LIBVLC_NEED_RWLOCK
>>  typedef struct vlc_rwlock
>> diff --git a/src/darwin/thread.c b/src/darwin/thread.c
>> index 1eca7755d2..349a4935f3 100644
>> --- a/src/darwin/thread.c
>> +++ b/src/darwin/thread.c
>> @@ -312,51 +312,6 @@ int vlc_cond_timedwait_daytime (vlc_cond_t 
>> *p_condvar, vlc_mutex_t *p_mutex,
>>  }
>>  
>>  
>> -/* Initialize a semaphore. */
>> -void vlc_sem_init (vlc_sem_t *sem, unsigned value)
>> -{
>> -    if (unlikely(semaphore_create(mach_task_self(), sem, 
>> SYNC_POLICY_FIFO, value) != KERN_SUCCESS))
>> -        abort ();
>> -}
>> -
>> -void vlc_sem_destroy (vlc_sem_t *sem)
>> -{
>> -    int val;
>> -
>> -    if (likely(semaphore_destroy(mach_task_self(), *sem) == 
>> KERN_SUCCESS))
>> -        return;
>> -
>> -    val = EINVAL;
>> -
>> -    VLC_THREAD_ASSERT ("destroying semaphore");
>> -}
>> -
>> -int vlc_sem_post (vlc_sem_t *sem)
>> -{
>> -    int val;
>> -
>> -    if (likely(semaphore_signal(*sem) == KERN_SUCCESS))
>> -        return 0;
>> -
>> -    val = EINVAL;
>> -
>> -    if (unlikely(val != EOVERFLOW))
>> -        VLC_THREAD_ASSERT ("unlocking semaphore");
>> -    return val;
>> -}
>> -
>> -void vlc_sem_wait (vlc_sem_t *sem)
>> -{
>> -    int val;
>> -
>> -    if (likely(semaphore_wait(*sem) == KERN_SUCCESS))
>> -        return;
>> -
>> -    val = EINVAL;
>> -
>> -    VLC_THREAD_ASSERT ("locking semaphore");
>> -}
>> -
>>  void vlc_rwlock_init (vlc_rwlock_t *lock)
>>  {
>>      if (unlikely(pthread_rwlock_init (lock, NULL)))
>> diff --git a/src/misc/threads.c b/src/misc/threads.c
>> index 82bd5b60fc..ccf002be62 100644
>> --- a/src/misc/threads.c
>> +++ b/src/misc/threads.c
>> @@ -145,7 +145,6 @@ bool vlc_mutex_marked(const vlc_mutex_t *mutex)
>>  #if defined (_WIN32) && (_WIN32_WINNT < _WIN32_WINNT_WIN8)
>>  /* Cannot define OS version-dependent stuff in public headers */
>>  # undef LIBVLC_NEED_SLEEP
>> -# undef LIBVLC_NEED_SEMAPHORE
>>  #endif
>>  
>>  #if defined(LIBVLC_NEED_SLEEP) || defined(LIBVLC_NEED_CONDVAR)
>> @@ -396,7 +395,6 @@ void vlc_rwlock_unlock (vlc_rwlock_t *lock)
>>  }
>>  #endif /* LIBVLC_NEED_RWLOCK */
>>  
>> -#ifdef LIBVLC_NEED_SEMAPHORE
>>  /*** Generic semaphores ***/
>>  
>>  void vlc_sem_init (vlc_sem_t *sem, unsigned value)
>> @@ -440,4 +438,3 @@ void vlc_sem_wait (vlc_sem_t *sem)
>>          }
>>      }
>>  }
>> -#endif /* LIBVLC_NEED_SEMAPHORE */
>> diff --git a/src/posix/thread.c b/src/posix/thread.c
>> index dfe0ea7c9d..9c9271a100 100644
>> --- a/src/posix/thread.c
>> +++ b/src/posix/thread.c
>> @@ -226,50 +226,6 @@ int vlc_cond_timedwait_daytime (vlc_cond_t 
>> *p_condvar, vlc_mutex_t *p_mutex,
>>      return val;
>>  }
>>  
>> -void vlc_sem_init (vlc_sem_t *sem, unsigned value)
>> -{
>> -    if (unlikely(sem_init (sem, 0, value)))
>> -        abort ();
>> -}
>> -
>> -void vlc_sem_destroy (vlc_sem_t *sem)
>> -{
>> -    int val;
>> -
>> -    if (likely(sem_destroy (sem) == 0))
>> -        return;
>> -
>> -    val = errno;
>> -
>> -    VLC_THREAD_ASSERT ("destroying semaphore");
>> -}
>> -
>> -int vlc_sem_post (vlc_sem_t *sem)
>> -{
>> -    int val;
>> -
>> -    if (likely(sem_post (sem) == 0))
>> -        return 0;
>> -
>> -    val = errno;
>> -
>> -    if (unlikely(val != EOVERFLOW))
>> -        VLC_THREAD_ASSERT ("unlocking semaphore");
>> -    return val;
>> -}
>> -
>> -void vlc_sem_wait (vlc_sem_t *sem)
>> -{
>> -    int val;
>> -
>> -    do
>> -        if (likely(sem_wait (sem) == 0))
>> -            return;
>> -    while ((val = errno) == EINTR);
>> -
>> -    VLC_THREAD_ASSERT ("locking semaphore");
>> -}
>> -
>>  void vlc_rwlock_init (vlc_rwlock_t *lock)
>>  {
>>      if (unlikely(pthread_rwlock_init (lock, NULL)))
>> diff --git a/src/win32/thread.c b/src/win32/thread.c
>> index 7bc6196519..4d6d7bdb5d 100644
>> --- a/src/win32/thread.c
>> +++ b/src/win32/thread.c
>> @@ -154,57 +154,6 @@ void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
>>      vlc_mutex_unmark(p_mutex);
>>  }
>>  
>> -/*** Semaphore ***/
>> -#if (_WIN32_WINNT < _WIN32_WINNT_WIN8)
>> -# include <stdalign.h>
>> -
>> -static inline HANDLE *vlc_sem_handle_p(vlc_sem_t *sem)
>> -{
>> -    /* NOTE: vlc_sem_t layout cannot easily depend on Windows
>version */
>> -    static_assert (sizeof (HANDLE) <= sizeof (vlc_sem_t), "Size
>mismatch!");
>> -    static_assert ((alignof (HANDLE) % alignof (vlc_sem_t)) == 0,
>> -                   "Alignment mismatch");
>> -    return (HANDLE *)sem;
>> -}
>> -#define vlc_sem_handle(sem) (*vlc_sem_handle_p(sem))
>> -
>> -void vlc_sem_init (vlc_sem_t *sem, unsigned value)
>> -{
>> -    HANDLE handle = CreateSemaphore(NULL, value, 0x7fffffff, NULL);
>> -    if (handle == NULL)
>> -        abort ();
>> -
>> -    vlc_sem_handle(sem) = handle;
>> -}
>> -
>> -void vlc_sem_destroy (vlc_sem_t *sem)
>> -{
>> -    CloseHandle(vlc_sem_handle(sem));
>> -}
>> -
>> -int vlc_sem_post (vlc_sem_t *sem)
>> -{
>> -    ReleaseSemaphore(vlc_sem_handle(sem), 1, NULL);
>> -    return 0; /* FIXME */
>> -}
>> -
>> -void vlc_sem_wait (vlc_sem_t *sem)
>> -{
>> -    HANDLE handle = vlc_sem_handle(sem);
>> -    DWORD result;
>> -
>> -    do
>> -    {
>> -        vlc_testcancel ();
>> -        result = WaitForSingleObjectEx(handle, INFINITE, TRUE);
>> -
>> -        /* Semaphore abandoned would be a bug. */
>> -        assert(result != WAIT_ABANDONED_0);
>> -    }
>> -    while (result == WAIT_IO_COMPLETION || result == WAIT_FAILED);
>> -}
>> -#endif
>> -
>>  /*** One-time initialization ***/
>>  static BOOL CALLBACK vlc_once_callback(INIT_ONCE *once, void *parm,
>void **ctx)
>>  {
>> -- 
>> 2.25.0
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200217/4fcc4034/attachment.html>


More information about the vlc-devel mailing list