[vlc-commits] [Git][videolan/vlc][master] 5 commits: win32: fix nested cancellation

Steve Lhomme (@robUx4) gitlab at videolan.org
Tue Sep 28 12:32:20 UTC 2021



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
5b73f0da by Rémi Denis-Courmont at 2021-09-28T11:30:03+00:00
win32: fix nested cancellation

A cancellation point within a cancellation cleanup handler should be
ignored. Cancellation cannot be nested.

- - - - -
d3814fc9 by Rémi Denis-Courmont at 2021-09-28T11:30:03+00:00
win32: fix memory order in vlc_cancel()

What the cancelling thread did before vlc_cancel() should be visible to
the cancelled thread's cleanup handlers.

- - - - -
ba695e43 by Rémi Denis-Courmont at 2021-09-28T11:30:03+00:00
win32: split vlc_testcancel()

No functional changes.

- - - - -
f4a7c922 by Rémi Denis-Courmont at 2021-09-28T11:30:03+00:00
win32: simplify sleeping on Windows 8+

Replace the "common" futex-based implementation with a simpler and
faster (lock-less) Windows-specific implementation

- - - - -
bd12c7ad by Rémi Denis-Courmont at 2021-09-28T11:30:03+00:00
win32: remove left-over macro hacks

- - - - -


3 changed files:

- include/vlc_threads.h
- src/misc/threads.c
- src/win32/thread.c


Changes:

=====================================
include/vlc_threads.h
=====================================
@@ -59,7 +59,6 @@ VLC_API void vlc_testcancel(void);
 typedef struct vlc_thread *vlc_thread_t;
 # define VLC_THREAD_CANCELED NULL
 
-# define LIBVLC_NEED_SLEEP
 typedef struct vlc_threadvar *vlc_threadvar_t;
 typedef struct vlc_timer *vlc_timer_t;
 


=====================================
src/misc/threads.c
=====================================
@@ -62,12 +62,7 @@ void vlc_global_mutex (unsigned n, bool acquire)
         vlc_mutex_unlock (lock);
 }
 
-#if defined (_WIN32) && (_WIN32_WINNT < _WIN32_WINNT_WIN8)
-/* Cannot define OS version-dependent stuff in public headers */
-# undef LIBVLC_NEED_SLEEP
-#endif
-
-#if defined(_WIN32) || defined(__ANDROID__)
+#ifdef LIBVLC_NEED_SLEEP
 static void do_vlc_cancel_addr_clear(void *addr)
 {
     vlc_cancel_addr_clear(addr);
@@ -89,12 +84,7 @@ static void vlc_cancel_addr_finish(atomic_uint *addr)
     /* Act on cancellation as potential wake-up source */
     vlc_testcancel();
 }
-#else
-# define vlc_cancel_addr_prepare(addr) ((void)0)
-# define vlc_cancel_addr_finish(addr) ((void)0)
-#endif
 
-#ifdef LIBVLC_NEED_SLEEP
 void (vlc_tick_wait)(vlc_tick_t deadline)
 {
     atomic_uint value = ATOMIC_VAR_INIT(0);


=====================================
src/win32/thread.c
=====================================
@@ -35,6 +35,7 @@
 #include "libvlc.h"
 #include <stdarg.h>
 #include <stdatomic.h>
+#include <stdnoreturn.h>
 #include <assert.h>
 #include <limits.h>
 #include <errno.h>
@@ -61,17 +62,11 @@ struct vlc_thread
     HANDLE         id;
 
     bool           killable;
-    atomic_bool    killed;
+    atomic_uint    killed;
     vlc_cleanup_t *cleaners;
 
     void        *(*entry) (void *);
     void          *data;
-
-    struct
-    {
-        atomic_uint     *addr;
-        CRITICAL_SECTION lock;
-    } wait;
 };
 
 /*** Thread-specific variables (TLS) ***/
@@ -372,8 +367,6 @@ int vlc_clone (vlc_thread_t *p_handle, void *(*entry) (void *),
     th->killable = false; /* not until vlc_entry() ! */
     atomic_init(&th->killed, false);
     th->cleaners = NULL;
-    th->wait.addr = NULL;
-    InitializeCriticalSection(&th->wait.lock);
 
     HANDLE h;
 #ifdef VLC_WINSTORE_APP
@@ -417,7 +410,6 @@ void vlc_join (vlc_thread_t th, void **result)
     if (result != NULL)
         *result = th->data;
     CloseHandle (th->id);
-    DeleteCriticalSection(&th->wait.lock);
     free(th);
 }
 
