200324 2019-07-31 16:49:04 -0700 [iOS][WK1] Unsafe unsafe of WeakPtr<Document> from UIThread under PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit() 2019-08-01 07:06:16 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 199922 1 cdumez cdumez commit-queue eric.carlson jer.noble joepeck rniwa webkit-bug-importer youennf oldest_to_newest 1557449 0 cdumez 2019-07-31 16:49:04 -0700 Unsafe unsafe of WeakPtr<Document> from UIThread under PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit(): TestWebKitAPI.WebKitLegacy.AudioSessionCategoryIOS ASSERTION FAILED: Thread::mayBeGCThread() || m_wasConstructedOnMainThread == isMainThread() /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug-iphonesimulator/usr/local/include/wtf/WeakPtr.h(65) : typename T::WeakValueType *WTF::WeakPtrImpl::get() [T = WebCore::Document] 1 0x10f4ded09 WTFCrash 2 0x11c87d34b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x11df066cc WebCore::Document::WeakValueType* WTF::WeakPtrImpl::get<WebCore::Document>() 4 0x11df06617 WTF::WeakPtr<WebCore::Document>::get() const 5 0x11df686c5 WTF::WeakPtr<WebCore::Document>::operator->() const 6 0x11f6fe626 WebCore::Quirks::needsQuirks() const 7 0x11f6fe799 WebCore::Quirks::needsSeekingSupportDisabled() const 8 0x11ef28c7f WebCore::HTMLMediaElement::supportsSeeking() const 9 0x11ceb0991 WebCore::PlaybackSessionModelMediaElement::duration() const 10 0x1206fbce5 VideoFullscreenControllerContext::duration() const 11 0x11cbbf20d WebCore::PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit(WebCore::PlaybackSessionModel&) 12 0x11cbbf89d WebCore::PlaybackSessionInterfaceAVKit::PlaybackSessionInterfaceAVKit(WebCore::PlaybackSessionModel&) 13 0x120702811 WebCore::PlaybackSessionInterfaceAVKit::create(WebCore::PlaybackSessionModel&) 14 0x1206fd5c2 VideoFullscreenControllerContext::setUpFullscreen(WebCore::HTMLVideoElement&, UIView*, unsigned int)::$_42::operator()() const 15 0x1206fd529 invocation function for block in VideoFullscreenControllerContext::setUpFullscreen(WebCore::HTMLVideoElement&, UIView*, unsigned int) 16 0x7fff52661888 _dispatch_call_block_and_release 17 0x7fff526627f9 _dispatch_client_callout 18 0x7fff5266ed22 _dispatch_main_queue_callback_4CF 19 0x7fff23b6de19 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ 20 0x7fff23b68a79 __CFRunLoopRun 21 0x7fff23b67e36 CFRunLoopRunSpecific 22 0x7fff2573d26f -[NSRunLoop(NSRunLoop) runMode:beforeDate:] 23 0x10adf0e46 TestWebKitAPI::Util::run(bool*) 24 0x10a98f4f0 TestWebKitAPI::WebKitLegacy_AudioSessionCategoryIOS_Test::TestBody() 25 0x10af2c56e void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) 26 0x10af0ad6b void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) 27 0x10af0ac96 testing::Test::Run() 28 0x10af0bb15 testing::TestInfo::Run() 29 0x10af0c9cf testing::TestCase::Run() 30 0x10af17d66 testing::internal::UnitTestImpl::RunAllTests() 31 0x10af3071e bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) Child process terminated with signal 11: Segmentation fault 1557450 1 375268 cdumez 2019-07-31 16:50:54 -0700 Created attachment 375268 Patch 1557482 2 rniwa 2019-07-31 18:43:34 -0700 I think the bug here is that VideoFullscreenControllerContext::setUpFullscreen isn't grabbing the web thread lock. If the web thread lock is held, isMainThread() should return true. 1557492 3 joepeck 2019-07-31 19:03:03 -0700 (In reply to Ryosuke Niwa from comment #2) > I think the bug here is that > VideoFullscreenControllerContext::setUpFullscreen isn't grabbing the web > thread lock. If the web thread lock is held, isMainThread() should return > true. Indeed. Source/WebCore/platform/iOS/WebVideoFullscreenControllerAVKit.mm: `VideoFullscreenControllerContext::setUpFullscreen` has: dispatch_async(dispatch_get_main_queue(), [protectedThis = makeRefPtr(this), this, videoElementClientRect, viewRef, mode, allowsPictureInPicture] { ASSERT(isUIThread()); Ref<PlaybackSessionInterfaceAVKit> sessionInterface = PlaybackSessionInterfaceAVKit::create(*this); m_interface = VideoFullscreenInterfaceAVKit::create(sessionInterface.get()); m_interface->setVideoFullscreenChangeObserver(this); m_interface->setVideoFullscreenModel(this); m_interface->setVideoFullscreenChangeObserver(this); m_videoFullscreenView = adoptNS([PAL::allocUIViewInstance() init]); m_interface->setupFullscreen(*m_videoFullscreenView.get(), videoElementClientRect, viewRef.get(), mode, allowsPictureInPicture, false); }); The block may be doing all kinds of things inside of WebCore on the UIThread without a WebThreadLock which could be unsafe. Plugging a single case (HTMLMediaElement::supportsSeeking) may just hide this issue until another issue later on. It would be better if we were able to use `WebThreadRun` or just took a `WebThreadLock` in here. It looks like this file already makes heavy use of `WebThreadRun` so maybe we can use that! I find this file rather confusing as the entire file is guarded by `#if PLATFORM(IOS_FAMILY)` but there are many equivalent feature guards within the file. Seems the sub-cases are stale and could simplify the file quite a bit. 1557500 4 375282 cdumez 2019-07-31 20:05:43 -0700 Created attachment 375282 Patch 1557501 5 cdumez 2019-07-31 20:06:23 -0700 Thanks for the feedback, I am super unfamiliar with the WebThread lock. I tried it and it fixes the crash nicely. 1557589 6 375282 commit-queue 2019-08-01 07:05:19 -0700 Comment on attachment 375282 Patch Clearing flags on attachment: 375282 Committed r248101: <https://trac.webkit.org/changeset/248101> 1557590 7 commit-queue 2019-08-01 07:05:21 -0700 All reviewed patches have been landed. Closing bug. 1557591 8 webkit-bug-importer 2019-08-01 07:06:16 -0700 <rdar://problem/53812730> 375268 2019-07-31 16:50:54 -0700 2019-07-31 20:05:41 -0700 Patch bug-200324-20190731165051.patch text/plain 2829 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ4MDQyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTBlNzFjNTYzMzM0ODg0 YTBiZDIxOGE5MjI0YzY5NmVhMDNjY2E0MC4uYWZjYTI0ZTFmMDYxNTYzZmYyZTcyZWJlZGJjMTQ1 ZTNkZjgwNDM0MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE5LTA3LTMxICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgW2lPU11bV0sxXSBVbnNhZmUg dW5zYWZlIG9mIFdlYWtQdHI8RG9jdW1lbnQ+IGZyb20gVUlUaHJlYWQgdW5kZXIgUGxheWJhY2tT ZXNzaW9uSW50ZXJmYWNlQVZLaXQ6OlBsYXliYWNrU2Vzc2lvbkludGVyZmFjZUFWS2l0KCkKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwMDMyNAorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBEb2N1bWVu dCBpcyBhIFdlYlRocmVhZCBvYmplY3QsIGJ1dCBhIFdlYWtQdHI8RG9jdW1lbnQ+IHdhcyBkZXJl ZmVyZW5jZWQgZnJvbSB0aGUKKyAgICAgICAgVUlUaHJlYWQgaW4gSFRNTE1lZGlhRWxlbWVudDo6 c3VwcG9ydHNTZWVraW5nKCkuIFRvIGFkZHJlc3MgdGhlIGlzc3VlLCBzYXZlIHRoZSByZXN1bHQK KyAgICAgICAgZnJvbSBkb2N1bWVudC5xdWlya3MoKS5uZWVkc1NlZWtpbmdTdXBwb3J0RGlzYWJs ZWQoKSBpbnNpZGUgdGhlIEhUTUxNZWRpYUVsZW1lbnQKKyAgICAgICAgY29uc3RydWN0b3IuCisK KyAgICAgICAgKiBodG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6 SFRNTE1lZGlhRWxlbWVudDo6SFRNTE1lZGlhRWxlbWVudCk6CisgICAgICAgIChXZWJDb3JlOjpI VE1MTWVkaWFFbGVtZW50OjpzdXBwb3J0c1NlZWtpbmcgY29uc3QpOgorICAgICAgICAqIGh0bWwv SFRNTE1lZGlhRWxlbWVudC5oOgorCiAyMDE5LTA3LTMxICBDaHJpcyBEdW1leiAgPGNkdW1lekBh cHBsZS5jb20+CiAKICAgICAgICAgRWxlbWVudC5vdXRlckhUTUwgaXMgbWlzc2luZyBhdHRyaWJ1 dGUgcHJlZml4ZXMgaW4gc29tZSBjYXNlcyBpbiBIVE1MIGRvY3VtZW50cwpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcCBiL1NvdXJjZS9XZWJDb3Jl L2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAKaW5kZXggODg1Y2I1NGM1OWJlYWQ1NGE1YmNmY2Uz MDhhYjdhOWVjMTVkZmI0My4uYzlmYWI2NjM5OGQ5NWIwZDE3YTBmYThkNDgzNTY1YmI3NDEzMThk NiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcAor KysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwCkBAIC00NzAsNiAr NDcwLDcgQEAgSFRNTE1lZGlhRWxlbWVudDo6SFRNTE1lZGlhRWxlbWVudChjb25zdCBRdWFsaWZp ZWROYW1lJiB0YWdOYW1lLCBEb2N1bWVudCYgZG9jdW0KICAgICAsIG1fbG9nZ2VyKCZkb2N1bWVu dC5sb2dnZXIoKSkKICAgICAsIG1fbG9nSWRlbnRpZmllcih1bmlxdWVMb2dJZGVudGlmaWVyKCkp CiAjZW5kaWYKKyAgICAsIG1fbmVlZHNTZWVraW5nU3VwcG9ydERpc2FibGVkKGRvY3VtZW50LnF1 aXJrcygpLm5lZWRzU2Vla2luZ1N1cHBvcnREaXNhYmxlZCgpKQogewogICAgIGFsbE1lZGlhRWxl bWVudHMoKS5hZGQodGhpcyk7CiAKQEAgLTc2OTUsNyArNzY5Niw3IEBAIHZvaWQgSFRNTE1lZGlh RWxlbWVudDo6ZGlkUmVjZWl2ZVJlbW90ZUNvbnRyb2xDb21tYW5kKFBsYXRmb3JtTWVkaWFTZXNz aW9uOjpSZW1vCiAKIGJvb2wgSFRNTE1lZGlhRWxlbWVudDo6c3VwcG9ydHNTZWVraW5nKCkgY29u c3QKIHsKLSAgICByZXR1cm4gIWRvY3VtZW50KCkucXVpcmtzKCkubmVlZHNTZWVraW5nU3VwcG9y dERpc2FibGVkKCkgJiYgIWlzTGl2ZVN0cmVhbSgpOworICAgIHJldHVybiAhbV9uZWVkc1NlZWtp bmdTdXBwb3J0RGlzYWJsZWQgJiYgIWlzTGl2ZVN0cmVhbSgpOwogfQogCiBib29sIEhUTUxNZWRp YUVsZW1lbnQ6OnNob3VsZE92ZXJyaWRlQmFja2dyb3VuZFBsYXliYWNrUmVzdHJpY3Rpb24oUGxh dGZvcm1NZWRpYVNlc3Npb246OkludGVycnVwdGlvblR5cGUgdHlwZSkgY29uc3QKZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5oIGIvU291cmNlL1dlYkNv cmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmgKaW5kZXggZDBiYWI5ZmM5ZDZlZGExYzFiYzJhNjA5 NjgzZjI3NmZlNzYwM2NmOC4uYTA0OGNiNjE0MWY0ZjkwNjY5MTU1MGEzYzlhNDFiNzA1MzFkYzEz ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmgKKysr IGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmgKQEAgLTEyMDUsNiArMTIw NSw3IEBAIHByaXZhdGU6CiAgICAgYm9vbCBtX2lzUGxheWluZ1RvV2lyZWxlc3NUYXJnZXQgeyBm YWxzZSB9OwogICAgIGJvb2wgbV9wbGF5aW5nT25TZWNvbmRTY3JlZW4geyBmYWxzZSB9OwogICAg IGJvb2wgbV9yZW1vdmVkQmVoYXZpb3JSZXN0cmljdGlvbnNBZnRlckZpcnN0VXNlckdlc3R1cmUg eyBmYWxzZSB9OworICAgIGJvb2wgbV9uZWVkc1NlZWtpbmdTdXBwb3J0RGlzYWJsZWQgeyBmYWxz ZSB9OwogfTsKIAogU3RyaW5nIGNvbnZlcnRFbnVtZXJhdGlvblRvU3RyaW5nKEhUTUxNZWRpYUVs ZW1lbnQ6OkF1dG9wbGF5RXZlbnRQbGF5YmFja1N0YXRlKTsK 375282 2019-07-31 20:05:43 -0700 2019-08-01 07:05:19 -0700 Patch bug-200324-20190731200540.patch text/plain 2037 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ4MDQyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMTBlNzFjNTYzMzM0ODg0 YTBiZDIxOGE5MjI0YzY5NmVhMDNjY2E0MC4uNmJiZTUwYzMzMDc5N2E2YzQ3ZjE5ZDRiM2MyMDIw YjlkNDk3ZmJiZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE5LTA3LTMxICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgW2lPU11bV0sxXSBVbnNhZmUg dW5zYWZlIG9mIFdlYWtQdHI8RG9jdW1lbnQ+IGZyb20gVUlUaHJlYWQgdW5kZXIgUGxheWJhY2tT ZXNzaW9uSW50ZXJmYWNlQVZLaXQ6OlBsYXliYWNrU2Vzc2lvbkludGVyZmFjZUFWS2l0KCkKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwMDMyNAorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBEb2N1bWVu dCBpcyBhIFdlYlRocmVhZCBvYmplY3QsIGJ1dCBhIFdlYWtQdHI8RG9jdW1lbnQ+IHdhcyBkZXJl ZmVyZW5jZWQgZnJvbSB0aGUKKyAgICAgICAgVUlUaHJlYWQgaW4gSFRNTE1lZGlhRWxlbWVudDo6 c3VwcG9ydHNTZWVraW5nKCksIGZyb20gdGhlIFBsYXliYWNrU2Vzc2lvbkludGVyZmFjZUFWS2l0 CisgICAgICAgIGNvbnN0cnVjdG9yLiBUbyBhZGRyZXNzIHRoZSBpc3N1ZSB3ZSBub3cgZ3JhYiB0 aGUgV2ViVGhyZWFkIGxvY2sgYmVmb3JlIGNvbnN0cnVjdGluZworICAgICAgICB0aGUgUGxheWJh Y2tTZXNzaW9uSW50ZXJmYWNlQVZLaXQuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9pb3MvV2ViVmlk ZW9GdWxsc2NyZWVuQ29udHJvbGxlckFWS2l0Lm1tOgorICAgICAgICAoVmlkZW9GdWxsc2NyZWVu Q29udHJvbGxlckNvbnRleHQ6OnNldFVwRnVsbHNjcmVlbik6CisKIDIwMTktMDctMzEgIENocmlz IER1bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBFbGVtZW50Lm91dGVySFRNTCBp cyBtaXNzaW5nIGF0dHJpYnV0ZSBwcmVmaXhlcyBpbiBzb21lIGNhc2VzIGluIEhUTUwgZG9jdW1l bnRzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3MvV2ViVmlkZW9GdWxs c2NyZWVuQ29udHJvbGxlckFWS2l0Lm1tIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW9zL1dl YlZpZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJBVktpdC5tbQppbmRleCA0MzZhOTcwNTkyZThhNTg1 NTg0ZDVhZjU3NDdmNGFiNjgyNDY0MTQ0Li5mZWQ0NDIwZjFjZDVmNDQ4ODcxMzMyZWU1YzMzZDNl M2ZiY2FiYTRjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3MvV2ViVmlk ZW9GdWxsc2NyZWVuQ29udHJvbGxlckFWS2l0Lm1tCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2lvcy9XZWJWaWRlb0Z1bGxzY3JlZW5Db250cm9sbGVyQVZLaXQubW0KQEAgLTk3Nyw2ICs5 NzcsNyBAQCB2b2lkIFZpZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJDb250ZXh0OjpzZXRVcEZ1bGxz Y3JlZW4oSFRNTFZpZGVvRWxlbWVudCYgdmlkZW9FbAogCiAgICAgZGlzcGF0Y2hfYXN5bmMoZGlz cGF0Y2hfZ2V0X21haW5fcXVldWUoKSwgW3Byb3RlY3RlZFRoaXMgPSBtYWtlUmVmUHRyKHRoaXMp LCB0aGlzLCB2aWRlb0VsZW1lbnRDbGllbnRSZWN0LCB2aWV3UmVmLCBtb2RlLCBhbGxvd3NQaWN0 dXJlSW5QaWN0dXJlXSB7CiAgICAgICAgIEFTU0VSVChpc1VJVGhyZWFkKCkpOworICAgICAgICBX ZWJUaHJlYWRMb2NrKCk7CiAKICAgICAgICAgUmVmPFBsYXliYWNrU2Vzc2lvbkludGVyZmFjZUFW S2l0PiBzZXNzaW9uSW50ZXJmYWNlID0gUGxheWJhY2tTZXNzaW9uSW50ZXJmYWNlQVZLaXQ6OmNy ZWF0ZSgqdGhpcyk7CiAgICAgICAgIG1faW50ZXJmYWNlID0gVmlkZW9GdWxsc2NyZWVuSW50ZXJm YWNlQVZLaXQ6OmNyZWF0ZShzZXNzaW9uSW50ZXJmYWNlLmdldCgpKTsK