Title: [278558] trunk/Source/WTF
Revision
278558
Author
ab...@igalia.com
Date
2021-06-07 04:45:59 -0700 (Mon, 07 Jun 2021)

Log Message

[WTF][GStreamer] Fix clang TSA warnings in WTF::DataMutex
https://bugs.webkit.org/show_bug.cgi?id=226719

Reviewed by Xabier Rodriguez-Calvar.

Fix the remaning clang thread safety warnings in WTF::DataMutex.

The goal of this patch is to reduce the number of warnings in the
GStreamer codebase. Whether DataMutex should be deprecated in favor of
Locker with the clang TSA annotations is outside of the scope of this
patch.

* wtf/DataMutex.h:

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (278557 => 278558)


--- trunk/Source/WTF/ChangeLog	2021-06-07 11:43:41 UTC (rev 278557)
+++ trunk/Source/WTF/ChangeLog	2021-06-07 11:45:59 UTC (rev 278558)
@@ -1,5 +1,21 @@
 2021-06-07  Alicia Boya García  <ab...@igalia.com>
 
+        [WTF][GStreamer] Fix clang TSA warnings in WTF::DataMutex
+        https://bugs.webkit.org/show_bug.cgi?id=226719
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Fix the remaning clang thread safety warnings in WTF::DataMutex.
+
+        The goal of this patch is to reduce the number of warnings in the
+        GStreamer codebase. Whether DataMutex should be deprecated in favor of
+        Locker with the clang TSA annotations is outside of the scope of this
+        patch.
+
+        * wtf/DataMutex.h:
+
+2021-06-07  Alicia Boya García  <ab...@igalia.com>
+
         [GStreamer] Remove spurious assert in WTF::DataMutex
         https://bugs.webkit.org/show_bug.cgi?id=226714
 

Modified: trunk/Source/WTF/wtf/DataMutex.h (278557 => 278558)


--- trunk/Source/WTF/wtf/DataMutex.h	2021-06-07 11:43:41 UTC (rev 278557)
+++ trunk/Source/WTF/wtf/DataMutex.h	2021-06-07 11:45:59 UTC (rev 278558)
@@ -69,15 +69,17 @@
         m_isLocked = true;
     }
 
-    ~DataMutexLocker() WTF_RELEASES_LOCK(m_dataMutex.m_mutex)
+    ~DataMutexLocker() WTF_RELEASES_LOCK()
     {
-        if (m_isLocked)
+        if (m_isLocked) {
+            assertIsHeld(m_dataMutex.m_mutex);
             mutex().unlock();
+        }
     }
 
     T* operator->()
     {
-        DATA_MUTEX_CHECK(mutex().isHeld());
+        DATA_MUTEX_CHECK(m_isLocked && mutex().isHeld());
         assertIsHeld(m_dataMutex.m_mutex);
         return &m_dataMutex.m_data;
     }
@@ -84,7 +86,7 @@
 
     T& operator*()
     {
-        DATA_MUTEX_CHECK(mutex().isHeld());
+        DATA_MUTEX_CHECK(m_isLocked && mutex().isHeld());
         assertIsHeld(m_dataMutex.m_mutex);
         return m_dataMutex.m_data;
     }
@@ -94,6 +96,9 @@
         return m_dataMutex.m_mutex;
     }
 
+    // Note: DataMutexLocker shouldn't be used after this. Due to limitations of clang thread safety analysis this can't
+    // currently be staticly checked (adding WTF_REQUIRES_LOCK() to operator->() doesn't work.)
+    // Run-time checks are still performed if enabled.
     void unlockEarly() WTF_RELEASES_LOCK(m_dataMutex.m_mutex)
     {
         DATA_MUTEX_CHECK(mutex().isHeld());
@@ -104,7 +109,7 @@
     // Used to avoid excessive brace scoping when only small parts of the code need to be run unlocked.
     // Please be mindful that accessing the wrapped data from the callback is unsafe and will fail on assertions.
     // It's helpful to use a minimal lambda capture to be conscious of what data you're having access to in these sections.
-    void runUnlocked(const Function<void()>& callback)
+    void runUnlocked(const Function<void()>& callback) WTF_IGNORES_THREAD_SAFETY_ANALYSIS
     {
         DATA_MUTEX_CHECK(mutex().isHeld());
         assertIsHeld(m_dataMutex.m_mutex);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to