Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 17eb50c5ed5d5466c4a16ef7160531ee6c2f2f98
https://github.com/WebKit/WebKit/commit/17eb50c5ed5d5466c4a16ef7160531ee6c2f2f98
Author: Chris Dumez <[email protected]>
Date: 2026-09-10 (Thu, 10 Sep 2026)
Changed paths:
M Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.h
M Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm
Log Message:
-----------
ImageDecoderAVFObjC mutates its sample map and sample images without the lock
the decoding work queue reads them under
https://bugs.webkit.org/show_bug.cgi?id=323739
Reviewed by Jean-Yves Avenard.
createFrameImageAtIndex() runs on AsyncImageDecoder's org.webkit.ImageDecoder
work queue and
takes m_sampleGeneratorLock to read m_sampleData, to read and advance m_cursor,
and to read and
replace the samples' decoded CGImages. readTrackMetadata() and setTrack() take
the same lock to
mutate that state from the main thread. Two other main-thread mutators did not.
readSamples() called m_sampleData.addSample() unlocked, while the work queue
searches and
iterates the same SampleMap, which is backed by StdMap; inserting into a
red-black tree while
another thread walks it can follow pointers through a rebalance. This was
already
self-inconsistent, since setTrack() clears the very same container under the
lock twenty lines
earlier.
clearFrameBufferCache() called setImage(nullptr) on each sample unlocked. Since
ImageDecoderAVFObjCSample::image() hands out a raw CGImageRef,
createFrameImageAtIndex()'s
"RetainPtr image = sampleData->image()" loads the pointer and retains it in two
steps, and this
released it in between, so the work queue could retain and return freed memory.
That window is
reachable rather than theoretical: BitmapImageSource::destroyDecodedData()
routes to
clearFrameBufferCache() precisely in the branch where the work queue is not
known to be idle.
Take m_sampleGeneratorLock in both. readSamples() collects the samples into a
Vector first and
inserts them under the lock, so that reading from the AVAssetReader does not
block the work
queue, and still fires the encoded-data-status callback outside the lock, since
that callback
re-enters BitmapImageSource.
Then annotate the state so this cannot regress. m_sampleData, m_cursor and
m_imageRotationSession are now WTF_GUARDED_BY_LOCK(m_sampleGeneratorLock), and
the main thread's
unlocked reads use the assertIsOwnerThread() helpers added in 320637@main.
storeSampleBuffer()
and advanceCursor() are only ever called from inside
createFrameImageAtIndex()'s critical
section, so they declare WTF_REQUIRES_LOCK rather than locking again.
sampleAtIndex() is reached
both from the work queue holding the lock and from the frame-metadata accessors
on the main
thread without it, so it requires the lock shared, which both callers satisfy.
Both unlocked
writes fixed here are writes to guarded state while holding only shared access,
which is exactly
what this reports.
m_size is deliberately left unguarded: it is only ever read on the main thread.
createFrameImageAtIndex() does not touch it, and the other reader, size() by
way of
frameSizeAtIndex(), is reached from
BitmapImageSource::fetchFrameMetaDataAtIndex() on the main
thread. The comment in readTrackMetadata() claiming otherwise is corrected; only
m_imageRotationSession is read off the main thread there, by
storeSampleBuffer().
Note that in the default Cocoa configuration the decoder runs in the GPU
process, driven purely
by IPC on one thread, with no AsyncImageDecoder and so no work queue. The
racing arrangement is
WebContent with UseGPUProcessForMediaEnabled off, where the factory constructs
ImageDecoderAVFObjC directly and AsyncImageDecoder decodes on its work queue.
* Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.h:
* Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:
(WebCore::ImageDecoderAVFObjC::readSamples):
(WebCore::ImageDecoderAVFObjC::readTrackMetadata):
(WebCore::ImageDecoderAVFObjC::encodedDataStatus const):
(WebCore::ImageDecoderAVFObjC::frameCount const):
(WebCore::ImageDecoderAVFObjC::frameIsCompleteAtIndex const):
(WebCore::ImageDecoderAVFObjC::frameDurationAtIndex const):
(WebCore::ImageDecoderAVFObjC::frameHasAlphaAtIndex const):
(WebCore::ImageDecoderAVFObjC::frameInfos const):
(WebCore::ImageDecoderAVFObjC::clearFrameBufferCache):
Canonical link: https://commits.webkit.org/320808@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
