Skip to content

Commit

Permalink
Cherry-pick b0a755e. rdar://118548733
Browse files Browse the repository at this point in the history
    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/267815.567@safari-7617.1.17.13-branch
  • Loading branch information
rjepstein committed Nov 18, 2023
1 parent 31b27a8 commit 6a43326
Showing 1 changed file with 21 additions and 7 deletions.
28 changes: 21 additions & 7 deletions Source/JavaScriptCore/runtime/Structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 6a43326

Please sign in to comment.