Commit 98621338 authored by andersca@apple.com's avatar andersca@apple.com

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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@161999 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 35a7be70
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>
Enable the experimental selector code generator by default
......
......@@ -169,7 +169,6 @@ wtf_sources += \
Source/WTF/wtf/ThreadFunctionInvocation.h \
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 \
Source/WTF/wtf/ThreadSpecificWin.cpp \
......
......@@ -295,7 +295,6 @@
<ClInclude Include="..\wtf\text\WTFString.h" />
<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" />
<ClInclude Include="..\wtf\threads\BinarySemaphore.h" />
......
......@@ -651,9 +651,6 @@
<ClInclude Include="..\wtf\ThreadingPrimitives.h">
<Filter>wtf</Filter>
</ClInclude>
<ClInclude Include="..\wtf\ThreadRestrictionVerifier.h">
<Filter>wtf</Filter>
</ClInclude>
<ClInclude Include="..\wtf\ValueCheck.h">
<Filter>wtf</Filter>
</ClInclude>
......
......@@ -246,7 +246,6 @@
A8A4744B151A825B004123FF /* Threading.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A47333151A825B004123FF /* Threading.h */; };
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 */; };
A8A47454151A825B004123FF /* ThreadSafeRefCounted.h in Headers */ = {isa = PBXBuildFile; fileRef = A8A4733E151A825B004123FF /* ThreadSafeRefCounted.h */; };
......@@ -524,7 +523,6 @@
A8A47333151A825B004123FF /* Threading.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Threading.h; sourceTree = "<group>"; };
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>"; };
A8A4733E151A825B004123FF /* ThreadSafeRefCounted.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ThreadSafeRefCounted.h; sourceTree = "<group>"; };
......@@ -827,7 +825,6 @@
A8A47333151A825B004123FF /* Threading.h */,
A8A47335151A825B004123FF /* ThreadingPrimitives.h */,
A8A47336151A825B004123FF /* ThreadingPthreads.cpp */,
A8A47338151A825B004123FF /* ThreadRestrictionVerifier.h */,
A8A4733E151A825B004123FF /* ThreadSafeRefCounted.h */,
A8A4733F151A825B004123FF /* ThreadSpecific.h */,
149EF16216BBFE0D000A4331 /* TriState.h */,
......@@ -1117,7 +1114,6 @@
A8A47449151A825B004123FF /* ThreadIdentifierDataPthreads.h in Headers */,
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 */,
149EF16316BBFE0D000A4331 /* TriState.h in Headers */,
......
......@@ -101,7 +101,6 @@ set(WTF_HEADERS
TCSpinLock.h
TCSystemAlloc.h
ThreadIdentifierDataPthreads.h
ThreadRestrictionVerifier.h
ThreadSafeRefCounted.h
ThreadSpecific.h
Threading.h
......
......@@ -25,7 +25,6 @@
#include <wtf/FastMalloc.h>
#include <wtf/Noncopyable.h>
#include <wtf/OwnPtr.h>
#include <wtf/ThreadRestrictionVerifier.h>
namespace WTF {
......@@ -43,18 +42,6 @@ public:
void ref()
{
#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);
#endif
......@@ -64,7 +51,6 @@ public:
bool hasOneRef() const
{
#if CHECK_REF_COUNTED_LIFECYCLE
ASSERT(m_verifier.isSafeToUse());
ASSERT(!m_deletionHasBegun);
#endif
return m_refCount == 1;
......@@ -72,35 +58,9 @@ public:
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
}
void relaxAdoptionRequirement()
{
#if CHECK_REF_COUNTED_LIFECYCLE
......@@ -132,7 +92,6 @@ protected:
bool derefBase()
{
#if CHECK_REF_COUNTED_LIFECYCLE
ASSERT(m_verifier.isSafeToUse());
ASSERT(!m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
#endif
......@@ -146,13 +105,6 @@ protected:
return true;
}
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;
}
......@@ -173,7 +125,6 @@ private:
#if CHECK_REF_COUNTED_LIFECYCLE
bool m_deletionHasBegun;
bool m_adoptionIsRequired;
ThreadRestrictionVerifier m_verifier;
#endif
};
......@@ -219,26 +170,6 @@ protected:
}
};
#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
using WTF::RefCounted;
......
/*
* Copyright (C) 2011 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
* met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following disclaimer
* in the documentation and/or other materials provided with the
* distribution.
* * Neither the name of Google Inc. nor the names of its
* contributors may be used to endorse or promote products derived from
* this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#ifndef ThreadRestrictionVerifier_h
#define ThreadRestrictionVerifier_h
#include <wtf/Assertions.h>
#include <wtf/Threading.h>
#include <wtf/ThreadingPrimitives.h>
#if HAVE(DISPATCH_H)
#include <dispatch/dispatch.h>
#endif
#ifndef NDEBUG
namespace WTF {
// Verifies that a class is used in a way that respects its lack of thread-safety.
// The default mode is to verify that the object will only be used on a single thread. The
// thread gets captured when setShared(true) is called.
// The mode may be changed by calling useMutexMode (or turnOffVerification).
// FIXME: This verifier is switched off because it fires false positives for
// objects that are used on multiple threads. Instead of an opt-out verifier,
// we probably need an opt-in verifier that marks select objects as being
// tied to select threads.
#if 0
class ThreadRestrictionVerifier {
public:
ThreadRestrictionVerifier()
: m_mode(SingleThreadVerificationMode)
, m_shared(false)
, m_owningThread(0)
, m_mutex(0)
#if HAVE(DISPATCH_H)
, m_owningQueue(0)
#endif
{
}
#if HAVE(DISPATCH_H)
~ThreadRestrictionVerifier()
{
if (m_owningQueue)
dispatch_release(m_owningQueue);
}
#endif
void setMutexMode(Mutex& mutex)
{
m_mode = MutexVerificationMode;
m_mutex = &mutex;
}
#if HAVE(DISPATCH_H)
void setDispatchQueueMode(dispatch_queue_t queue)
{
m_mode = SingleDispatchQueueVerificationMode;
m_owningQueue = queue;
dispatch_retain(m_owningQueue);
}
#endif
void turnOffVerification()
{
m_mode = NoVerificationMode;
}
// Indicates that the object may (or may not) be owned by more than one place.
void setShared(bool shared)
{
#if !ASSERT_DISABLED
bool previouslyShared = m_shared;
#endif
m_shared = shared;
if (!m_shared)
return;
switch (m_mode) {
case SingleThreadVerificationMode:
ASSERT(shared != previouslyShared);
// Capture the current thread to verify that subsequent ref/deref happen on this thread.
m_owningThread = currentThread();
return;
#if HAVE(DISPATCH_H)
case SingleDispatchQueueVerificationMode:
#endif
case MutexVerificationMode:
case NoVerificationMode:
return;
}
ASSERT_NOT_REACHED();
}
// Is it OK to use the object at this moment on the current thread?
bool isSafeToUse() const
{
if (!m_shared)
return true;
switch (m_mode) {
case SingleThreadVerificationMode:
return m_owningThread == currentThread();
case MutexVerificationMode:
if (!m_mutex->tryLock())
return true;
m_mutex->unlock();
return false;
#if HAVE(DISPATCH_H)
case SingleDispatchQueueVerificationMode:
return m_owningQueue == dispatch_get_current_queue();
#endif
case NoVerificationMode:
return true;
}
ASSERT_NOT_REACHED();
return true;
}
private:
enum VerificationMode {
SingleThreadVerificationMode,
MutexVerificationMode,
NoVerificationMode,
#if HAVE(DISPATCH_H)
SingleDispatchQueueVerificationMode,
#endif
};
VerificationMode m_mode;
bool m_shared;
// Used by SingleThreadVerificationMode
ThreadIdentifier m_owningThread;
// Used by MutexVerificationMode.
Mutex* m_mutex;
#if HAVE(DISPATCH_H)
// Used by SingleDispatchQueueVerificationMode.
dispatch_queue_t m_owningQueue;
#endif
};
#else
class ThreadRestrictionVerifier {
public:
ThreadRestrictionVerifier()
{
}
void setMutexMode(Mutex&)
{
}
#if HAVE(DISPATCH_H)
void setDispatchQueueMode(dispatch_queue_t)
{
}
#endif
void turnOffVerification()
{
}
// Indicates that the object may (or may not) be owned by more than one place.
void setShared(bool)
{
}
// Is it OK to use the object at this moment on the current thread?
bool isSafeToUse() const
{
return true;
}
};
#endif
}
#endif
#endif
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>
[iOS] Directly allocate NSMutableDictionary in QLPreviewConverterDictionary() and QLContentDictionary()
......@@ -402,7 +402,6 @@ Image* IconDatabase::defaultIcon(const IntSize& size)
if (!m_defaultIconRecord) {
m_defaultIconRecord = IconRecord::create("urlIcon");
m_defaultIconRecord->setMutexForVerifier(m_urlAndIconLock);
loadDefaultIconRecord(m_defaultIconRecord.get());
}
......@@ -544,8 +543,6 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal,
return;
RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0);
if (data)
data->setMutexForVerifier(m_urlAndIconLock);
String iconURL = iconURLOriginal.isolatedCopy();
Vector<String> pageURLs;
......@@ -909,7 +906,6 @@ PassRefPtr<IconRecord> IconDatabase::getOrCreateIconRecord(const String& iconURL
return icon;
RefPtr<IconRecord> newIcon = IconRecord::create(iconURL);
newIcon->setMutexForVerifier(m_urlAndIconLock);
m_iconURLToRecordMap.set(iconURL, newIcon.get());
return newIcon.release();
......@@ -1506,7 +1502,6 @@ bool IconDatabase::readFromDatabase()
for (unsigned i = 0; i < icons.size(); ++i) {
didAnyWork = true;
RefPtr<SharedBuffer> imageData = getImageDataForIconURLFromSQLDatabase(icons[i]->iconURL());
imageData->setMutexForVerifier(m_urlAndIconLock);
// Verify this icon still wants to be read from disk
{
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment