Title: [269221] trunk/Source/WebCore
- Revision
- 269221
- Author
- [email protected]
- Date
- 2020-10-30 23:52:36 -0700 (Fri, 30 Oct 2020)
Log Message
Usage of allWorkletGlobalScopesSet() in WorkletGlobalScope is not thread-safe
https://bugs.webkit.org/show_bug.cgi?id=218408
Reviewed by Geoff Garen.
The WorkletGlobalScope constructor / destructor were adding / removing themselves
in allWorkletGlobalScopesSet(), without locking. This used to be safe when we
only had paint worklets, because those are main-thread only. However,
AudioWorketGlobalScopes get constructed / destroyed on background thread so this
is no longer safe.
Since this is only used by Internals for layout tests to figure out how many
worklet instances are alive, I replaced the map with an std::atomic<unsigned>.
No new tests, found when running existing tests with GuardMalloc.
* testing/Internals.cpp:
(WebCore::Internals::isAnyWorkletGlobalScopeAlive const):
* worklets/WorkletGlobalScope.cpp:
(WebCore::WorkletGlobalScope::WorkletGlobalScope):
(WebCore::WorkletGlobalScope::~WorkletGlobalScope):
(WebCore::WorkletGlobalScope::numberOfWorkletGlobalScopes):
* worklets/WorkletGlobalScope.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (269220 => 269221)
--- trunk/Source/WebCore/ChangeLog 2020-10-31 03:34:22 UTC (rev 269220)
+++ trunk/Source/WebCore/ChangeLog 2020-10-31 06:52:36 UTC (rev 269221)
@@ -1,3 +1,29 @@
+2020-10-30 Chris Dumez <[email protected]>
+
+ Usage of allWorkletGlobalScopesSet() in WorkletGlobalScope is not thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=218408
+
+ Reviewed by Geoff Garen.
+
+ The WorkletGlobalScope constructor / destructor were adding / removing themselves
+ in allWorkletGlobalScopesSet(), without locking. This used to be safe when we
+ only had paint worklets, because those are main-thread only. However,
+ AudioWorketGlobalScopes get constructed / destroyed on background thread so this
+ is no longer safe.
+
+ Since this is only used by Internals for layout tests to figure out how many
+ worklet instances are alive, I replaced the map with an std::atomic<unsigned>.
+
+ No new tests, found when running existing tests with GuardMalloc.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::isAnyWorkletGlobalScopeAlive const):
+ * worklets/WorkletGlobalScope.cpp:
+ (WebCore::WorkletGlobalScope::WorkletGlobalScope):
+ (WebCore::WorkletGlobalScope::~WorkletGlobalScope):
+ (WebCore::WorkletGlobalScope::numberOfWorkletGlobalScopes):
+ * worklets/WorkletGlobalScope.h:
+
2020-10-29 Myles C. Maxfield <[email protected]>
[Cocoa] [GPU Process] Perform image decoding of color fonts in the Web Process
Modified: trunk/Source/WebCore/testing/Internals.cpp (269220 => 269221)
--- trunk/Source/WebCore/testing/Internals.cpp 2020-10-31 03:34:22 UTC (rev 269220)
+++ trunk/Source/WebCore/testing/Internals.cpp 2020-10-31 06:52:36 UTC (rev 269221)
@@ -2679,11 +2679,7 @@
bool Internals::isAnyWorkletGlobalScopeAlive() const
{
-#if ENABLE(CSS_PAINTING_API)
- return !WorkletGlobalScope::allWorkletGlobalScopesSet().isEmpty();
-#else
- return false;
-#endif
+ return WorkletGlobalScope::numberOfWorkletGlobalScopes();
}
String Internals::serviceWorkerClientIdentifier(const Document& document) const
Modified: trunk/Source/WebCore/worklets/WorkletGlobalScope.cpp (269220 => 269221)
--- trunk/Source/WebCore/worklets/WorkletGlobalScope.cpp 2020-10-31 03:34:22 UTC (rev 269220)
+++ trunk/Source/WebCore/worklets/WorkletGlobalScope.cpp 2020-10-31 06:52:36 UTC (rev 269221)
@@ -47,6 +47,8 @@
WTF_MAKE_ISO_ALLOCATED_IMPL(WorkletGlobalScope);
+static std::atomic<unsigned> gNumberOfWorkletGlobalScopes { 0 };
+
WorkletGlobalScope::WorkletGlobalScope(WorkerOrWorkletThread& thread, const WorkletParameters& parameters)
: WorkerOrWorkletGlobalScope(JSC::VM::create(), &thread)
, m_topOrigin(SecurityOrigin::createUnique())
@@ -53,8 +55,7 @@
, m_url(parameters.windowURL)
, m_jsRuntimeFlags(parameters.jsRuntimeFlags)
{
- auto addResult = allWorkletGlobalScopesSet().add(this);
- ASSERT_UNUSED(addResult, addResult);
+ ++gNumberOfWorkletGlobalScopes;
setSecurityOriginPolicy(SecurityOriginPolicy::create(SecurityOrigin::create(this->url())));
setContentSecurityPolicy(makeUnique<ContentSecurityPolicy>(URL { this->url() }, *this));
@@ -68,8 +69,7 @@
, m_jsRuntimeFlags(document.settings()._javascript_RuntimeFlags())
, m_code(WTFMove(code))
{
- auto addResult = allWorkletGlobalScopesSet().add(this);
- ASSERT_UNUSED(addResult, addResult);
+ ++gNumberOfWorkletGlobalScopes;
ASSERT(document.page());
@@ -81,10 +81,15 @@
{
ASSERT(!script());
removeFromContextsMap();
- auto removeResult = allWorkletGlobalScopesSet().remove(this);
- ASSERT_UNUSED(removeResult, removeResult);
+ ASSERT(gNumberOfWorkletGlobalScopes);
+ --gNumberOfWorkletGlobalScopes;
}
+unsigned WorkletGlobalScope::numberOfWorkletGlobalScopes()
+{
+ return gNumberOfWorkletGlobalScopes;
+}
+
void WorkletGlobalScope::prepareForDestruction()
{
WorkerOrWorkletGlobalScope::prepareForDestruction();
@@ -95,12 +100,6 @@
}
}
-auto WorkletGlobalScope::allWorkletGlobalScopesSet() -> WorkletGlobalScopesSet&
-{
- static NeverDestroyed<WorkletGlobalScopesSet> scopes;
- return scopes;
-}
-
String WorkletGlobalScope::userAgent(const URL& url) const
{
if (!m_document)
Modified: trunk/Source/WebCore/worklets/WorkletGlobalScope.h (269220 => 269221)
--- trunk/Source/WebCore/worklets/WorkletGlobalScope.h 2020-10-31 03:34:22 UTC (rev 269220)
+++ trunk/Source/WebCore/worklets/WorkletGlobalScope.h 2020-10-31 06:52:36 UTC (rev 269221)
@@ -59,9 +59,6 @@
public:
virtual ~WorkletGlobalScope();
- using WorkletGlobalScopesSet = HashSet<const WorkletGlobalScope*>;
- WEBCORE_EXPORT static WorkletGlobalScopesSet& allWorkletGlobalScopesSet();
-
#if ENABLE(CSS_PAINTING_API)
virtual bool isPaintWorkletGlobalScope() const { return false; }
#endif
@@ -69,6 +66,8 @@
virtual bool isAudioWorkletGlobalScope() const { return false; }
#endif
+ WEBCORE_EXPORT static unsigned numberOfWorkletGlobalScopes();
+
MessagePortChannelProvider& messagePortChannelProvider();
const URL& url() const final { return m_url; }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
