Changeset 161999 in webkit

Timestamp:

Jan 14, 2014 1:38:38 PM (10 years ago)

Author:

andersca@apple.com

Message:

Get rid of ThreadRestrictionVerifier
​https://bugs.webkit.org/show_bug.cgi?id=127004

Reviewed by Sam Weinig.

Source/WebCore:

Remove now unneeded calls.

• loader/icon/IconDatabase.cpp:

(WebCore::IconDatabase::defaultIcon):
(WebCore::IconDatabase::setIconDataForIconURL):
(WebCore::IconDatabase::getOrCreateIconRecord):
(WebCore::IconDatabase::readFromDatabase):

Source/WTF:

The ThreadRestrictionVerifier mechanism has been turned off for a while
since it doesn't really have a good solution for objects that are passed between threads.
Instead of having dead code in the tree, just get rid of it.

• GNUmakefile.list.am:
• WTF.vcxproj/WTF.vcxproj:
• WTF.vcxproj/WTF.vcxproj.filters:
• WTF.xcodeproj/project.pbxproj:
• wtf/CMakeLists.txt:
• wtf/RefCounted.h:

(WTF::RefCountedBase::ref):
(WTF::RefCountedBase::hasOneRef):
(WTF::RefCountedBase::refCount):
(WTF::RefCountedBase::derefBase):

• wtf/ThreadRestrictionVerifier.h: Removed.

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/GNUmakefile.list.am (modified) (1 diff)
• WTF/WTF.vcxproj/WTF.vcxproj (modified) (1 diff)
• WTF/WTF.vcxproj/WTF.vcxproj.filters (modified) (1 diff)
• WTF/WTF.xcodeproj/project.pbxproj (modified) (4 diffs)
• WTF/wtf/CMakeLists.txt (modified) (1 diff)
• WTF/wtf/RefCounted.h (modified) (8 diffs)
• WTF/wtf/ThreadRestrictionVerifier.h (deleted)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/icon/IconDatabase.cpp (modified) (4 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2014-01-14  Anders Carlsson  <andersca@apple.com>
+
+        Get rid of ThreadRestrictionVerifier
+        https://bugs.webkit.org/show_bug.cgi?id=127004
+
+        Reviewed by Sam Weinig.
+
+        The ThreadRestrictionVerifier mechanism has been turned off for a while
+        since it doesn't really have a good solution for objects that are passed between threads.
+        Instead of having dead code in the tree, just get rid of it.
+        
+        * GNUmakefile.list.am:
+        * WTF.vcxproj/WTF.vcxproj:
+        * WTF.vcxproj/WTF.vcxproj.filters:
+        * WTF.xcodeproj/project.pbxproj:
+        * wtf/CMakeLists.txt:
+        * wtf/RefCounted.h:
+        (WTF::RefCountedBase::ref):
+        (WTF::RefCountedBase::hasOneRef):
+        (WTF::RefCountedBase::refCount):
+        (WTF::RefCountedBase::derefBase):
+        * wtf/ThreadRestrictionVerifier.h: Removed.
+
 2014-01-14  Benjamin Poulain  <benjamin@webkit.org>
 

trunk/Source/WTF/GNUmakefile.list.am

@@ -170 +170 @@
     Source/WTF/wtf/ThreadIdentifierDataPthreads.cpp \
     Source/WTF/wtf/ThreadIdentifierDataPthreads.h \
-    Source/WTF/wtf/ThreadRestrictionVerifier.h \
     Source/WTF/wtf/ThreadSafeRefCounted.h \
     Source/WTF/wtf/ThreadSpecific.h \

trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj

@@ -296 +296 @@
     <ClInclude Include="..\wtf\Threading.h" />
     <ClInclude Include="..\wtf\ThreadingPrimitives.h" />
-    <ClInclude Include="..\wtf\ThreadRestrictionVerifier.h" />
     <ClInclude Include="..\wtf\threadsafeRefCounted.h" />
     <ClInclude Include="..\wtf\threadspecific.h" />

trunk/Source/WTF/WTF.vcxproj/WTF.vcxproj.filters

@@ -650 +650 @@
     </ClInclude>
     <ClInclude Include="..\wtf\ThreadingPrimitives.h">
-      <Filter>wtf</Filter>
-    </ClInclude>
-    <ClInclude Include="..\wtf\ThreadRestrictionVerifier.h">
       <Filter>wtf</Filter>
     </ClInclude>

trunk/Source/WTF/WTF.xcodeproj/project.pbxproj

@@ -247 +247 @@
                 A8A4744D151A825B004123FF /* ThreadingPrimitives.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47335151A825B004123FF /* ThreadingPrimitives.h */; };
                 A8A4744E151A825B004123FF /* ThreadingPthreads.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A47336151A825B004123FF /* ThreadingPthreads.cpp */; };
