[vlc-commits] darwin/thread: Ignore vlc_mutex_unlock failures

[Marvin Scholz]

vlc | branch: master | Marvin Scholz <epirat07 at gmail.com> | Mon Oct 28 18:24:44 2019 +0100| [778f2203177a9a0de7ca11da600e39b06eb8093e] | committer: Thomas Guillem

darwin/thread: Ignore vlc_mutex_unlock failures

macOS pthread implementation for pthread_cond_wait is buggy, causing
sometimes the mutex to not be locked when running thread cancellation
cleanup handlers.
This causes random failures of vlc_mutex_unlock, as the cancellation
cleanup handler does not hold a lock on the mutex in rare cases.

Signed-off-by: Thomas Guillem <thomas at gllm.fr>

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=778f2203177a9a0de7ca11da600e39b06eb8093e
---

 src/darwin/thread.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/darwin/thread.c b/src/darwin/thread.c
index 9dcffad28d..d8cd73d0cc 100644
--- a/src/darwin/thread.c
+++ b/src/darwin/thread.c
@@ -172,7 +172,35 @@ int vlc_mutex_trylock (vlc_mutex_t *p_mutex)
 void vlc_mutex_unlock (vlc_mutex_t *p_mutex)
 {
     int val = pthread_mutex_unlock( p_mutex );
-    VLC_THREAD_ASSERT ("unlocking mutex");
+    /* FIXME: We can't check for the success of the unlock
+     * here as due to a bug in Apple pthread implementation.
+     * The `pthread_cond_wait` function does not behave like
+     * it should According to POSIX, pthread_cond_wait is a
+     * cancellation point and when a thread is cancelled while
+     * in a condition wait, the mutex is re-acquired before
+     * calling the first cancellation cleanup handler:
+     *
+     * > The effect is as if the thread were unblocked, allowed
+     * > to execute up to the point of returning from the call to
+     * > pthread_cond_timedwait() or pthread_cond_wait(), but at
+     * > that point notices the cancellation request and instead
+     * > of returning to the caller of pthread_cond_timedwait()
+     * > or pthread_cond_wait(), starts the thread cancellation
+     * > activities, which includes calling cancellation cleanup
+     * > handlers.
+     *
+     * Unfortunately the mutex is not locked sometimes, causing
+     * the call to `pthread_mutex_unlock` to fail.
+     * Until this is fixed, enabling this assertion would lead to
+     * spurious test failures and VLC crashes when compiling with
+     * debug enabled, which would make it nearly impossible to
+     * proeprly test with debug builds on macOS.
+     * This was reported to Apple as FB6152751.
+     */
+#ifndef NDEBUG
+    if (val != EPERM)
+        VLC_THREAD_ASSERT ("unlocking mutex");
+#endif
     vlc_mutex_unmark(p_mutex);
 }