<html><head></head><body>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.<br><br>We can remove vlc_thread_self() and vlc_thread_equals() though.<br><br><div class="gmail_quote">Le 2 mars 2020 14:37:40 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 2020-02-18 11:07, Thomas Guillem wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">I don't like recursive locking either.<br>I though recursive locking was avalaible on all platforms, that why I <br>wondered why you would add something to implement it.<br><br>Anyway, I'm OK with this patch, vlc_thread_self() might be very useful <br>for debug purpose.<br></blockquote><br>It returns an OS specific type, vlc_thread_id() is better for that (and <br>that's what is used for logging, although darwin doesn't provide a <br>meaningful value for now).<br><br>There is no more code using vlc_thread_self(). Should it be removed ?<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">On Tue, Feb 18, 2020, at 11:02, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Hi,<br><br> IMO, recursive locking is bad design to begin with. With that said, <br> VLC uses it even though not all platforms support it ditectly, and I'm <br> not going to remove the usages.<br><br> But if you want to, be my guest. Then we can drop vlc_thread_self() <br> entirely.<br><br> Le 18 février 2020 10:57:43 GMT+02:00, Thomas Guillem <thomas@gllm.fr> <br> a écrit :<br><br><br> On Mon, Feb 17, 2020, at 21:56, Rémi Denis-Courmont wrote:<br><br> This changes the return type of vlc_thread_self() to match<br> (one of) the<br> C run-time native thread handle type. Then we add a macro to<br> compare<br> handles for equality, which may be useful for self-debugging or to<br> implement recursive locking.<br><br><br> I am curious, why a project like VLC would want to implement<br> recursive locking ?<br> I think it's already available on all OS, no ?<br> It reminds me this MR on libbluray:<br> <a href="https://code.videolan.org/videolan/libbluray/merge_requests/17">https://code.videolan.org/videolan/libbluray/merge_requests/17</a><br><br> Without this, vlc_thread_self() returned NULL on non-POSIX<br> platforms<br> on threads not created by vlc_clone() - such as the main<br> thread - which<br> was not terribly helpful.<br><br> This function overlaps with vlc_thread_id(). But unlike the<br> later, it<br> works on all platforms. There are in particular no portable<br> ways to<br> implement vlc_thread_id() on POSIX Threads.<hr> include/vlc_threads.h | 36 ++++++++++++++++++++++++++++--------<br> src/android/thread.c | 4 ++--<br> src/os2/thread.c | 4 ++--<br> src/win32/thread.c | 4 ++--<br> 4 files changed, 34 insertions(+), 14 deletions(-)<br><br> diff --git a/include/vlc_threads.h b/include/vlc_threads.h<br> index 647471e517..02f630ae52 100644<br> --- a/include/vlc_threads.h<br> +++ b/include/vlc_threads.h<br> @@ -54,6 +54,10 @@ VLC_API void vlc_testcancel(void);<br><br> typedef struct vlc_thread *vlc_thread_t;<br> # define VLC_THREAD_CANCELED NULL<br> +<br> +typedef HANDLE vlc_osthread_t;<br> +#define vlc_thread_equal(a,b) ((a) == (b))<br> +<br> # define LIBVLC_NEED_SLEEP<br> typedef struct<br> {<br> @@ -99,6 +103,10 @@ static inline int vlc_poll(struct pollfd<br> *fds,<br> unsigned nfds, int timeout)<br><br> typedef struct vlc_thread *vlc_thread_t;<br> #define VLC_THREAD_CANCELED NULL<br> +<br> +typedef unsigned long vlc_osthread_t;<br> +#define vlc_thread_equal(a,b) ((a) == (b))<br> +<br> typedef struct<br> {<br> bool dynamic;<br> @@ -164,6 +172,8 @@ static inline int vlc_poll (struct pollfd<br> *fds,<br> unsigned nfds, int timeout)<br><br> typedef struct vlc_thread *vlc_thread_t;<br> #define VLC_THREAD_CANCELED NULL<br> +typedef pthread_t vlc_osthread_t;<br> +#define vlc_thread_equal(a,b) pthread_equal(a,b)<br> typedef pthread_mutex_t vlc_mutex_t;<br> #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER<br> typedef pthread_once_t vlc_once_t;<br> @@ -209,6 +219,8 @@ static inline int vlc_poll (struct pollfd<br> *fds,<br> unsigned nfds, int timeout)<br><br> typedef pthread_t vlc_thread_t;<br> #define VLC_THREAD_CANCELED PTHREAD_CANCELED<br> +typedef pthread_t vlc_osthread_t;<br> +#define vlc_thread_equal(a,b) pthread_equal(a,b)<br> typedef pthread_mutex_t vlc_mutex_t;<br> #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER<br> typedef pthread_rwlock_t vlc_rwlock_t;<br> @@ -252,6 +264,9 @@ typedef struct<br> */<br> #define VLC_THREAD_CANCELED PTHREAD_CANCELED<br><br> +typedef pthread_t vlc_osthread_t;<br> +#define vlc_thread_equal(a,b) pthread_equal(a,b)<br> +<br> /**<br> * Mutex.<br> *<br> @@ -816,18 +831,21 @@ VLC_API void<br> vlc_control_cancel(vlc_cleanup_t *);<br> * Thread handle.<br> *<br> * This function returns the thread handle of the calling thread.<br> + * This works even if the thread was <b>not</b> created with<br> vlc_clone().<br> + * As a consequence, depending on the platform, this might or<br> might not be the<br> + * same as the @ref vlc_thread_t thread handle returned by<br> vlc_clone().<br> *<br> - * \note The exact type of the thread handle depends on the<br> platform,<br> - * including an integer type, a pointer type or a compound<br> type of any size.<br> - * If you need an integer identifier, use vlc_thread_id()<br> instead.<br> + * Also depending on the platform, this might be an integer<br> type, a pointer<br> + * type, or a compound type of any (reasonable) size. To<br> compare two thread<br> + * handles, use the vlc_thread_equal() macro, not a<br> hand-coded comparison.<br> + * Comparing the calling thread for equality with another<br> thread is in fact<br> + * pretty much the only purpose of this function.<br> *<br> - * \note vlc_join(vlc_thread_self(), NULL) is undefined,<br> - * as it obviously does not make any sense (it might result<br> in a deadlock, but<br> - * there are no warranties that it will).<br> + * \note If you need an integer identifier, use<br> vlc_thread_id() instead.<br> *<br> - * \return the thread handle<br> + * \return the OS run-time thread handle<br> */<br> -VLC_API vlc_thread_t vlc_thread_self(void) VLC_USED;<br> +VLC_API vlc_osthread_t vlc_thread_self(void) VLC_USED;<br><br> /**<br> * Thread identifier.<br> @@ -841,6 +859,8 @@ VLC_API vlc_thread_t vlc_thread_self(void)<br> VLC_USED;<br> * There are no particular semantics to the thread ID with LibVLC.<br> * It is provided mainly for tracing and debugging.<br> *<br> + * See also vlc_thread_self().<br> + *<br> * \warning This function is not currently implemented on all<br> supported<br> * platforms. Where not implemented, it returns (unsigned long)-1.<br> *<br> diff --git a/src/android/thread.c b/src/android/thread.c<br> index d3e8b25c47..da128a769c 100644<br> --- a/src/android/thread.c<br> +++ b/src/android/thread.c<br> @@ -150,9 +150,9 @@ struct vlc_thread<br><br> static thread_local struct vlc_thread *thread = NULL;<br><br> -vlc_thread_t vlc_thread_self (void)<br> +pthread_t vlc_thread_self(void)<br> {<br> - return thread;<br> + return pthread_self();<br> }<br><br> void vlc_threads_setup (libvlc_int_t *p_libvlc)<br> diff --git a/src/os2/thread.c b/src/os2/thread.c<br> index f2eb435129..150b916988 100644<br> --- a/src/os2/thread.c<br> +++ b/src/os2/thread.c<br> @@ -674,9 +674,9 @@ int vlc_set_priority (vlc_thread_t th, int<br> priority)<br> return VLC_SUCCESS;<br> }<br><br> -vlc_thread_t vlc_thread_self (void)<br> +unsigned long vlc_thread_self(void)<br> {<br> - return vlc_threadvar_get (thread_key);<br> + return vlc_thread_id();<br> }<br><br> unsigned long vlc_thread_id (void)<br> diff --git a/src/win32/thread.c b/src/win32/thread.c<br> index e0539f9322..59d55b3925 100644<br> --- a/src/win32/thread.c<br> +++ b/src/win32/thread.c<br> @@ -536,9 +536,9 @@ int vlc_clone_detach (vlc_thread_t<br> *p_handle, void<br> *(*entry) (void *),<br> return vlc_clone_attr (p_handle, true, entry, data, priority);<br> }<br><br> -vlc_thread_t vlc_thread_self (void)<br> +unsigned long vlc_thread_self(void)<br> {<br> - return TlsGetValue(thread_key);<br> + return GetCurrentThreadId();<br> }<br><br> unsigned long vlc_thread_id (void)<br> -- <br> 2.25.0<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez <br> excuser ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>