[vlc-devel] [PATCH 4/4] thread: make vlc_thread_self() always work
Thomas Guillem
thomas at gllm.fr
Tue Feb 18 11:07:19 CET 2020
I don't like recursive locking either.
I though recursive locking was avalaible on all platforms, that why I wondered why you would add something to implement it.
Anyway, I'm OK with this patch, vlc_thread_self() might be very useful for debug purpose.
On Tue, Feb 18, 2020, at 11:02, Rémi Denis-Courmont wrote:
> 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.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/20200218/8610302c/attachment.html>
More information about the vlc-devel
mailing list