Title: [264078] releases/WebKitGTK/webkit-2.28
- Revision
- 264078
- Author
- [email protected]
- Date
- 2020-07-08 03:06:49 -0700 (Wed, 08 Jul 2020)
Log Message
Merge r259879 - [WTF] DataMutex: Add runUnlocked()
https://bugs.webkit.org/show_bug.cgi?id=209811
Reviewed by Xabier Rodriguez-Calvar.
Source/WTF:
This patch introduces a runUnlocked() method in WTF::DataMutex::LockedWrapper
to run a lambda function without the lock. This is intended to be used for
small sections of the code that need to be unlocked, in cases where using
scoping would prove non-ergonomic or where running the unlocked section is only
necessary or desired when a certain condition is met -- something that cannot
be done with C++ scoping.
Safety mechanisms are provided. First, because this is used with a lambda, all
variables to be used in the unlocked section have to be specified in the
capture (global capture is possible but not recommended to simplify analysis).
Second, additional checks have been added to DataMutex to detect unlocked
accesses among other conditions. This will detect among other things naive
access to protected members by means of capturing the LockedWrapper by
reference.
* wtf/DataMutex.h:
(WTF::OwnerAwareLockAdapter::lock):
(WTF::OwnerAwareLockAdapter::unlock):
(WTF::OwnerAwareLockAdapter::tryLock):
(WTF::OwnerAwareLockAdapter::isLocked const):
(WTF::DataMutex::LockedWrapper::operator->):
(WTF::DataMutex::LockedWrapper::operator*):
(WTF::DataMutex::LockedWrapper::mutex):
(WTF::DataMutex::LockedWrapper::lockHolder):
(WTF::DataMutex::LockedWrapper::runUnlocked):
Tools:
Tests for runUnlocked() and DataMutex checks are introduced.
* TestWebKitAPI/Tests/WTF/DataMutex.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.28/Source/WTF/ChangeLog (264077 => 264078)
--- releases/WebKitGTK/webkit-2.28/Source/WTF/ChangeLog 2020-07-08 10:06:43 UTC (rev 264077)
+++ releases/WebKitGTK/webkit-2.28/Source/WTF/ChangeLog 2020-07-08 10:06:49 UTC (rev 264078)
@@ -1,3 +1,36 @@
+2020-04-10 Alicia Boya García <[email protected]>
+
+ [WTF] DataMutex: Add runUnlocked()
+ https://bugs.webkit.org/show_bug.cgi?id=209811
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ This patch introduces a runUnlocked() method in WTF::DataMutex::LockedWrapper
+ to run a lambda function without the lock. This is intended to be used for
+ small sections of the code that need to be unlocked, in cases where using
+ scoping would prove non-ergonomic or where running the unlocked section is only
+ necessary or desired when a certain condition is met -- something that cannot
+ be done with C++ scoping.
+
+ Safety mechanisms are provided. First, because this is used with a lambda, all
+ variables to be used in the unlocked section have to be specified in the
+ capture (global capture is possible but not recommended to simplify analysis).
+ Second, additional checks have been added to DataMutex to detect unlocked
+ accesses among other conditions. This will detect among other things naive
+ access to protected members by means of capturing the LockedWrapper by
+ reference.
+
+ * wtf/DataMutex.h:
+ (WTF::OwnerAwareLockAdapter::lock):
+ (WTF::OwnerAwareLockAdapter::unlock):
+ (WTF::OwnerAwareLockAdapter::tryLock):
+ (WTF::OwnerAwareLockAdapter::isLocked const):
+ (WTF::DataMutex::LockedWrapper::operator->):
+ (WTF::DataMutex::LockedWrapper::operator*):
+ (WTF::DataMutex::LockedWrapper::mutex):
+ (WTF::DataMutex::LockedWrapper::lockHolder):
+ (WTF::DataMutex::LockedWrapper::runUnlocked):
+
2020-04-16 Alberto Garcia <[email protected]>
Unreviewed, set CeilingOnPageSize for MIPS64
Modified: releases/WebKitGTK/webkit-2.28/Source/WTF/wtf/DataMutex.h (264077 => 264078)
--- releases/WebKitGTK/webkit-2.28/Source/WTF/wtf/DataMutex.h 2020-07-08 10:06:43 UTC (rev 264077)
+++ releases/WebKitGTK/webkit-2.28/Source/WTF/wtf/DataMutex.h 2020-07-08 10:06:49 UTC (rev 264078)
@@ -21,10 +21,74 @@
#pragma once
#include <wtf/Lock.h>
+#include <wtf/Threading.h>
namespace WTF {
-template<typename T>
+// By default invalid access checks are only done in Debug builds.
+#if !defined(ENABLE_DATA_MUTEX_CHECKS)
+#if defined(NDEBUG)
+#define ENABLE_DATA_MUTEX_CHECKS 0
+#else
+#define ENABLE_DATA_MUTEX_CHECKS 1
+#endif
+#endif
+
+#if ENABLE_DATA_MUTEX_CHECKS
+#define DATA_MUTEX_CHECK(expr) RELEASE_ASSERT(expr)
+#else
+#define DATA_MUTEX_CHECK(expr)
+#endif
+
+template<typename LockType>
+class OwnerAwareLockAdapter {
+public:
+ void lock()
+ {
+ DATA_MUTEX_CHECK(m_owner != &Thread::current()); // Thread attempted recursive lock (unsupported).
+ m_lock.lock();
+#if ENABLE_DATA_MUTEX_CHECKS
+ ASSERT(!m_owner);
+ m_owner = &Thread::current();
+#endif
+ }
+
+ void unlock()
+ {
+#if ENABLE_DATA_MUTEX_CHECKS
+ m_owner = nullptr;
+#endif
+ m_lock.unlock();
+ }
+
+ bool tryLock()
+ {
+ DATA_MUTEX_CHECK(m_owner != &Thread::current()); // Thread attempted recursive lock (unsupported).
+ if (!m_lock.tryLock())
+ return false;
+
+#if ENABLE_DATA_MUTEX_CHECKS
+ ASSERT(!m_owner);
+ m_owner = &Thread::current();
+#endif
+ return true;
+ }
+
+ bool isLocked() const
+ {
+ return m_lock.isLocked();
+ }
+
+private:
+#if ENABLE_DATA_MUTEX_CHECKS
+ Thread* m_owner { nullptr }; // Use Thread* instead of RefPtr<Thread> since m_owner thread is always alive while m_owner is set.
+#endif
+ LockType m_lock;
+};
+
+using OwnerAwareLock = OwnerAwareLockAdapter<Lock>;
+
+template<typename T, typename LockType = OwnerAwareLock>
class DataMutex {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(DataMutex);
@@ -44,32 +108,44 @@
T* operator->()
{
+ DATA_MUTEX_CHECK(m_mutex.isLocked());
return &m_data;
}
T& operator*()
{
+ DATA_MUTEX_CHECK(m_mutex.isLocked());
return m_data;
}
- Lock& mutex()
+ LockType& mutex()
{
return m_mutex;
}
- LockHolder& lockHolder()
+ Locker<LockType>& lockHolder()
{
return m_lockHolder;
}
+ // 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(WTF::Function<void()> callback)
+ {
+ m_mutex.unlock();
+ callback();
+ m_mutex.lock();
+ }
+
private:
- Lock& m_mutex;
- LockHolder m_lockHolder;
+ LockType& m_mutex;
+ Locker<LockType> m_lockHolder;
T& m_data;
};
private:
- Lock m_mutex;
+ LockType m_mutex;
T m_data;
};
Modified: releases/WebKitGTK/webkit-2.28/Tools/ChangeLog (264077 => 264078)
--- releases/WebKitGTK/webkit-2.28/Tools/ChangeLog 2020-07-08 10:06:43 UTC (rev 264077)
+++ releases/WebKitGTK/webkit-2.28/Tools/ChangeLog 2020-07-08 10:06:49 UTC (rev 264078)
@@ -1,3 +1,15 @@
+2020-04-10 Alicia Boya García <[email protected]>
+
+ [WTF] DataMutex: Add runUnlocked()
+ https://bugs.webkit.org/show_bug.cgi?id=209811
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ Tests for runUnlocked() and DataMutex checks are introduced.
+
+ * TestWebKitAPI/Tests/WTF/DataMutex.cpp:
+ (TestWebKitAPI::TEST):
+
2020-03-25 Ting-Wei Lan <[email protected]>
[GTK] Add user agent quirk for auth.mayohr.com
Modified: releases/WebKitGTK/webkit-2.28/Tools/TestWebKitAPI/Tests/WTF/DataMutex.cpp (264077 => 264078)
--- releases/WebKitGTK/webkit-2.28/Tools/TestWebKitAPI/Tests/WTF/DataMutex.cpp 2020-07-08 10:06:43 UTC (rev 264077)
+++ releases/WebKitGTK/webkit-2.28/Tools/TestWebKitAPI/Tests/WTF/DataMutex.cpp 2020-07-08 10:06:49 UTC (rev 264078)
@@ -25,6 +25,9 @@
*/
#include "config.h"
+
+// For this file, we force checks even if we're running on release.
+#define ENABLE_DATA_MUTEX_CHECKS 1
#include <wtf/DataMutex.h>
using namespace WTF;
@@ -34,18 +37,24 @@
struct MyStructure {
int number;
};
-DataMutex<MyStructure> myDataMutex;
TEST(WTF_DataMutex, TakingTheMutex)
{
- Lock* mutex;
+ DataMutex<MyStructure> myDataMutex;
+
+ OwnerAwareLock* mutex;
{
DataMutex<MyStructure>::LockedWrapper wrapper(myDataMutex);
mutex = &wrapper.mutex();
- ASSERT_TRUE(mutex->isHeld());
+ ASSERT_TRUE(mutex->isLocked());
wrapper->number = 5;
+
+ wrapper.runUnlocked([mutex]() {
+ ASSERT_FALSE(mutex->isLocked());
+ });
+ ASSERT_TRUE(mutex->isLocked());
}
- ASSERT_FALSE(mutex->isHeld());
+ ASSERT_FALSE(mutex->isLocked());
{
DataMutex<MyStructure>::LockedWrapper wrapper(myDataMutex);
@@ -53,4 +62,24 @@
}
}
+TEST(WTF_DataMutex, RunUnlockedIllegalAccessDeathTest)
+{
+ ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+ DataMutex<MyStructure> myDataMutex;
+ DataMutex<MyStructure>::LockedWrapper wrapper(myDataMutex);
+ wrapper->number = 5;
+
+ ASSERT_DEATH(wrapper.runUnlocked([&]() {
+ wrapper->number++;
+ }), "");
+}
+
+TEST(WTF_DataMutex, DoubleLockDeathTest)
+{
+ ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+ DataMutex<MyStructure> myDataMutex;
+ DataMutex<MyStructure>::LockedWrapper wrapper1(myDataMutex);
+ ASSERT_DEATH(DataMutex<MyStructure>::LockedWrapper wrapper2(myDataMutex), "");
+}
+
} // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
