Bug 147085 - -[AVPlayerItem currentTime] and -[AVPlayer isExternalPlaybackActive] deadlock
Summary: -[AVPlayerItem currentTime] and -[AVPlayer isExternalPlaybackActive] deadlock
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ada Chan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-18 23:12 PDT by Ada Chan
Modified: 2015-07-20 22:42 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.43 KB, patch)
2015-07-18 23:51 PDT, Ada Chan
jer.noble: review+
Details | Formatted Diff | Diff
Followup patch (2.11 KB, patch)
2015-07-20 21:57 PDT, Ada Chan
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ada Chan 2015-07-18 23:12:43 PDT
Repro steps:
https://files.scene.org/get/parties/2015/solskogen15/demo_newschool/primitive-_-i_want_to_see_other_worlds.html.zip

Extract, open the HTML
In Safari, for “Fullscreen?” click “No”
Web process hangs in MediaPlayerPrivateAVFoundationObjC::currentMediaTime()

The main thread is stuck here:
#0	0x00007fff8b613f02 in __psynch_mutexwait ()
...
#6	0x00007fff9ad38eb3 in -[AVPlayerItem currentTime] ()
#7	0x0000000111fbf42b in WebCore::MediaPlayerPrivateAVFoundationObjC::currentMediaTime() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1288
#8	0x0000000111fa6ec2 in WebCore::MediaPlayer::currentTime() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/platform/graphics/MediaPlayer.cpp:535
#9	0x0000000111576986 in WebCore::HTMLMediaElement::refreshCachedTime() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:2584
#10	0x000000011157c2fe in WebCore::HTMLMediaElement::currentMediaTime() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:2671
#11	0x0000000111581907 in WebCore::HTMLMediaElement::endedPlayback() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:4586
#12	0x00000001115883ac in WebCore::HTMLMediaElement::couldPlayIfEnoughData() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:4558
#13	0x000000011157d688 in WebCore::HTMLMediaElement::potentiallyPlaying() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:4544
#14	0x000000011157dddf in WebCore::HTMLMediaElement::updatePlayState() at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:4676
#15	0x0000000111581e5b in WebCore::HTMLMediaElement::playInternal() at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:2893
#16	0x0000000111581c40 in WebCore::HTMLMediaElement::play() at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:2839
#17	0x0000000111b0a4e1 in WebCore::jsHTMLMediaElementPrototypeFunctionPlay(JSC::ExecState*) at /Volumes/Data/Users/adachan/Build/Debug/DerivedSources/WebCore/JSHTMLMediaElement.cpp:1531

The audio I/O thread is stuck here:
#0	0x00007fff8b60e932 in semaphore_wait_trap ()
...
#7	0x00007fff9ada0423 in -[AVPlayer isExternalPlaybackActive] ()
#8	0x0000000111fc904c in WebCore::MediaPlayerPrivateAVFoundationObjC::isCurrentPlaybackTargetWireless() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2743
#9	0x0000000111fa7f8b in WebCore::MediaPlayer::isCurrentPlaybackTargetWireless() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/platform/graphics/MediaPlayer.cpp:859
#10	0x000000011158dadc in WebCore::HTMLMediaElement::mediaState() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:6392
#11	0x000000011158dd2c in non-virtual thunk to WebCore::HTMLMediaElement::mediaState() const at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/html/HTMLMediaElement.cpp:6384
#12	0x000000011100b696 in WebCore::Document::updateIsPlayingMedia() at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/dom/Document.cpp:3496
#13	0x0000000110b4bb0d in WebCore::AudioContext::isPlayingAudioDidChange() at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/Modules/webaudio/AudioContext.cpp:1080
#14	0x0000000110b69074 in WebCore::AudioDestinationNode::updateIsEffectivelyPlayingAudio() at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/Modules/webaudio/AudioDestinationNode.cpp:128
#15	0x0000000110b68f53 in WebCore::AudioDestinationNode::setIsSilent(bool) at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/Modules/webaudio/AudioDestinationNode.cpp:117
#16	0x0000000110b68ecf in WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long) at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/Modules/webaudio/AudioDestinationNode.cpp:98
#17	0x0000000110b68f9f in non-virtual thunk to WebCore::AudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long) at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/Modules/webaudio/AudioDestinationNode.cpp:56
#18	0x0000000110b687d4 in WebCore::AudioDestinationMac::render(unsigned int, AudioBufferList*) at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:165
#19	0x0000000110b68607 in WebCore::AudioDestinationMac::inputProc(void*, unsigned int*, AudioTimeStamp const*, unsigned int, unsigned int, AudioBufferList*) at /Volumes/Data/Users/adachan/WebKit/OpenSource/Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:189
#20	0x000000012b97a0b3 in AUInputElement::PullInput(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int) ()

The audio I/O thread should never call into WebCore in a render callback, since the call into WebCore might block and we'll end up with a deadlock.  We should make sure that call is done asynchronously on the main thread.
Comment 1 Ada Chan 2015-07-18 23:28:58 PDT
<rdar://problem/21878275>
Comment 2 Ada Chan 2015-07-18 23:48:59 PDT
Eric Carlson suggested this change:

 void AudioContext::isPlayingAudioDidChange()
 {
-    document()->updateIsPlayingMedia();
+    RefPtr<AudioContext> strongThis(this);
+    callOnMainThread([strongThis] {
+        strongThis->document()->updateIsPlayingMedia();
+    });
 }

I could reproduce this deadlock before this change, and could not reproduce the deadlock anymore after this change.

Posting a patch very soon.
Comment 3 Ada Chan 2015-07-18 23:51:04 PDT
Created attachment 257056 [details]
Patch

Fix as suggested by Eric.  Thanks Eric!
Comment 4 Ada Chan 2015-07-20 11:10:02 PDT
Committed:
http://trac.webkit.org/changeset/187025
Comment 5 Darin Adler 2015-07-20 17:44:35 PDT
Comment on attachment 257056 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=257056&action=review

> Source/WebCore/Modules/webaudio/AudioContext.cpp:1076
> +        strongThis->document()->updateIsPlayingMedia();

What guarantees that document() is non-null later when this is called on the main thread? It seems we could have a problem if this is pending when the audio context’s document is destroyed.
Comment 6 Ada Chan 2015-07-20 21:55:59 PDT
That's a good point. I'll post a followup patch to address that.
Comment 7 Ada Chan 2015-07-20 21:57:09 PDT
Created attachment 257160 [details]
Followup patch

Add a null check for document() in case the audio context's document has been destroyed by the time it's called on the main thread.
Comment 8 Ada Chan 2015-07-20 22:42:13 PDT
Thanks Eric!

Follow-up patch has been committed:
http://trac.webkit.org/changeset/187098