[webkit-changes] [WebKit/WebKit] 14530d: JSObject::getDirectConcurrently should take the ce...

[Justin Michaud]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 14530da9c09d490a1759b96da2231698f1aae0af
      https://github.com/WebKit/WebKit/commit/14530da9c09d490a1759b96da2231698f1aae0af
  Author: Justin Michaud <justin_michaud at apple.com>
  Date:   2023-07-31 (Mon, 31 Jul 2023)

  Changed paths:
    A JSTests/stress/get-concurrently-should-take-cell-lock.js
    M Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp
    M Source/JavaScriptCore/bytecode/PropertyCondition.cpp
    M Source/JavaScriptCore/dfg/DFGGraph.cpp
    M Source/JavaScriptCore/runtime/JSArray.cpp
    M Source/JavaScriptCore/runtime/JSCell.h
    M Source/JavaScriptCore/runtime/JSObject.h

  Log Message:
  -----------
  JSObject::getDirectConcurrently should take the cell lock.
https://bugs.webkit.org/show_bug.cgi?id=257285
rdar://108166258

Reviewed by Yusuke Suzuki.

`JSArray::unshiftCountWithArrayStorage` takes the cell lock and then the
structure lock to prevent the compiler thread from accessing the butterfly
before it is fully initialized.

`JSObject::getDirectConcurrently` only takes the structure lock. This means
that the compiler can take the structure lock, the cell can transition
to a new structure, then unshift can mess up the butterfly, and finally
the compiler thread proceeds to see garbage.

The attached POC only reproduces if waits are introduced to extend the race window.

It seems that the comment above cellLock is outdated, as our current concurrency
protocol to prevent deadlocks is to take the cell lock then the structure lock.
I could not find anywhere that uses the reverse, but if I missed something,
a deadlock will be pretty easy to debug.

* Source/JavaScriptCore/runtime/JSArray.cpp:
(JSC::JSArray::unshiftCountWithArrayStorage):
* Source/JavaScriptCore/runtime/JSCell.h:
(JSC::JSCell::cellLock const):
(JSC::JSCell::cellLock): Deleted.
* Source/JavaScriptCore/runtime/JSObject.h:
(JSC::JSObject::getDirectConcurrently const):

Originally-landed-as: 259548.798 at safari-7615-branch (b7e3ebd9c372). rdar://108166258
Canonical link: https://commits.webkit.org/266435@main