From 00352dd86bfa102b6e4b792120e3ef3498a27d1e Mon Sep 17 00:00:00 2001 From: Russell Epstein 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 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);