Title: [278936] trunk
- Revision
- 278936
- Author
- ab...@igalia.com
- Date
- 2021-06-16 08:41:15 -0700 (Wed, 16 Jun 2021)
Log Message
[WTF] DataMutex: Assert on double locking on the same thread
https://bugs.webkit.org/show_bug.cgi?id=227069
Reviewed by Xabier Rodriguez-Calvar.
Source/WTF:
DataMutex used to use OwnerAwareLock to track what thread is holding
the mutex and emit assertion errors if a thread is found attempting to
lock a mutex held by that same thread. This turns deadlocks into
runtime errors.
OwnerAwareLock was removed when DataMutex got clang thread safety
annotations. This patch reintroduces the same logic, while keeping
thread-safety annotations.
This fixes WTF_DataMutex.DoubleLockDeathTest, which tested this
functionality and was previously regressed.
* wtf/DataMutex.h:
Tools:
Update expectations to include the fixed test.
* TestWebKitAPI/glib/TestExpectations.json:
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (278935 => 278936)
--- trunk/Source/WTF/ChangeLog 2021-06-16 15:16:40 UTC (rev 278935)
+++ trunk/Source/WTF/ChangeLog 2021-06-16 15:41:15 UTC (rev 278936)
@@ -1,3 +1,24 @@
+2021-06-16 Alicia Boya García <ab...@igalia.com>
+
+ [WTF] DataMutex: Assert on double locking on the same thread
+ https://bugs.webkit.org/show_bug.cgi?id=227069
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ DataMutex used to use OwnerAwareLock to track what thread is holding
+ the mutex and emit assertion errors if a thread is found attempting to
+ lock a mutex held by that same thread. This turns deadlocks into
+ runtime errors.
+
+ OwnerAwareLock was removed when DataMutex got clang thread safety
+ annotations. This patch reintroduces the same logic, while keeping
+ thread-safety annotations.
+
+ This fixes WTF_DataMutex.DoubleLockDeathTest, which tested this
+ functionality and was previously regressed.
+
+ * wtf/DataMutex.h:
+
2021-06-06 Darin Adler <da...@apple.com>
Delete some recently-obsoleted files
Modified: trunk/Source/WTF/wtf/DataMutex.h (278935 => 278936)
--- trunk/Source/WTF/wtf/DataMutex.h 2021-06-16 15:16:40 UTC (rev 278935)
+++ trunk/Source/WTF/wtf/DataMutex.h 2021-06-16 15:41:15 UTC (rev 278936)
@@ -57,6 +57,9 @@
Lock m_mutex;
T m_data WTF_GUARDED_BY_LOCK(m_mutex);
+#if ENABLE_DATA_MUTEX_CHECKS
+ Thread* m_currentMutexHolder { nullptr };
+#endif
};
template <typename T>
@@ -65,15 +68,13 @@
explicit DataMutexLocker(DataMutex<T>& dataMutex) WTF_ACQUIRES_LOCK(m_dataMutex.m_mutex)
: m_dataMutex(dataMutex)
{
- mutex().lock();
- m_isLocked = true;
+ lock();
}
~DataMutexLocker() WTF_RELEASES_LOCK()
{
if (m_isLocked) {
- assertIsHeld(m_dataMutex.m_mutex);
- mutex().unlock();
+ unlock();
}
}
@@ -101,9 +102,8 @@
// Run-time checks are still performed if enabled.
void unlockEarly() WTF_RELEASES_LOCK(m_dataMutex.m_mutex)
{
- DATA_MUTEX_CHECK(mutex().isHeld());
- m_isLocked = false;
- mutex().unlock();
+ assertIsHeld(m_dataMutex.m_mutex);
+ unlock();
}
// Used to avoid excessive brace scoping when only small parts of the code need to be run unlocked.
@@ -111,16 +111,35 @@
// 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) WTF_IGNORES_THREAD_SAFETY_ANALYSIS
{
- DATA_MUTEX_CHECK(mutex().isHeld());
- assertIsHeld(m_dataMutex.m_mutex);
- mutex().unlock();
+ unlock();
callback();
- mutex().lock();
+ lock();
}
private:
DataMutex<T>& m_dataMutex;
bool m_isLocked { false };
+
+ void lock() WTF_ACQUIRES_LOCK(m_dataMutex.m_mutex)
+ {
+ DATA_MUTEX_CHECK(m_dataMutex.m_currentMutexHolder != &Thread::current()); // Thread attempted recursive lock on non-recursive lock.
+ mutex().lock();
+ m_isLocked = true;
+#if ENABLE_DATA_MUTEX_CHECKS
+ m_dataMutex.m_currentMutexHolder = &Thread::current();
+#endif
+ }
+
+ void unlock() WTF_RELEASES_LOCK(m_dataMutex.m_mutex)
+ {
+ DATA_MUTEX_CHECK(mutex().isHeld());
+ assertIsHeld(m_dataMutex.m_mutex);
+#if ENABLE_DATA_MUTEX_CHECKS
+ m_dataMutex.m_currentMutexHolder = nullptr;
+#endif
+ m_isLocked = false;
+ mutex().unlock();
+ }
};
} // namespace WTF
Modified: trunk/Tools/ChangeLog (278935 => 278936)
--- trunk/Tools/ChangeLog 2021-06-16 15:16:40 UTC (rev 278935)
+++ trunk/Tools/ChangeLog 2021-06-16 15:41:15 UTC (rev 278936)
@@ -1,3 +1,14 @@
+2021-06-16 Alicia Boya García <ab...@igalia.com>
+
+ [WTF] DataMutex: Assert on double locking on the same thread
+ https://bugs.webkit.org/show_bug.cgi?id=227069
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ Update expectations to include the fixed test.
+
+ * TestWebKitAPI/glib/TestExpectations.json:
+
2021-06-15 Alex Christensen <achristen...@webkit.org>
Allow legacy SecurityOrigin behavior for x-apple-ql-id2 scheme
Modified: trunk/Tools/TestWebKitAPI/glib/TestExpectations.json (278935 => 278936)
--- trunk/Tools/TestWebKitAPI/glib/TestExpectations.json 2021-06-16 15:16:40 UTC (rev 278935)
+++ trunk/Tools/TestWebKitAPI/glib/TestExpectations.json 2021-06-16 15:41:15 UTC (rev 278936)
@@ -289,9 +289,6 @@
},
"WTF.WorkerPoolDecrease": {
"expected": {"gtk": {"status": ["TIMEOUT", "PASS"], "bug": "webkit.org/b/214803"}}
- },
- "WTF_DataMutex.DoubleLockDeathTest": {
- "expected": {"gtk": {"status": ["TIMEOUT"], "bug": "webkit.org/b/227021"}}
}
}
},
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
