Diff
Modified: trunk/Source/_javascript_Core/API/JSContextRef.cpp (248532 => 248533)
--- trunk/Source/_javascript_Core/API/JSContextRef.cpp 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/_javascript_Core/API/JSContextRef.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -66,6 +66,7 @@
JSContextGroupRef JSContextGroupCreate()
{
+ WTF::initializeMainThread();
initializeThreading();
return toRef(&VM::createContextGroup().leakRef());
}
@@ -116,6 +117,7 @@
JSGlobalContextRef JSGlobalContextCreate(JSClassRef globalObjectClass)
{
+ WTF::initializeMainThread();
initializeThreading();
#if OS(DARWIN)
@@ -131,6 +133,7 @@
JSGlobalContextRef JSGlobalContextCreateInGroup(JSContextGroupRef group, JSClassRef globalObjectClass)
{
+ WTF::initializeMainThread();
initializeThreading();
Ref<VM> vm = group ? Ref<VM>(*toJS(group)) : VM::createContextGroup();
Modified: trunk/Source/_javascript_Core/ChangeLog (248532 => 248533)
--- trunk/Source/_javascript_Core/ChangeLog 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-08-12 18:10:26 UTC (rev 248533)
@@ -1,5 +1,17 @@
2019-08-12 Chris Dumez <[email protected]>
+ Add threading assertions to RefCounted
+ https://bugs.webkit.org/show_bug.cgi?id=200507
+
+ Reviewed by Ryosuke Niwa.
+
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::Plan):
+ Disable threading assertions for DFG::Plan::m_inlineCallFrames while the JSC team
+ investigates.
+
+2019-08-12 Chris Dumez <[email protected]>
+
Unreviewed, rolling out r248525.
Revert new threading assertions while I work on fixing the
Modified: trunk/Source/_javascript_Core/b3/testb3_1.cpp (248532 => 248533)
--- trunk/Source/_javascript_Core/b3/testb3_1.cpp 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/_javascript_Core/b3/testb3_1.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -902,6 +902,7 @@
break;
}
+ WTF::initializeMainThread();
JSC::initializeThreading();
for (unsigned i = 0; i <= 2; ++i) {
Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.cpp (248532 => 248533)
--- trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -150,6 +150,7 @@
, m_stage(Preparing)
{
RELEASE_ASSERT(m_codeBlock->alternative()->jitCode());
+ m_inlineCallFrames->disableThreadingChecks();
}
Plan::~Plan()
Modified: trunk/Source/WTF/ChangeLog (248532 => 248533)
--- trunk/Source/WTF/ChangeLog 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WTF/ChangeLog 2019-08-12 18:10:26 UTC (rev 248533)
@@ -1,5 +1,37 @@
2019-08-12 Chris Dumez <[email protected]>
+ Add threading assertions to RefCounted
+ https://bugs.webkit.org/show_bug.cgi?id=200507
+
+ Reviewed by Ryosuke Niwa.
+
+ Add threading assertions to RefCounted to try and catch unsafe concurrent ref'ing / derefing of
+ RefCounted objects from several threads. If you hit these new assertions, it likely means you either
+ need to:
+ 1. Have your class subclass ThreadSafeRefCounted instead of RefCounted
+ or
+ 2. Make sure your objects always gets ref'd / deref'd from the same thread.
+
+ These assertions already found several thread safety bugs in our code base, which I fixed via
+ dependency bugs.
+
+ These assertions are currently enabled in WebKit (UIProcess, child processes and
+ WebKitLegacy), they do not apply other _javascript_Core API clients.
+
+ * WTF.xcodeproj/project.pbxproj:
+ * wtf/CMakeLists.txt:
+ * wtf/RefCounted.cpp: Added.
+ * wtf/RefCounted.h:
+ (WTF::RefCountedBase::ref const):
+ (WTF::RefCountedBase::disableThreadingChecks):
+ (WTF::RefCountedBase::enableThreadingChecksGlobally):
+ (WTF::RefCountedBase::RefCountedBase):
+ (WTF::RefCountedBase::areThreadingCheckedEnabled const):
+ (WTF::RefCountedBase::derefBase const):
+ * wtf/SizeLimits.cpp:
+
+2019-08-12 Chris Dumez <[email protected]>
+
Unreviewed, rolling out r248525.
Revert new threading assertions while I work on fixing the
Modified: trunk/Source/WTF/WTF.xcodeproj/project.pbxproj (248532 => 248533)
--- trunk/Source/WTF/WTF.xcodeproj/project.pbxproj 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WTF/WTF.xcodeproj/project.pbxproj 2019-08-12 18:10:26 UTC (rev 248533)
@@ -61,6 +61,7 @@
2CDED0F318115C85004DBA70 /* RunLoop.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2CDED0F118115C85004DBA70 /* RunLoop.cpp */; };
3337DB9CE743410FAF076E17 /* StackTrace.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 313EDEC9778E49C9BEA91CFC /* StackTrace.cpp */; };
4427C5AA21F6D6C300A612A4 /* ASCIICType.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4427C5A921F6D6C300A612A4 /* ASCIICType.cpp */; };
+ 46BEB6EB22FFE24900269867 /* RefCounted.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 46BEB6E922FFDDD500269867 /* RefCounted.cpp */; };
50DE35F5215BB01500B979C7 /* ExternalStringImpl.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 50DE35F3215BB01500B979C7 /* ExternalStringImpl.cpp */; };
515F794E1CFC9F4A00CCED93 /* CrossThreadCopier.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 515F794B1CFC9F4A00CCED93 /* CrossThreadCopier.cpp */; };
517F82D71FD22F3000DA3DEA /* CrossThreadTaskHandler.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 517F82D51FD22F2F00DA3DEA /* CrossThreadTaskHandler.cpp */; };
@@ -344,6 +345,7 @@
430B47871AAAAC1A001223DA /* StringCommon.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StringCommon.h; sourceTree = "<group>"; };
4427C5A921F6D6C300A612A4 /* ASCIICType.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ASCIICType.cpp; sourceTree = "<group>"; };
46BA9EAB1F4CD61E009A2BBC /* CompletionHandler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CompletionHandler.h; sourceTree = "<group>"; };
+ 46BEB6E922FFDDD500269867 /* RefCounted.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = RefCounted.cpp; sourceTree = "<group>"; };
50DE35F3215BB01500B979C7 /* ExternalStringImpl.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ExternalStringImpl.cpp; sourceTree = "<group>"; };
50DE35F4215BB01500B979C7 /* ExternalStringImpl.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ExternalStringImpl.h; sourceTree = "<group>"; };
513E170A1CD7D5BF00E3650B /* LoggingAccumulator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LoggingAccumulator.h; sourceTree = "<group>"; };
@@ -1107,6 +1109,7 @@
0FDE87F61DFD07CC0064C390 /* RecursiveLockAdapter.h */,
A8A472FE151A825B004123FF /* RedBlackTree.h */,
26299B6D17A9E5B800ADEBE5 /* Ref.h */,
+ 46BEB6E922FFDDD500269867 /* RefCounted.cpp */,
A8A472FF151A825B004123FF /* RefCounted.h */,
A8A47300151A825B004123FF /* RefCountedArray.h */,
A8A47301151A825B004123FF /* RefCountedLeakCounter.cpp */,
@@ -1600,6 +1603,7 @@
A3B725EC987446AD93F1A440 /* RandomDevice.cpp in Sources */,
A8A47414151A825B004123FF /* RandomNumber.cpp in Sources */,
0FEC3C5E1F368A9700F59B6C /* ReadWriteLock.cpp in Sources */,
+ 46BEB6EB22FFE24900269867 /* RefCounted.cpp in Sources */,
A8A4741A151A825B004123FF /* RefCountedLeakCounter.cpp in Sources */,
E392FA2722E92BFF00ECDC73 /* ResourceUsageCocoa.cpp in Sources */,
2CDED0F318115C85004DBA70 /* RunLoop.cpp in Sources */,
Modified: trunk/Source/WTF/wtf/CMakeLists.txt (248532 => 248533)
--- trunk/Source/WTF/wtf/CMakeLists.txt 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WTF/wtf/CMakeLists.txt 2019-08-12 18:10:26 UTC (rev 248533)
@@ -393,6 +393,7 @@
RandomDevice.cpp
RandomNumber.cpp
ReadWriteLock.cpp
+ RefCounted.cpp
RefCountedLeakCounter.cpp
RunLoop.cpp
SHA1.cpp
Added: trunk/Source/WTF/wtf/RefCounted.cpp (0 => 248533)
--- trunk/Source/WTF/wtf/RefCounted.cpp (rev 0)
+++ trunk/Source/WTF/wtf/RefCounted.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB. If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include "config.h"
+#include <wtf/RefCounted.h>
+
+namespace WTF {
+
+#if !ASSERT_DISABLED
+bool RefCountedBase::areThreadingChecksEnabledGlobally { false };
+#endif
+
+}
Modified: trunk/Source/WTF/wtf/RefCounted.h (248532 => 248533)
--- trunk/Source/WTF/wtf/RefCounted.h 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WTF/wtf/RefCounted.h 2019-08-12 18:10:26 UTC (rev 248533)
@@ -22,6 +22,7 @@
#include <wtf/Assertions.h>
#include <wtf/FastMalloc.h>
+#include <wtf/MainThread.h>
#include <wtf/Noncopyable.h>
namespace WTF {
@@ -39,6 +40,16 @@
public:
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
+
#if CHECK_REF_COUNTED_LIFECYCLE
ASSERT_WITH_SECURITY_IMPLICATION(!m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
@@ -68,9 +79,28 @@
#endif
}
+ // Please only call this method if you really know that what you're doing is safe (e.g.
+ // locking at call sites).
+ void disableThreadingChecks()
+ {
+#if !ASSERT_DISABLED
+ m_areThreadingChecksEnabled = false;
+#endif
+ }
+
+ static void enableThreadingChecksGlobally()
+ {
+#if !ASSERT_DISABLED
+ areThreadingChecksEnabledGlobally = true;
+#endif
+ }
+
protected:
RefCountedBase()
: m_refCount(1)
+#if !ASSERT_DISABLED
+ , m_isOwnedByMainThread(isMainThread())
+#endif
#if CHECK_REF_COUNTED_LIFECYCLE
, m_deletionHasBegun(false)
, m_adoptionIsRequired(true)
@@ -78,6 +108,13 @@
{
}
+#if !ASSERT_DISABLED
+ bool areThreadingCheckedEnabled() const
+ {
+ return areThreadingChecksEnabledGlobally && m_areThreadingChecksEnabled;
+ }
+#endif
+
~RefCountedBase()
{
#if CHECK_REF_COUNTED_LIFECYCLE
@@ -89,6 +126,16 @@
// Returns whether the pointer should be freed or not.
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
+
#if CHECK_REF_COUNTED_LIFECYCLE
ASSERT_WITH_SECURITY_IMPLICATION(!m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
@@ -120,6 +167,11 @@
#endif
mutable unsigned m_refCount;
+#if !ASSERT_DISABLED
+ mutable bool m_isOwnedByMainThread;
+ bool m_areThreadingChecksEnabled { true };
+ WTF_EXPORT_PRIVATE static bool areThreadingChecksEnabledGlobally;
+#endif
#if CHECK_REF_COUNTED_LIFECYCLE
mutable bool m_deletionHasBegun;
mutable bool m_adoptionIsRequired;
Modified: trunk/Source/WTF/wtf/SizeLimits.cpp (248532 => 248533)
--- trunk/Source/WTF/wtf/SizeLimits.cpp 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WTF/wtf/SizeLimits.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -45,6 +45,8 @@
int a;
bool b;
bool c;
+ bool d;
+ bool e;
// The debug version may get bigger.
};
#else
Modified: trunk/Source/WebKit/ChangeLog (248532 => 248533)
--- trunk/Source/WebKit/ChangeLog 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WebKit/ChangeLog 2019-08-12 18:10:26 UTC (rev 248533)
@@ -1,5 +1,22 @@
2019-08-12 Chris Dumez <[email protected]>
+ Add threading assertions to RefCounted
+ https://bugs.webkit.org/show_bug.cgi?id=200507
+
+ Reviewed by Ryosuke Niwa.
+
+ Enable new RefCounted threading assertions for WebKit2
+ (UIProcess + auxiliary processes).
+
+ * Shared/AuxiliaryProcess.cpp:
+ (WebKit::AuxiliaryProcess::initialize):
+ * Shared/Cocoa/WebKit2InitializeCocoa.mm:
+ (WebKit::runInitializationCode):
+ * Shared/WebKit2Initialize.cpp:
+ (WebKit::InitializeWebKit2):
+
+2019-08-12 Chris Dumez <[email protected]>
+
Unreviewed, rolling out r248525.
Revert new threading assertions while I work on fixing the
Modified: trunk/Source/WebKit/Shared/AuxiliaryProcess.cpp (248532 => 248533)
--- trunk/Source/WebKit/Shared/AuxiliaryProcess.cpp 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WebKit/Shared/AuxiliaryProcess.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -60,6 +60,8 @@
void AuxiliaryProcess::initialize(const AuxiliaryProcessInitializationParameters& parameters)
{
+ WTF::RefCountedBase::enableThreadingChecksGlobally();
+
RELEASE_ASSERT_WITH_MESSAGE(parameters.processIdentifier, "Unable to initialize child process without a WebCore process identifier");
Process::setIdentifier(*parameters.processIdentifier);
Modified: trunk/Source/WebKit/Shared/Cocoa/WebKit2InitializeCocoa.mm (248532 => 248533)
--- trunk/Source/WebKit/Shared/Cocoa/WebKit2InitializeCocoa.mm 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WebKit/Shared/Cocoa/WebKit2InitializeCocoa.mm 2019-08-12 18:10:26 UTC (rev 248533)
@@ -31,6 +31,7 @@
#import <WebCore/LogInitialization.h>
#import <mutex>
#import <wtf/MainThread.h>
+#import <wtf/RefCounted.h>
#import <wtf/RunLoop.h>
#if PLATFORM(IOS_FAMILY)
@@ -50,6 +51,8 @@
JSC::initializeThreading();
RunLoop::initializeMainRunLoop();
+ WTF::RefCountedBase::enableThreadingChecksGlobally();
+
#if !LOG_DISABLED || !RELEASE_LOG_DISABLED
WebCore::initializeLogChannelsIfNecessary();
WebKit::initializeLogChannelsIfNecessary();
Modified: trunk/Source/WebKit/Shared/WebKit2Initialize.cpp (248532 => 248533)
--- trunk/Source/WebKit/Shared/WebKit2Initialize.cpp 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WebKit/Shared/WebKit2Initialize.cpp 2019-08-12 18:10:26 UTC (rev 248533)
@@ -30,6 +30,7 @@
#include <_javascript_Core/InitializeThreading.h>
#include <WebCore/LogInitialization.h>
#include <wtf/MainThread.h>
+#include <wtf/RefCounted.h>
#include <wtf/RunLoop.h>
namespace WebKit {
@@ -41,6 +42,8 @@
JSC::initializeThreading();
RunLoop::initializeMainRunLoop();
+ WTF::RefCountedBase::enableThreadingChecksGlobally();
+
#if !LOG_DISABLED || !RELEASE_LOG_DISABLED
WebCore::initializeLogChannelsIfNecessary();
WebKit::initializeLogChannelsIfNecessary();
Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (248532 => 248533)
--- trunk/Source/WebKitLegacy/mac/ChangeLog 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog 2019-08-12 18:10:26 UTC (rev 248533)
@@ -1,5 +1,16 @@
2019-08-12 Chris Dumez <[email protected]>
+ Add threading assertions to RefCounted
+ https://bugs.webkit.org/show_bug.cgi?id=200507
+
+ Reviewed by Ryosuke Niwa.
+
+ * WebView/WebView.mm:
+ (+[WebView initialize]):
+ Enable new RefCounted threading assertions for WebKitLegacy.
+
+2019-08-12 Chris Dumez <[email protected]>
+
Unreviewed, rolling out r248525.
Revert new threading assertions while I work on fixing the
Modified: trunk/Source/WebKitLegacy/mac/WebView/WebView.mm (248532 => 248533)
--- trunk/Source/WebKitLegacy/mac/WebView/WebView.mm 2019-08-12 17:27:51 UTC (rev 248532)
+++ trunk/Source/WebKitLegacy/mac/WebView/WebView.mm 2019-08-12 18:10:26 UTC (rev 248533)
@@ -5416,6 +5416,8 @@
RunLoop::initializeMainRunLoop();
#endif
+ WTF::RefCountedBase::enableThreadingChecksGlobally();
+
WTF::setProcessPrivileges(allPrivileges());
WebCore::NetworkStorageSession::permitProcessToUseCookieAPI(true);
