Skip to content
Permalink
Browse files
[EME] Possible deadlock when aborting a SourceBufferPrivateAVFObjC ap…
…pend operation

https://bugs.webkit.org/show_bug.cgi?id=180486

Reviewed by Eric Carlson.

It's possible that an abort() operation which is blocked on the ongoing appendBuffer()
operation completing will deadlock forever, with either the willProvideContentKeyRequest
or didProvideContentKeyRequest callbacks blocking waiting to be run on the main thread
(which is itself blocked waiting for the append operation to complete).

To break this deadlock, add a new abortSemaphore which is signaled in the abort() method
and have the willProvide... and didProvide... methods block both on their own semaphores
as well as the abortSemaphore, allowing them to break out even if their main thread calls
never get a chance to execute.

* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(-[WebAVStreamDataParserListener streamDataParserWillProvideContentKeyRequestInitializationData:forTrackID:]):
(-[WebAVStreamDataParserListener streamDataParser:didProvideContentKeyRequestInitializationData:forTrackID:]):
(WebCore::SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC):
(WebCore::SourceBufferPrivateAVFObjC::abort):


Canonical link: https://commits.webkit.org/196455@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225635 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
jernoble committed Dec 7, 2017
1 parent 665ee3a commit 2866ff26b5ff3263fbe162526e15537443f08c64
Showing 2 changed files with 52 additions and 2 deletions.
@@ -1,3 +1,26 @@
2017-12-07 Jer Noble <jer.noble@apple.com>

[EME] Possible deadlock when aborting a SourceBufferPrivateAVFObjC append operation
https://bugs.webkit.org/show_bug.cgi?id=180486

Reviewed by Eric Carlson.

It's possible that an abort() operation which is blocked on the ongoing appendBuffer()
operation completing will deadlock forever, with either the willProvideContentKeyRequest
or didProvideContentKeyRequest callbacks blocking waiting to be run on the main thread
(which is itself blocked waiting for the append operation to complete).

To break this deadlock, add a new abortSemaphore which is signaled in the abort() method
and have the willProvide... and didProvide... methods block both on their own semaphores
as well as the abortSemaphore, allowing them to break out even if their main thread calls
never get a chance to execute.

* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(-[WebAVStreamDataParserListener streamDataParserWillProvideContentKeyRequestInitializationData:forTrackID:]):
(-[WebAVStreamDataParserListener streamDataParser:didProvideContentKeyRequestInitializationData:forTrackID:]):
(WebCore::SourceBufferPrivateAVFObjC::SourceBufferPrivateAVFObjC):
(WebCore::SourceBufferPrivateAVFObjC::abort):

2017-12-07 Eric Carlson <eric.carlson@apple.com>

Simplify log channel configuration UI
@@ -108,9 +108,11 @@ - (void)removeStreamDataParser:(AVStreamDataParser *)streamDataParser;

@interface WebAVStreamDataParserListener : NSObject<AVStreamDataParserOutputHandling> {
WeakPtr<WebCore::SourceBufferPrivateAVFObjC> _parent;
OSObjectPtr<dispatch_semaphore_t> _abortSemaphore;
AVStreamDataParser* _parser;
}
@property (assign) WeakPtr<WebCore::SourceBufferPrivateAVFObjC> parent;
@property (assign) OSObjectPtr<dispatch_semaphore_t> abortSemaphore;
- (id)initWithParser:(AVStreamDataParser*)parser parent:(WeakPtr<WebCore::SourceBufferPrivateAVFObjC>)parent;
@end

@@ -129,6 +131,7 @@ - (id)initWithParser:(AVStreamDataParser*)parser parent:(WeakPtr<WebCore::Source
}

@synthesize parent=_parent;
@synthesize abortSemaphore=_abortSemaphore;

- (void)dealloc
{
@@ -203,10 +206,22 @@ - (void)streamDataParserWillProvideContentKeyRequestInitializationData:(AVStream

// We must call synchronously to the main thread, as the AVStreamSession must be associated
// with the streamDataParser before the delegate method returns.
dispatch_sync(dispatch_get_main_queue(), [parent = _parent, trackID]() {
OSObjectPtr<dispatch_semaphore_t> respondedSemaphore = adoptOSObject(dispatch_semaphore_create(0));
callOnMainThread([parent = _parent, trackID, respondedSemaphore]() {
if (parent)
parent->willProvideContentKeyRequestInitializationDataForTrackID(trackID);
dispatch_semaphore_signal(respondedSemaphore.get());
});

while (true) {
if (!dispatch_semaphore_wait(respondedSemaphore.get(), dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_MSEC * 100)))
return;

if (!dispatch_semaphore_wait(_abortSemaphore.get(), dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_MSEC * 100))) {
dispatch_semaphore_signal(_abortSemaphore.get());
return;
}
}
}

- (void)streamDataParser:(AVStreamDataParser *)streamDataParser didProvideContentKeyRequestInitializationData:(NSData *)initData forTrackID:(CMPersistentTrackID)trackID
@@ -218,7 +233,16 @@ - (void)streamDataParser:(AVStreamDataParser *)streamDataParser didProvideConten
if (parent)
parent->didProvideContentKeyRequestInitializationDataForTrackID(protectedInitData.get(), trackID, hasSessionSemaphore);
});
dispatch_semaphore_wait(hasSessionSemaphore.get(), DISPATCH_TIME_FOREVER);

while (true) {
if (!dispatch_semaphore_wait(hasSessionSemaphore.get(), dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_MSEC * 100)))
return;

if (!dispatch_semaphore_wait(_abortSemaphore.get(), dispatch_time(DISPATCH_TIME_NOW, NSEC_PER_MSEC * 100))) {
dispatch_semaphore_signal(_abortSemaphore.get());
return;
}
}
}
@end

@@ -487,6 +511,7 @@ static void bufferWasConsumedCallback(CMNotificationCenterRef, const void*, CFSt
, m_mediaSource(parent)
{
CMNotificationCenterAddListener(CMNotificationCenterGetDefaultLocalCenter(), this, bufferWasConsumedCallback, kCMSampleBufferConsumerNotification_BufferConsumed, nullptr, 0);
m_delegate.get().abortSemaphore = adoptOSObject(dispatch_semaphore_create(0));
}

SourceBufferPrivateAVFObjC::~SourceBufferPrivateAVFObjC()
@@ -713,9 +738,11 @@ static dispatch_queue_t globalDataParserQueue()
// semaphore, the m_isAppendingGroup wait operation will deadlock.
if (m_hasSessionSemaphore)
dispatch_semaphore_signal(m_hasSessionSemaphore.get());
dispatch_semaphore_signal(m_delegate.get().abortSemaphore.get());
dispatch_group_wait(m_isAppendingGroup.get(), DISPATCH_TIME_FOREVER);
m_appendWeakFactory.revokeAll();
m_delegate.get().parent = m_appendWeakFactory.createWeakPtr(*this);
m_delegate.get().abortSemaphore = adoptOSObject(dispatch_semaphore_create(0));
}

void SourceBufferPrivateAVFObjC::resetParserState()

0 comments on commit 2866ff2

Please sign in to comment.