You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
80 lines
3.7 KiB
80 lines
3.7 KiB
From 00352dd86bfa102b6e4b792120e3ef3498a27d1e Mon Sep 17 00:00:00 2001 |
|
From: Russell Epstein <repstein@apple.com> |
|
Date: Fri, 17 Nov 2023 15:48:32 -0800 |
|
Subject: [PATCH] Cherry-pick b0a755e34426. |
|
https://bugs.webkit.org/show_bug.cgi?id=265067 |
|
|
|
Race condition between JSObject::getDirectConcurrently users and Structure::flattenDictionaryStructure |
|
https://bugs.webkit.org/show_bug.cgi?id=265067 |
|
rdar://118548733 |
|
|
|
Reviewed by Justin Michaud and Mark Lam. |
|
|
|
Like Array shift/unshift, flattenDictionaryStructure is the other code which can shrink butterfly for named properties (no other code does it). |
|
Compiler threads rely on the fact that normally named property storage never shrunk. And we should catch this exceptional case by taking a cellLock |
|
in the compiler thread. But flattenDictionaryStructure is not taking cellLock correctly. |
|
|
|
This patch computes afterOutOfLineCapacity first to detect that whether this flattening will shrink the butterfly. |
|
And if it is, then we take a cellLock. We do not need to take it if we do not shrink the butterfly. |
|
|
|
* Source/JavaScriptCore/runtime/Structure.cpp: |
|
(JSC::Structure::flattenDictionaryStructure): |
|
|
|
Canonical link: https://commits.webkit.org/267815.577@safari-7617-branch |
|
|
|
Canonical link: https://commits.webkit.org/265870.632@safari-7616.2.9.10-branch |
|
--- |
|
Source/JavaScriptCore/runtime/Structure.cpp | 28 +++++++++++++++------ |
|
1 file changed, 21 insertions(+), 7 deletions(-) |
|
|
|
diff --git a/Source/JavaScriptCore/runtime/Structure.cpp b/Source/JavaScriptCore/runtime/Structure.cpp |
|
index 2922e2478794c..9d094e2c8adc8 100644 |
|
--- a/Source/JavaScriptCore/runtime/Structure.cpp |
|
+++ b/Source/JavaScriptCore/runtime/Structure.cpp |
|
@@ -913,17 +913,31 @@ Structure* Structure::flattenDictionaryStructure(VM& vm, JSObject* object) |
|
checkOffsetConsistency(); |
|
ASSERT(isDictionary()); |
|
ASSERT(object->structure() == this); |
|
- |
|
- GCSafeConcurrentJSLocker locker(m_lock, vm); |
|
- |
|
- object->setStructureIDDirectly(id().nuke()); |
|
- WTF::storeStoreFence(); |
|
|
|
+ Locker<JSCellLock> cellLocker(NoLockingNecessary); |
|
+ |
|
+ PropertyTable* table = nullptr; |
|
size_t beforeOutOfLineCapacity = this->outOfLineCapacity(); |
|
+ size_t afterOutOfLineCapacity = beforeOutOfLineCapacity; |
|
if (isUncacheableDictionary()) { |
|
- PropertyTable* table = propertyTableOrNull(); |
|
+ table = propertyTableOrNull(); |
|
ASSERT(table); |
|
+ PropertyOffset maxOffset = invalidOffset; |
|
+ if (unsigned propertyCount = table->size()) |
|
+ maxOffset = offsetForPropertyNumber(propertyCount - 1, m_inlineCapacity); |
|
+ afterOutOfLineCapacity = outOfLineCapacity(maxOffset); |
|
+ } |
|
|
|
+ // This is the only case we shrink butterfly in this function. We should take a cell lock to protect against concurrent access to the butterfly. |
|
+ if (beforeOutOfLineCapacity != afterOutOfLineCapacity) |
|
+ cellLocker = Locker { object->cellLock() }; |
|
+ |
|
+ GCSafeConcurrentJSLocker locker(m_lock, vm); |
|
+ |
|
+ object->setStructureIDDirectly(id().nuke()); |
|
+ WTF::storeStoreFence(); |
|
+ |
|
+ if (isUncacheableDictionary()) { |
|
size_t propertyCount = table->size(); |
|
|
|
// Holds our values compacted by insertion order. This is OK since GC is deferred. |
|
@@ -955,7 +969,7 @@ Structure* Structure::flattenDictionaryStructure(VM& vm, JSObject* object) |
|
setDictionaryKind(NoneDictionaryKind); |
|
setHasBeenFlattenedBefore(true); |
|
|
|
- size_t afterOutOfLineCapacity = this->outOfLineCapacity(); |
|
+ ASSERT(this->outOfLineCapacity() == afterOutOfLineCapacity); |
|
|
|
if (object->butterfly() && beforeOutOfLineCapacity != afterOutOfLineCapacity) { |
|
ASSERT(beforeOutOfLineCapacity > afterOutOfLineCapacity);
|
|
|