[webkit-changes] [WebKit/WebKit] 779576: REGRESSION(265596 at main) VM::performOpportunistical...

Commit Queue noreply at github.com
Fri Jul 7 17:51:49 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 7795764072454f1c3da0ad56a5144f4afbffbed7
      https://github.com/WebKit/WebKit/commit/7795764072454f1c3da0ad56a5144f4afbffbed7
  Author: Mark Lam <mark.lam at apple.com>
  Date:   2023-07-07 (Fri, 07 Jul 2023)

  Changed paths:
    M Source/JavaScriptCore/runtime/JSLock.cpp
    M Source/JavaScriptCore/runtime/VM.cpp

  Log Message:
  -----------
  REGRESSION(265596 at main) VM::performOpportunisticallyScheduledTasks() should hold the JSLock when calling the sweeper.
https://bugs.webkit.org/show_bug.cgi?id=259007
rdar://111505837

Reviewed by Justin Michaud.

This makes it consistent with the incremental sweeper that currently fires off a timer, which also
hold the JSLock while sweeping.

Not holding the JSLock while sweeping has resulted in the RELEASE_ASSERT in the JSLock::DropAllLocks
constructor failing.  Here's how it happens:

1. For a normal timer triggered sweep, we go through JSRunLoopTimer::timerDidFire() which acquires
   the JSLock, and holds it while sweeping.
2. For an idle triggered VM::performOpportunisticallyScheduledTasks(), it does NOT hold the JSLock
   while sweeping.
3. While sweeping, we may call back into some ObjC code that wraps a JSValue.
4. Releasing that JSValue for the sweep, requires acquiring the JSLock.
5. On releasing that JSLock, if it’s the outermost lock (i.e. not a re-entrant lock), then the
   JSLock unlocking code will call Heap::releaseDelayedReleasedObjects().
6. Heap::releaseDelayedReleasedObjects() uses DropAllLocks, which fails the RELEASE_ASSERT because
   we’re currently holding the JSLock and doing sweeping.
7. In contrast, for the normal timer triggered sweep, the outer most lock of the JSLock is
   JSRunLoopTimer::timerDidFire().  Hence, Heap::releaseDelayedReleasedObjects() won’t be called,
   and we won’t encounter this issue.

The fix is simply to acquire and hold the JSLock while sweeping in VM::performOpportunisticallyScheduledTasks().

* Source/JavaScriptCore/runtime/JSLock.cpp:
(JSC::JSLock::DropAllLocks::DropAllLocks):
* Source/JavaScriptCore/runtime/VM.cpp:
(JSC::VM::performOpportunisticallyScheduledTasks):

Canonical link: https://commits.webkit.org/265871@main




More information about the webkit-changes mailing list