-                A8A47450151A825B004123FF /* ThreadRestrictionVerifier.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47338151A825B004123FF /* ThreadRestrictionVerifier.h */; };
                 A8A47451151A825B004123FF /* BinarySemaphore.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A8A4733A151A825B004123FF /* BinarySemaphore.cpp */; };
                 A8A47452151A825B004123FF /* BinarySemaphore.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A4733B151A825B004123FF /* BinarySemaphore.h */; };
@@ -525 +524 @@
                 A8A47335151A825B004123FF /* ThreadingPrimitives.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ThreadingPrimitives.h; sourceTree = "<group>"; };
                 A8A47336151A825B004123FF /* ThreadingPthreads.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ThreadingPthreads.cpp; sourceTree = "<group>"; };
-                A8A47338151A825B004123FF /* ThreadRestrictionVerifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ThreadRestrictionVerifier.h; sourceTree = "<group>"; };
                 A8A4733A151A825B004123FF /* BinarySemaphore.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BinarySemaphore.cpp; sourceTree = "<group>"; };
                 A8A4733B151A825B004123FF /* BinarySemaphore.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BinarySemaphore.h; sourceTree = "<group>"; };
@@ -828 +826 @@
                                 A8A47335151A825B004123FF /* ThreadingPrimitives.h */,
                                 A8A47336151A825B004123FF /* ThreadingPthreads.cpp */,
-                                A8A47338151A825B004123FF /* ThreadRestrictionVerifier.h */,
                                 A8A4733E151A825B004123FF /* ThreadSafeRefCounted.h */,
                                 A8A4733F151A825B004123FF /* ThreadSpecific.h */,
@@ -1118 +1115 @@
                                 A8A4744B151A825B004123FF /* Threading.h in Headers */,
                                 A8A4744D151A825B004123FF /* ThreadingPrimitives.h in Headers */,
-                                A8A47450151A825B004123FF /* ThreadRestrictionVerifier.h in Headers */,
                                 A8A47454151A825B004123FF /* ThreadSafeRefCounted.h in Headers */,
                                 A8A47455151A825B004123FF /* ThreadSpecific.h in Headers */,

trunk/Source/WTF/wtf/CMakeLists.txt

@@ -102 +102 @@
     TCSystemAlloc.h
     ThreadIdentifierDataPthreads.h
-    ThreadRestrictionVerifier.h
     ThreadSafeRefCounted.h
     ThreadSpecific.h

trunk/Source/WTF/wtf/RefCounted.h

@@ -26 +26 @@
 #include <wtf/Noncopyable.h>
 #include <wtf/OwnPtr.h>
