[vlc-devel] [PATCH 4/4] thread: make vlc_thread_self() always work
Rémi Denis-Courmont
remi at remlab.net
Mon Mar 2 13:48:14 CET 2020
vlc_thread_id() is not available on all targets, so it can't be used for much anything beyond debugging. Recursive locking would be easier if it were.
We can remove vlc_thread_self() and vlc_thread_equals() though.
Le 2 mars 2020 14:37:40 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>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
>>
>_______________________________________________
>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/20200302/e12e75db/attachment.html>
More information about the vlc-devel
mailing list