parent
fffcc0c1fd
commit
d345afdca2
@ -0,0 +1,80 @@
|
|||||||
|
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);
|
Loading…
Reference in new issue