Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 6f1389789211dc9bd677c8ef2fe152f12584b335
https://github.com/WebKit/WebKit/commit/6f1389789211dc9bd677c8ef2fe152f12584b335
Author: Chris Dumez <[email protected]>
Date: 2026-09-08 (Tue, 08 Sep 2026)
Changed paths:
M Source/WTF/wtf/ThreadAssertions.h
M Source/WebCore/dom/Range.cpp
M Source/WebCore/dom/Range.h
Log Message:
-----------
Annotate Range's boundary points as owner-thread state
https://bugs.webkit.org/show_bug.cgi?id=323647
Reviewed by Geoffrey Garen.
Range::m_start and m_end are mutated on the main thread under
m_boundaryPointLock and read
on the GC threads by visitNodesInGCThread(), which locks. That reader is
reached through
JSRange::visitAdditionalChildrenInGCThread(), so unlike
isReachableFromOpaqueRoots() and
hasPendingActivity() it runs concurrently with the main thread rather than with
the world
paused, and the lock it takes is load-bearing. The eleven mutation sites
already took the
lock; nothing enforced that they did.
Guard both members with WTF_GUARDED_BY_LOCK() and assert on the main thread's
unlocked read
paths. Most of those reads are not in Range.cpp but in the five inline accessors
startContainer(), startOffset(), endContainer(), endOffset() and collapsed(),
which is
where the bulk of WebCore reaches the boundary points; annotating them is what
makes the
guard hold for their callers. setStart() and setEnd() each read the opposite
boundary point
to decide whether the range collapses before entering their critical section,
so they are
the assert-then-lock case and call releaseOwnerThreadAssertion() before the
Locker; they
are its first users. compareBoundaryPoints() reads a second Range's boundary
points, and
shared access to one instance's lock grants nothing for another's, so it
asserts on
sourceRange as well.
Also add WTF_DECLARE_OWNER_THREAD_ASSERTIONS(), which declares
assertIsOwnerThread() and
releaseOwnerThreadAssertion() members for a class, so call sites name neither
the lock nor
the owner thread and the choice of owner is stated once. Being members is what
lets
compareBoundaryPoints() write sourceRange.assertIsOwnerThread() to grant access
to that
object's lock. An RAII form was considered instead of the explicit release. It
is worse
here: it does nothing for the common case of a function that only reads, and for
setStart()/setEnd() it forces the result variable out of its initializer and
into an extra
scope, since leaving the assertion live to the end of the enclosing scope still
reports the
Locker as acquiring a lock that is already held. Omitting the release is a
build failure,
not a silent bug, so there is nothing for RAII to make safe.
No behaviour change is intended.
* Source/WTF/wtf/ThreadAssertions.h:
* Source/WebCore/dom/Range.cpp:
(WebCore::Range::setStart):
(WebCore::Range::setEnd):
(WebCore::Range::compareNode const):
(WebCore::Range::compareBoundaryPoints const):
(WebCore::Range::processContents):
(WebCore::Range::cloneRange const):
(WebCore::Range::debugDescription const):
(WebCore::Range::parentlessNodeMovedToNewDocumentAffectsRange):
* Source/WebCore/dom/Range.h:
Canonical link: https://commits.webkit.org/320703@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
