[webkit-changes] [WebKit/WebKit] 2cf73e: ASSERTION FAILED: !retainedPointer || !m_controlBl...

[Chris Dumez]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 2cf73e50c05c8cfa3980090d0d635e8a67611d63
      https://github.com/WebKit/WebKit/commit/2cf73e50c05c8cfa3980090d0d635e8a67611d63
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-06-17 (Sat, 17 Jun 2023)

  Changed paths:
    M Source/WTF/wtf/ThreadSafeWeakHashSet.h
    M Source/WTF/wtf/ThreadSafeWeakPtr.h
    M Source/WebCore/PAL/pal/cf/CoreMediaSoftLink.cpp
    M Source/WebCore/PAL/pal/cf/CoreMediaSoftLink.h
    M Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h
    M Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm
    M Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm
    M Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.h
    M Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm

  Log Message:
  -----------
  ASSERTION FAILED: !retainedPointer || !m_controlBlock->objectHasBeenDeleted() in the GPUProcess
https://bugs.webkit.org/show_bug.cgi?id=254186
rdar://106963694

Reviewed by Alex Christensen.

The assertion was being hit on a background thread inside MediaRecorderPrivateWriter::compressedVideoOutputBufferCallback()
when constructing a ThreadSafeWeakPtr from MediaRecorderPrivateWriter pointer. You can see in the crash logs that the
MediaRecorderPrivateWriter destructor is currently running on the main thread at the time.

MediaRecorderPrivateWriter owns the VideoSampleBufferCompressor & AudioSampleBufferCompressor via m_videoCompressor &
m_audioCompressor unique pointers. When constructed, VideoSampleBufferCompressor & AudioSampleBufferCompressor
register triggers that will cause compressedVideoOutputBufferCallback() & compressedAudioOutputBufferCallback() to
get called off the main thread with a pointer to the MediaRecorderPrivateWriter object.

I updated the VideoSampleBufferCompressor & AudioSampleBufferCompressor destructors to unregister those triggers so
that we guarantee that the compressedVideoOutputBufferCallback() & compressedAudioOutputBufferCallback() functions
cannot get called once the VideoSampleBufferCompressor & AudioSampleBufferCompressor objects have been destroyed.
Given that these objects are owned by the MediaRecorderPrivateWriter via unique_ptr, it also guarantees that the
MediaRecorderPrivateWriter pointer passed to these callbacks will never be stale / dead.

However, even with this fix, we'd still hit the assertion in ThreadSafeWeakPtr, even though the code is now safe.
The reason is that `m_controlBlock->objectHasBeenDeleted()` return true as soon as the object has started deletion.
It doesn't indicate that the object has been deleted. I have renamed this function to objectHasStartedDeletion()
for clarity.

I am also removing these assertions from ThreadSafeWeakPtr because there are too restrictive and would prevent this
safe code from using ThreadSafeWeakPtr. At the time, compressedVideoOutputBufferCallback() constructs the
TheadSafeWeakPtr, the MediaRecorderPrivateWriter destructor may have started running but we know the objects is
still alive thanks to the trigger removal in the VideoSampleBufferCompressor destructor. The callback just needs a
ThreadSafeWeakPtr to check if the object is STILL alive once the dispatch to the main thread has occurred. This is
important since the MediaRecorderPrivateWriter gets destroyed on the main thread and since the object may have been
destroyed by the time the task runs on the main thread. ThreadSafeWeakPtr is able to deal with this just fine once
you remove the `objectHasStartedDeletion` assertions. It will successfully create the ThreadSafeWeakPtr when the
object has started destruction. If you later call `get()` on this ThreadSafeWeakPtr, it will return null, as
expected.

* Source/WTF/wtf/ThreadSafeWeakHashSet.h:
* Source/WTF/wtf/ThreadSafeWeakPtr.h:
(WTF::ThreadSafeWeakPtrControlBlock::objectHasStartedDeletion const):
(WTF::ThreadSafeWeakPtr::ThreadSafeWeakPtr):
(WTF::ThreadSafeWeakPtr::operator=):
(WTF::ThreadSafeWeakPtrControlBlock::objectHasBeenDeleted const): Deleted.
* Source/WebCore/PAL/pal/cf/CoreMediaSoftLink.cpp:
* Source/WebCore/PAL/pal/cf/CoreMediaSoftLink.h:
* Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.h:
* Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm:
(WebCore::AudioSampleBufferCompressor::~AudioSampleBufferCompressor):
(WebCore::AudioSampleBufferCompressor::initialize):
* Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
(WebCore::MediaRecorderPrivateWriter::~MediaRecorderPrivateWriter):
* Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.h:
* Source/WebCore/platform/mediarecorder/cocoa/VideoSampleBufferCompressor.mm:
(WebCore::VideoSampleBufferCompressor::~VideoSampleBufferCompressor):
(WebCore::VideoSampleBufferCompressor::initialize):

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