@@ -445,15 +437,8 @@ static void CALLBACK vlc_cancel_self (ULONG_PTR self)
 
 void vlc_cancel (vlc_thread_t th)
 {
-    atomic_store_explicit(&th->killed, true, memory_order_relaxed);
-
-    EnterCriticalSection(&th->wait.lock);
-    if (th->wait.addr != NULL)
-    {
-        atomic_fetch_or_explicit(th->wait.addr, 1, memory_order_relaxed);
-        vlc_atomic_notify_all(th->wait.addr);
-    }
-    LeaveCriticalSection(&th->wait.lock);
+    atomic_store_explicit(&th->killed, true, memory_order_release);
+    vlc_atomic_notify_one(&th->killed);
 
 #if IS_INTERRUPTIBLE
     QueueUserAPC (vlc_cancel_self, th->id, (uintptr_t)th);
@@ -483,17 +468,9 @@ void vlc_restorecancel (int state)
     th->killable = state != 0;
 }
 
-void vlc_testcancel (void)
+noreturn static void vlc_docancel(struct vlc_thread *th)
 {
-    struct vlc_thread *th = TlsGetValue(thread_key);
-    if (th == NULL)
-        return; /* Main thread - cannot be cancelled anyway */
-    if (!th->killable)
-        return;
-    if (!atomic_load_explicit(&th->killed, memory_order_relaxed))
-        return;
-
-    th->killable = true; /* Do not re-enter cancellation cleanup */
+    th->killable = false; /* Do not re-enter cancellation cleanup */
 
     for (vlc_cleanup_t *p = th->cleaners; p != NULL; p = p->next)
         p->proc (p->data);
@@ -506,6 +483,19 @@ void vlc_testcancel (void)
 #endif // !VLC_WINSTORE_APP
 }
 
+void vlc_testcancel (void)
+{
+    struct vlc_thread *th = TlsGetValue(thread_key);
+    if (th == NULL)
+        return; /* Main thread - cannot be cancelled anyway */
+    if (!th->killable)
+        return;
+    if (!atomic_load_explicit(&th->killed, memory_order_relaxed))
+        return;
+
+    vlc_docancel(th);
+}
+
 void vlc_control_cancel (vlc_cleanup_t *cleaner)
 {
     /* NOTE: This function only modifies thread-specific data, so there is no
@@ -528,30 +518,6 @@ void vlc_control_cancel (vlc_cleanup_t *cleaner)
     }
 }
 
-void vlc_cancel_addr_set(atomic_uint *addr)
-{
-    struct vlc_thread *th = TlsGetValue(thread_key);
-    if (th == NULL)
-        return; /* Main thread - cannot be cancelled anyway */
-
-    EnterCriticalSection(&th->wait.lock);
-    assert(th->wait.addr == NULL);
-    th->wait.addr = addr;
-    LeaveCriticalSection(&th->wait.lock);
-}
-
-void vlc_cancel_addr_clear(atomic_uint *addr)
-{
-    struct vlc_thread *th = TlsGetValue(thread_key);
-    if (th == NULL)
-        return; /* Main thread - cannot be cancelled anyway */
-
-    EnterCriticalSection(&th->wait.lock);
-    assert(th->wait.addr == addr);
-    th->wait.addr = NULL;
-    LeaveCriticalSection(&th->wait.lock);
-}
-
 /*** Clock ***/
 static union
 {
@@ -643,20 +609,39 @@ vlc_tick_t vlc_tick_now (void)
     return mdate_selected ();
 }
 
-#if (_WIN32_WINNT < _WIN32_WINNT_WIN8)
 void (vlc_tick_wait)(vlc_tick_t deadline)
 {
     vlc_tick_t delay;
+#if (_WIN32_WINNT >= _WIN32_WINNT_WIN8)
+    struct vlc_thread *th = TlsGetValue(thread_key);
 
+    if (th != NULL && th->killable)
+    {
+        do
+        {
+            if (atomic_load_explicit(&th->killed, memory_order_acquire))
+                vlc_docancel(th);
+        }
+        while (vlc_atomic_timedwait(&th->killed, false, deadline) == 0);
+
+        return;
+    }
+#else
     vlc_testcancel();
+#endif
+
     while ((delay = (deadline - vlc_tick_now())) > 0)
     {
         delay = (delay + (1000-1)) / 1000;
         if (unlikely(delay > 0x7fffffff))
             delay = 0x7fffffff;
 
+#if (_WIN32_WINNT >= _WIN32_WINNT_WIN8)
+        Sleep(delay);
+#else
         SleepEx(delay, TRUE);
         vlc_testcancel();
+#endif
     }
 }
 
@@ -664,7 +649,6 @@ void (vlc_tick_sleep)(vlc_tick_t delay)
 {
     vlc_tick_wait (vlc_tick_now () + delay);
 }
-#endif
 
 static BOOL SelectClockSource(vlc_object_t *obj)
 {



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/bd815b233e9ebc890d7c77023046b7a0ae205b19...bd12c7ad37642a2ea6e3b3fa9e5e4210d4a7c4a0

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/bd815b233e9ebc890d7c77023046b7a0ae205b19...bd12c7ad37642a2ea6e3b3fa9e5e4210d4a7c4a0
You're receiving this email because of your account on code.videolan.org.




More information about the vlc-commits mailing list