[vlc-devel] [PATCH 4/4] thread: make vlc_thread_self() always work
Rémi Denis-Courmont
remi at remlab.net
Tue Feb 18 11:02:00 CET 2020
Hi,
IMO, recursive locking is bad design to begin with. With that said, VLC uses it even though not all platforms support it ditectly, and I'm not going to remove the usages.
But if you want to, be my guest. Then we can drop vlc_thread_self() entirely.
Le 18 février 2020 10:57:43 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>On Mon, Feb 17, 2020, at 21:56, Rémi Denis-Courmont wrote:
>> This changes the return type of vlc_thread_self() to match (one of)
>the
>> C run-time native thread handle type. Then we add a macro to compare
>> handles for equality, which may be useful for self-debugging or to
>> implement recursive locking.
>
>I am curious, why a project like VLC would want to implement recursive
>locking ?
>I think it's already available on all OS, no ?
>It reminds me this MR on libbluray:
>https://code.videolan.org/videolan/libbluray/merge_requests/17
>
>>
>> Without this, vlc_thread_self() returned NULL on non-POSIX platforms
>> on threads not created by vlc_clone() - such as the main thread -
>which
>> was not terribly helpful.
>>
>> This function overlaps with vlc_thread_id(). But unlike the later, it
>> works on all platforms. There are in particular no portable ways to
>> implement vlc_thread_id() on POSIX Threads.
>> ---
>> include/vlc_threads.h | 36 ++++++++++++++++++++++++++++--------
>> src/android/thread.c | 4 ++--
>> src/os2/thread.c | 4 ++--
>> src/win32/thread.c | 4 ++--
>> 4 files changed, 34 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>> index 647471e517..02f630ae52 100644
>> --- a/include/vlc_threads.h
>> +++ b/include/vlc_threads.h
>> @@ -54,6 +54,10 @@ VLC_API void vlc_testcancel(void);
>>
>> typedef struct vlc_thread *vlc_thread_t;
>> # define VLC_THREAD_CANCELED NULL
>> +
>> +typedef HANDLE vlc_osthread_t;
>> +#define vlc_thread_equal(a,b) ((a) == (b))
>> +
>> # define LIBVLC_NEED_SLEEP
>> typedef struct
>> {
>> @@ -99,6 +103,10 @@ static inline int vlc_poll(struct pollfd *fds,
>> unsigned nfds, int timeout)
>>
>> typedef struct vlc_thread *vlc_thread_t;
>> #define VLC_THREAD_CANCELED NULL
>> +
>> +typedef unsigned long vlc_osthread_t;
>> +#define vlc_thread_equal(a,b) ((a) == (b))
>> +
>> typedef struct
>> {
>> bool dynamic;
>> @@ -164,6 +172,8 @@ static inline int vlc_poll (struct pollfd *fds,
>> unsigned nfds, int timeout)
>>
>> typedef struct vlc_thread *vlc_thread_t;
>> #define VLC_THREAD_CANCELED NULL
>> +typedef pthread_t vlc_osthread_t;
>> +#define vlc_thread_equal(a,b) pthread_equal(a,b)
>> typedef pthread_mutex_t vlc_mutex_t;
>> #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER
>> typedef pthread_once_t vlc_once_t;
>> @@ -209,6 +219,8 @@ static inline int vlc_poll (struct pollfd *fds,
>> unsigned nfds, int timeout)
>>
>> typedef pthread_t vlc_thread_t;
>> #define VLC_THREAD_CANCELED PTHREAD_CANCELED
>> +typedef pthread_t vlc_osthread_t;
>> +#define vlc_thread_equal(a,b) pthread_equal(a,b)
>> typedef pthread_mutex_t vlc_mutex_t;
>> #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER
>> typedef pthread_rwlock_t vlc_rwlock_t;
>> @@ -252,6 +264,9 @@ typedef struct
>> */
>> #define VLC_THREAD_CANCELED PTHREAD_CANCELED
>>
>> +typedef pthread_t vlc_osthread_t;
>> +#define vlc_thread_equal(a,b) pthread_equal(a,b)
>> +
>> /**
>> * Mutex.
>> *
>> @@ -816,18 +831,21 @@ VLC_API void vlc_control_cancel(vlc_cleanup_t
>*);
>> * Thread handle.
>> *
>> * This function returns the thread handle of the calling thread.
>> + * This works even if the thread was <b>not</b> created with
>vlc_clone().
>> + * As a consequence, depending on the platform, this might or might
>not be the
>> + * same as the @ref vlc_thread_t thread handle returned by
>vlc_clone().
>> *
>> - * \note The exact type of the thread handle depends on the
>platform,
>> - * including an integer type, a pointer type or a compound type of
>any size.
>> - * If you need an integer identifier, use vlc_thread_id() instead.
>> + * Also depending on the platform, this might be an integer type, a
>pointer
>> + * type, or a compound type of any (reasonable) size. To compare two
>thread
>> + * handles, use the vlc_thread_equal() macro, not a hand-coded
>comparison.
>> + * Comparing the calling thread for equality with another thread is
>in fact
>> + * pretty much the only purpose of this function.
>> *
>> - * \note vlc_join(vlc_thread_self(), NULL) is undefined,
>> - * as it obviously does not make any sense (it might result in a
>deadlock, but
>> - * there are no warranties that it will).
>> + * \note If you need an integer identifier, use vlc_thread_id()
>instead.
>> *
>> - * \return the thread handle
>> + * \return the OS run-time thread handle
>> */
>> -VLC_API vlc_thread_t vlc_thread_self(void) VLC_USED;
>> +VLC_API vlc_osthread_t vlc_thread_self(void) VLC_USED;
>>
>> /**
>> * Thread identifier.
>> @@ -841,6 +859,8 @@ VLC_API vlc_thread_t vlc_thread_self(void)
>VLC_USED;
>> * There are no particular semantics to the thread ID with LibVLC.
>> * It is provided mainly for tracing and debugging.
>> *
>> + * See also vlc_thread_self().
>> + *
>> * \warning This function is not currently implemented on all
>supported
>> * platforms. Where not implemented, it returns (unsigned long)-1.
>> *
>> diff --git a/src/android/thread.c b/src/android/thread.c
>> index d3e8b25c47..da128a769c 100644
>> --- a/src/android/thread.c
>> +++ b/src/android/thread.c
>> @@ -150,9 +150,9 @@ struct vlc_thread
>>
>> static thread_local struct vlc_thread *thread = NULL;
>>
>> -vlc_thread_t vlc_thread_self (void)
>> +pthread_t vlc_thread_self(void)
>> {
>> - return thread;
>> + return pthread_self();
>> }
>>
>> void vlc_threads_setup (libvlc_int_t *p_libvlc)
>> diff --git a/src/os2/thread.c b/src/os2/thread.c
>> index f2eb435129..150b916988 100644
>> --- a/src/os2/thread.c
>> +++ b/src/os2/thread.c
>> @@ -674,9 +674,9 @@ int vlc_set_priority (vlc_thread_t th, int
>priority)
>> return VLC_SUCCESS;
>> }
>>
>> -vlc_thread_t vlc_thread_self (void)
>> +unsigned long vlc_thread_self(void)
>> {
>> - return vlc_threadvar_get (thread_key);
>> + return vlc_thread_id();
>> }
>>
>> unsigned long vlc_thread_id (void)
>> diff --git a/src/win32/thread.c b/src/win32/thread.c
>> index e0539f9322..59d55b3925 100644
>> --- a/src/win32/thread.c
>> +++ b/src/win32/thread.c
>> @@ -536,9 +536,9 @@ int vlc_clone_detach (vlc_thread_t *p_handle,
>void
>> *(*entry) (void *),
>> return vlc_clone_attr (p_handle, true, entry, data, priority);
>> }
>>
>> -vlc_thread_t vlc_thread_self (void)
>> +unsigned long vlc_thread_self(void)
>> {
>> - return TlsGetValue(thread_key);
>> + return GetCurrentThreadId();
>> }
>>
>> unsigned long vlc_thread_id (void)
>> --
>> 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/20200218/c9744fdc/attachment.html>
More information about the vlc-devel
mailing list