[vlc-devel] [PATCH 4/4] thread: make vlc_thread_self() always work
Steve Lhomme
robux4 at ycbcr.xyz
Mon Mar 2 13:37:40 CET 2020
On 2020-02-18 11:07, Thomas Guillem wrote:
> 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.
It returns an OS specific type, vlc_thread_id() is better for that (and
that's what is used for logging, although darwin doesn't provide a
meaningful value for now).
There is no more code using vlc_thread_self(). Should it be removed ?
> 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.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é.
>> _______________________________________________
>> 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
>
More information about the vlc-devel
mailing list