Changeset 248544 in webkit

Timestamp:

Aug 12, 2019 1:15:26 PM (5 years ago)

Author:

Chris Dumez

Message:

Unreviewed, fix post landing review comments for r248533 from Darin.

• wtf/RefCounted.h:

(WTF::RefCountedBase::ref const):
(WTF::RefCountedBase::applyRefDerefThreadingCheck const):
(WTF::RefCountedBase::derefBase const):
(WTF::RefCountedBase::areThreadingCheckedEnabled const): Deleted.

Location:

trunk/Source/WTF

Files:

• ChangeLog (modified) (1 diff)
• wtf/RefCounted.h (modified) (3 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2019-08-12  Chris Dumez  <cdumez@apple.com>
+
+        Unreviewed, fix post landing review comments for r248533 from Darin.
+
+        * wtf/RefCounted.h:
+        (WTF::RefCountedBase::ref const):
+        (WTF::RefCountedBase::applyRefDerefThreadingCheck const):
+        (WTF::RefCountedBase::derefBase const):
+        (WTF::RefCountedBase::areThreadingCheckedEnabled const): Deleted.
+
 2019-08-12  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WTF/wtf/RefCounted.h

@@ -41 +41 @@
     void ref() const
     {
-#if !ASSERT_DISABLED
-        if (m_isOwnedByMainThread != isMainThread() && hasOneRef())
-            m_isOwnedByMainThread = isMainThread(); // Likely ownership transfer.
-
-        // If you hit this assertion, it means that the RefCounted object was ref'd or deref'd
-        // concurrent from several threads, which is not safe. You should either subclass
-        // ThreadSafeRefCounted instead, or make sure to always ref / deref from the same thread.
-        ASSERT_WITH_MESSAGE(!areThreadingCheckedEnabled() || m_isOwnedByMainThread == isMainThread(), "Should not be ref'd / deref'd concurrently from several threads");
-#endif
+        applyRefDerefThreadingCheck();
 
 #if CHECK_REF_COUNTED_LIFECYCLE
@@ -109 +101 @@
     }
 
-#if !ASSERT_DISABLED
-    bool areThreadingCheckedEnabled() const
-    {
-        return areThreadingChecksEnabledGlobally && m_areThreadingChecksEnabled;
-    }
-#endif
+    void applyRefDerefThreadingCheck() const
+    {
+#if !ASSERT_DISABLED
+        if (hasOneRef()) {
+            // Likely an ownership transfer across threads that may be safe.
+            m_isOwnedByMainThread = isMainThread();
+        } else if (areThreadingChecksEnabledGlobally && m_areThreadingChecksEnabled) {
+            // If you hit this assertion, it means that the RefCounted object was ref/deref'd
+            // from both the main thread and another in a way that is likely concurrent and unsafe.
+            // Derive from ThreadSafeRefCounted and make sure the destructor is safe on threads
+            // that call deref, or ref/deref from a single thread.
+            ASSERT_WITH_MESSAGE(m_isOwnedByMainThread == isMainThread(), "Unsafe to ref/deref from different threads");
+        }
+#endif
+    }
 
     ~RefCountedBase()
@@ -127 +128 @@
     bool derefBase() const
     {
-#if !ASSERT_DISABLED
-        if (m_isOwnedByMainThread != isMainThread() && hasOneRef())
-            m_isOwnedByMainThread = isMainThread(); // Likely ownership transfer.
-
-        // If you hit this assertion, it means that the RefCounted object was ref'd or deref'd
-        // concurrent from several threads, which is not safe. You should either subclass
-        // ThreadSafeRefCounted instead, or make sure to always ref / deref from the same thread.
-        ASSERT_WITH_MESSAGE(!areThreadingCheckedEnabled() || m_isOwnedByMainThread == isMainThread(), "Should not be ref'd / deref'd concurrently from several threads");
-#endif
+        applyRefDerefThreadingCheck();
 
 #if CHECK_REF_COUNTED_LIFECYCLE