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