parent
8f16c9de46
commit
52fba13746
@ -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