[vlc-devel] [PATCH 3/7] thread: use generic semaphores on all platforms
Thomas Guillem
thomas at gllm.fr
Mon Feb 17 16:19:51 CET 2020
On Mon, Feb 17, 2020, at 15:18, Rémi Denis-Courmont wrote:
> 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.
Thanks for the explanation.
OK for this set (after fixing problems you mentioned yourself).
>
> 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.0vlc-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é.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200217/ba0bf380/attachment.html>
More information about the vlc-devel
mailing list