Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: a90ff51d22256639ba1250a1d589f75a73c5ca9f
https://github.com/WebKit/WebKit/commit/a90ff51d22256639ba1250a1d589f75a73c5ca9f
Author: Alex Christensen <[email protected]>
Date: 2026-08-26 (Wed, 26 Aug 2026)
Changed paths:
M Source/WTF/wtf/Forward.h
M Source/WTF/wtf/Ref.h
M Source/WTF/wtf/RefPtr.h
M Source/WTF/wtf/ThreadSafeWeakPtr.h
M Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp
M Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h
M Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.cpp
M Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.h
M
Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.messages.in
M Source/WebKit/WebProcess/GPU/media/AudioVideoRendererRemote.cpp
M Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp
Log Message:
-----------
Make ThreadSafeWeakPtr safe to mutate from different threads
https://bugs.webkit.org/show_bug.cgi?id=283929
rdar://141168403
Reviewed by David Kilzer.
ThreadSafeWeakPtr needs more information than just a pointer to support
multiple inheritance.
This is tested in the existing test WTF_ThreadSafeWeakPtr.MultipleInheritance.
This was implemented by keeping two pointers, but that makes it so that if
multiple threads
are accessing the same ThreadSafeWeakPtr from multiple threads and mutating it,
there can
be inconsistent state, which leads to crashes. See rdar://184638719.
ThreadSafeWeakPtr
was as safe as std::shared_ptr<T>, but this makes it as safe as
std::atomic<std::shared_ptr<T>>.
Instead of keeping two pointers, I keep a pointer and a 16-bit offset to make
room for a Lock.
RemoteAudioVideoRendererProxyManager needed to be fixed to fit into this new
model. It was
returning the control block of the GPUConnectionToWebProcess, which resulted in
arbitrarily
large offsets needing to be stored because it was the difference of two
pointers to two
different objects not related by inheritance. I made
RemoteAudioVideoRendererProxyManager
have its own control block to fix this.
I added some AI-generated tests that showed some threading issues before this
fix.
I measured the performance of this change with this simple benchmark:
Ref counter = adoptRef(*new ThreadSafeInstanceCounter());
WallTime begin = WallTime::now();
for (size_t i = 0; i < 1000000; i++) {
ThreadSafeWeakPtr weakPtr { counter.get() };
RefPtr getResult = weakPtr.get();
}
WallTime end = WallTime::now();
WTFLogAlways("%f ms", (end - begin).milliseconds());
The time spent before this change was about 8ms:
8.530140 ms
8.382082 ms
8.578062 ms
8.245945 ms
8.296013 ms
The time spent after this change was about 19ms:
19.480944 ms
19.495010 ms
19.415855 ms
19.705057 ms
19.248009 ms
So this does make this lightweight operation about twice as expensive, but
that's the
cost of thread safety. If this is found to be a performance bottleneck for
anything,
we could either use WeakPtr or make a non-mutable version of ThreadSafeWeakPtr
that
doesn't have a lock, or we could rewrite the code to not be so dependent on
ThreadSafeWeakPtr performance.
Test: Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp
* Source/WTF/wtf/Forward.h:
* Source/WTF/wtf/Ref.h:
* Source/WTF/wtf/RefPtr.h:
* Source/WTF/wtf/ThreadSafeWeakPtr.h:
(WTF::ThreadSafeWeakPtrControlBlock::makeStrongReferenceIfPossible const):
(WTF::ThreadSafeWeakPtrControlBlock::objectOffset const):
(WTF::ThreadSafeWeakPtrControlBlock::refObjectIfAlive const):
(WTF::ThreadSafeWeakPtrStorage::ThreadSafeWeakPtrStorage):
(WTF::ThreadSafeWeakPtrStorage::~ThreadSafeWeakPtrStorage):
(WTF::ThreadSafeWeakPtrStorage::makeStrongReferenceIfPossible const):
(WTF::ThreadSafeWeakPtrStorage::set):
(WTF::ThreadSafeWeakPtrStorage::clear):
(WTF::ThreadSafeWeakPtrStorage::copyFrom):
(WTF::ThreadSafeWeakPtrStorage::moveFrom):
(WTF::ThreadSafeWeakPtrStorage::adopt):
(WTF::ThreadSafeWeakPtrStorage::WTF_GUARDED_BY_LOCK):
(WTF::ThreadSafeWeakPtr::ThreadSafeWeakPtr):
(WTF::ThreadSafeWeakPtr::operator=):
(WTF::ThreadSafeWeakPtr::get const):
(WTF::ThreadSafeWeakPtr::controlBlockAndObjectOffset):
(WTF::ThreadSafeWeakRef::ThreadSafeWeakRef):
(WTF::ThreadSafeWeakRef::operator=):
(WTF::ThreadSafeWeakRef::get const):
(WTF::ThreadSafeWeakRef::controlBlockAndObjectOffset):
(WTF::protect):
* Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp:
(WebKit::GPUConnectionToWebProcess::remoteAudioVideoRendererProxyManager):
* Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h:
* Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.cpp:
(WebKit::RemoteAudioVideoRendererProxyManager::create):
(WebKit::RemoteAudioVideoRendererProxyManager::createManager):
(WebKit::RemoteAudioVideoRendererProxyManager::ref const): Deleted.
(WebKit::RemoteAudioVideoRendererProxyManager::deref const): Deleted.
(WebKit::RemoteAudioVideoRendererProxyManager::controlBlock const): Deleted.
* Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.h:
*
Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.messages.in:
* Source/WebKit/WebProcess/GPU/media/AudioVideoRendererRemote.cpp:
(WebKit::AudioVideoRendererRemote::AudioVideoRendererRemote):
* Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp:
(TestWebKitAPI::performWithOptionalLock):
(TestWebKitAPI::testTornPairOnAssignment):
(TestWebKitAPI::TEST(WTF_ThreadSafeWeakPtr, TornPairOnAssignment)):
(TestWebKitAPI::testControlBlockFreedUnderReader):
(TestWebKitAPI::TEST(WTF_ThreadSafeWeakPtr, ControlBlockFreedUnderReader)):
(TestWebKitAPI::TEST(WTF_ThreadSafeWeakPtr,
GetRacingWithLastWeakReferenceRelease)):
(TestWebKitAPI::testDoubleWeakDeref):
(TestWebKitAPI::TEST(WTF_ThreadSafeWeakPtr, DoubleWeakDeref)):
(TestWebKitAPI::testWeakHashSetTimeOfCheckTimeOfUse):
(TestWebKitAPI::TEST(WTF_ThreadSafeWeakPtr, WeakHashSetTimeOfCheckTimeOfUse)):
Canonical link: https://commits.webkit.org/319915@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