-#include <wtf/ThreadRestrictionVerifier.h>
 
 namespace WTF {
@@ -44 +43 @@
     {
 #if CHECK_REF_COUNTED_LIFECYCLE
-        // Start thread verification as soon as the ref count gets to 2. This
-        // heuristic reflects the fact that items are often created on one thread
-        // and then given to another thread to be used.
-        // FIXME: Make this restriction tigher. Especially as we move to more
-        // common methods for sharing items across threads like CrossThreadCopier.h
-        // We should be able to add a "detachFromThread" method to make this explicit.
-        if (m_refCount == 1)
-            m_verifier.setShared(true);
-        // If this assert fires, it either indicates a thread safety issue or
-        // that the verification needs to change. See ThreadRestrictionVerifier for
-        // the different modes.
-        ASSERT(m_verifier.isSafeToUse());
         ASSERT(!m_deletionHasBegun);
         ASSERT(!m_adoptionIsRequired);
@@ -65 +52 @@
     {
 #if CHECK_REF_COUNTED_LIFECYCLE
-        ASSERT(m_verifier.isSafeToUse());
         ASSERT(!m_deletionHasBegun);
 #endif
@@ -73 +59 @@
     unsigned refCount() const
     {
-#if CHECK_REF_COUNTED_LIFECYCLE
-        ASSERT(m_verifier.isSafeToUse());
-#endif
         return m_refCount;
-    }
-
-    void setMutexForVerifier(Mutex&);
-
-#if HAVE(DISPATCH_H)
-    void setDispatchQueueForVerifier(dispatch_queue_t);
-#endif
-
-    // Turns off verification. Use of this method is discouraged (instead extend
-    // ThreadRestrictionVerifier to verify your case).
-    // NB. It is necessary to call this in the constructor of many objects in
-    // JavaScriptCore, because JavaScriptCore objects may be used from multiple
-    // threads even if the reference counting is done in a racy manner. This is
-    // because a JSC instance may be used from multiple threads so long as all
-    // accesses into that instance are protected by a per-instance lock. It would
-    // be absolutely wrong to prohibit this pattern, and it would be a disastrous
-    // regression to require that the objects within that instance use a thread-
-    // safe version of reference counting.
-    void turnOffVerifier()
-    {
-#if CHECK_REF_COUNTED_LIFECYCLE
-        m_verifier.turnOffVerification();
-#endif
     }
 
@@ -133 +93 @@
     {
 #if CHECK_REF_COUNTED_LIFECYCLE
-        ASSERT(m_verifier.isSafeToUse());
         ASSERT(!m_deletionHasBegun);
         ASSERT(!m_adoptionIsRequired);
@@ -147 +106 @@
         }
         m_refCount = tempRefCount;
-
-#if CHECK_REF_COUNTED_LIFECYCLE
-        // Stop thread verification when the ref goes to 1 because it
-        // is safe to be passed to another thread at this point.
-        if (m_refCount == 1)
-            m_verifier.setShared(false);
-#endif
         return false;
     }
@@ -174 +126 @@
     bool m_deletionHasBegun;
     bool m_adoptionIsRequired;
-    ThreadRestrictionVerifier m_verifier;
 #endif
 };
@@ -220 +171 @@
 };
 
-#if CHECK_REF_COUNTED_LIFECYCLE
-inline void RefCountedBase::setMutexForVerifier(Mutex& mutex)
-{
-    m_verifier.setMutexMode(mutex);
-}
-#else
-inline void RefCountedBase::setMutexForVerifier(Mutex&) { }
-#endif
-
-#if HAVE(DISPATCH_H)
-#if CHECK_REF_COUNTED_LIFECYCLE
-inline void RefCountedBase::setDispatchQueueForVerifier(dispatch_queue_t queue)
-{
-    m_verifier.setDispatchQueueMode(queue);
-}
-#else
-inline void RefCountedBase::setDispatchQueueForVerifier(dispatch_queue_t) { }
-#endif
-#endif // HAVE(DISPATCH_H)
-
 } // namespace WTF
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-14  Anders Carlsson  <andersca@apple.com>
+
+        Get rid of ThreadRestrictionVerifier
+        https://bugs.webkit.org/show_bug.cgi?id=127004
+
+        Reviewed by Sam Weinig.
+
+        Remove now unneeded calls.
+
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::defaultIcon):
+        (WebCore::IconDatabase::setIconDataForIconURL):
+        (WebCore::IconDatabase::getOrCreateIconRecord):
+        (WebCore::IconDatabase::readFromDatabase):
+
 2014-01-14  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebCore/loader/icon/IconDatabase.cpp

@@ -403 +403 @@
     if (!m_defaultIconRecord) {
         m_defaultIconRecord = IconRecord::create("urlIcon");
-        m_defaultIconRecord->setMutexForVerifier(m_urlAndIconLock);
         loadDefaultIconRecord(m_defaultIconRecord.get());
     }
@@ -545 +544 @@
     
     RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0);
-    if (data)
-        data->setMutexForVerifier(m_urlAndIconLock);
     String iconURL = iconURLOriginal.isolatedCopy();
     
@@ -910 +907 @@
 
     RefPtr<IconRecord> newIcon = IconRecord::create(iconURL);
-    newIcon->setMutexForVerifier(m_urlAndIconLock);
     m_iconURLToRecordMap.set(iconURL, newIcon.get());
 
@@ -1507 +1503 @@
         didAnyWork = true;
         RefPtr<SharedBuffer> imageData = getImageDataForIconURLFromSQLDatabase(icons[i]->iconURL());
-        imageData->setMutexForVerifier(m_urlAndIconLock);
 
         // Verify this icon still wants to be read from disk