200599 2019-08-09 16:25:18 -0700 Possible non-thread safe usage of RefCounted in ~VideoFullscreenControllerContext() 2019-08-09 17:57:15 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 200507 1 cdumez cdumez achristensen commit-queue eric.carlson ggaren jer.noble rniwa webkit-bug-importer oldest_to_newest 1560111 0 cdumez 2019-08-09 16:25:18 -0700 Possible non-thread safe usage of RefCounted in ~VideoFullscreenControllerContext(): ASSERTION FAILED: !areThreadingCheckedEnabled() || m_isOwnedByMainThread == isMainThread() /Volumes/Data/WebKit/OpenSource/WebKitBuild/Debug-iphonesimulator/usr/local/include/wtf/RefCounted.h(117) : bool WTF::RefCountedBase::derefBase() const 1 0x1106c60c9 WTFCrash 2 0x11e5e8dbb WTFCrashWithInfo(int, char const*, char const*, int) 3 0x11e5fd0be WTF::RefCountedBase::derefBase() const 4 0x11e6a457f WTF::RefCounted<WebCore::EventListener>::deref() const 5 0x12248ce25 void WTF::derefIfNotNull<WebCore::PlaybackSessionModelMediaElement>(WebCore::PlaybackSessionModelMediaElement*) 6 0x12248cde9 WTF::RefPtr<WebCore::PlaybackSessionModelMediaElement, WTF::DumbPtrTraits<WebCore::PlaybackSessionModelMediaElement> >::~RefPtr() 7 0x12247b215 WTF::RefPtr<WebCore::PlaybackSessionModelMediaElement, WTF::DumbPtrTraits<WebCore::PlaybackSessionModelMediaElement> >::~RefPtr() 8 0x12247aef5 VideoFullscreenControllerContext::~VideoFullscreenControllerContext() 9 0x12247b275 VideoFullscreenControllerContext::~VideoFullscreenControllerContext() 10 0x12247b319 VideoFullscreenControllerContext::~VideoFullscreenControllerContext() 11 0x12248cd82 WTF::ThreadSafeRefCounted<VideoFullscreenControllerContext, (WTF::DestructionThread)0>::deref() const::'lambda'()::operator()() const 12 0x12248cd3d WTF::ThreadSafeRefCounted<VideoFullscreenControllerContext, (WTF::DestructionThread)0>::deref() const 13 0x12248cfc5 void WTF::derefIfNotNull<VideoFullscreenControllerContext>(VideoFullscreenControllerContext*) 14 0x12248cf89 WTF::RefPtr<VideoFullscreenControllerContext, WTF::DumbPtrTraits<VideoFullscreenControllerContext> >::~RefPtr() 15 0x12247bbe5 WTF::RefPtr<VideoFullscreenControllerContext, WTF::DumbPtrTraits<VideoFullscreenControllerContext> >::~RefPtr() 16 0x12248afc5 VideoFullscreenControllerContext::volumeChanged(double)::$_21::~$_21() 17 0x122480725 VideoFullscreenControllerContext::volumeChanged(double)::$_21::~$_21() 18 0x122480709 __destroy_helper_block_e8_32c64_ZTSKZN32VideoFullscreenControllerContext13volumeChangedEdE4$_21 19 0x7fff52a089c2 _Block_release 20 0x7fff529777f9 _dispatch_client_callout 21 0x7fff52983d22 _dispatch_main_queue_callback_4CF 22 0x7fff23b72e19 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ 23 0x7fff23b6da79 __CFRunLoopRun 24 0x7fff23b6ce36 CFRunLoopRunSpecific 25 0x7fff2574803f -[NSRunLoop(NSRunLoop) runMode:beforeDate:] 26 0x10f6575c6 TestWebKitAPI::Util::run(bool*) 27 0x10f1f4cd4 TestWebKitAPI::WebKitLegacy_AudioSessionCategoryIOS_Test::TestBody() 28 0x10f794dee void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) 29 0x10f7735eb void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) 30 0x10f773516 testing::Test::Run() 31 0x10f774395 testing::TestInfo::Run() 1560116 1 cdumez 2019-08-09 16:32:36 -0700 Likely the ref'ing of |this| here: void VideoFullscreenControllerContext::volumeChanged(double volume) { if (WebThreadIsCurrent()) { dispatch_async(dispatch_get_main_queue(), [protectedThis = makeRefPtr(this), volume] { protectedThis->volumeChanged(volume); }); return; } 1560117 2 cdumez 2019-08-09 16:35:49 -0700 So we end up destroying the VideoFullscreenControllerContext on the UIThread, which is fine. However, this ends up deref'ing a PlaybackSessionModelMediaElement on the UIThread, even though PlaybackSessionModelMediaElement objects are WebThread objects (and we're not holding the WebThreadLock). 1560119 3 cdumez 2019-08-09 16:38:15 -0700 This is likely VideoFullscreenControllerContext::m_playbackModel, which is a RefPtr<PlaybackSessionModelMediaElement> and PlaybackSessionModelMediaElement is RefCounted (not ThreadSafeRefCounted). We could mark it as ThreadSafeRefCounted but I doubt this is the right fix here since we may end up destroying the object on the wrong thread. It is more likely I need to make sure the object gets destroyed on the WebThread, or grab the WebThreadLock. 1560124 4 375980 cdumez 2019-08-09 16:54:30 -0700 Created attachment 375980 Patch 1560134 5 375980 ggaren 2019-08-09 17:14:38 -0700 Comment on attachment 375980 Patch r=me 1560143 6 375980 commit-queue 2019-08-09 17:56:37 -0700 Comment on attachment 375980 Patch Clearing flags on attachment: 375980 Committed r248492: <https://trac.webkit.org/changeset/248492> 1560144 7 commit-queue 2019-08-09 17:56:39 -0700 All reviewed patches have been landed. Closing bug. 1560145 8 webkit-bug-importer 2019-08-09 17:57:15 -0700 <rdar://problem/54150479> 375980 2019-08-09 16:54:30 -0700 2019-08-09 17:56:37 -0700 Patch bug-200599-20190809165429.patch text/plain 1991 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ4NDgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2FjYzc1YmY4NTU5NTc1 ZTk2N2M1OGYwYThiNDQ3MTIyMzAxMjY4Ni4uNGQzN2E3YTIzMjkwMjNjYmY5NjE4OTBmODYxNzE2 YzQ1YmI4YWI0ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE5LTA4LTA5ICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgUG9zc2libGUgbm9uLXRocmVh ZCBzYWZlIHVzYWdlIG9mIFJlZkNvdW50ZWQgaW4gflZpZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJD b250ZXh0KCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTIwMDU5OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFdlYlZpZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJBVktpdCdzIG1fcGxheWJhY2tNb2RlbCAmIG1f ZnVsbHNjcmVlbk1vZGVsIGRhdGEgbWVtYmVycyBhcmUKKyAgICAgICAgV2ViVGhyZWFkIG9iamVj dHMgc28gd2UgbmVlZCB0byBtYWtlIHN1cmUgd2UgZ3JhYiB0aGUgV2ViVGhyZWFkIGxvY2sgYmVm b3JlIGRlcmVmZXJlbmNpbmcKKyAgICAgICAgdGhlbSBpbiB0aGUgV2ViVmlkZW9GdWxsc2NyZWVu Q29udHJvbGxlckFWS2l0IGRlc3RydWN0b3IsIHdoZW4gZGVzdHJveWVkIG9uIHRoZSBVSVRocmVh ZC4KKworICAgICAgICAqIHBsYXRmb3JtL2lvcy9XZWJWaWRlb0Z1bGxzY3JlZW5Db250cm9sbGVy QVZLaXQubW06CisgICAgICAgIChWaWRlb0Z1bGxzY3JlZW5Db250cm9sbGVyQ29udGV4dDo6flZp ZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJDb250ZXh0KToKKwogMjAxOS0wOC0wOSAgWW91ZW5uIEZh YmxldCAgPHlvdWVubkBhcHBsZS5jb20+CiAKICAgICAgICAgUGFzcyBhIFNjcmlwdEV4ZWN1dGlv bkNvbnRleHQgYXMgaW5wdXQgdG8gcmVnaXN0ZXIvdW5yZWdpc3RlciBVUkxSZWdpc3RyeSByb3V0 aW5lcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW9zL1dlYlZpZGVvRnVs bHNjcmVlbkNvbnRyb2xsZXJBVktpdC5tbSBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2lvcy9X ZWJWaWRlb0Z1bGxzY3JlZW5Db250cm9sbGVyQVZLaXQubW0KaW5kZXggZmVkNDQyMGYxY2Q1ZjQ0 ODg3MTMzMmVlNWMzM2QzZTNmYmNhYmE0Yy4uZjhhMzQ3NDYwNjEzZTFiNjZhMGE3MjM4NGFiZjJm YTQyNzI3N2I2ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW9zL1dlYlZp ZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJBVktpdC5tbQorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9pb3MvV2ViVmlkZW9GdWxsc2NyZWVuQ29udHJvbGxlckFWS2l0Lm1tCkBAIC0yMjYsOSAr MjI2LDEyIEBAIFZpZGVvRnVsbHNjcmVlbkNvbnRyb2xsZXJDb250ZXh0Ojp+VmlkZW9GdWxsc2Ny ZWVuQ29udHJvbGxlckNvbnRleHQoKQogICAgICAgICB3aGlsZSAoIW1fZnVsbHNjcmVlbkNsaWVu dHMuaXNFbXB0eSgpKQogICAgICAgICAgICAgKCptX2Z1bGxzY3JlZW5DbGllbnRzLmJlZ2luKCkp LT5tb2RlbERlc3Ryb3llZCgpOwogICAgIH07Ci0gICAgaWYgKGlzVUlUaHJlYWQoKSkKKyAgICBp ZiAoaXNVSVRocmVhZCgpKSB7CisgICAgICAgIFdlYlRocmVhZExvY2soKTsKICAgICAgICAgbm90 aWZ5Q2xpZW50c01vZGVsV2FzRGVzdHJveWVkKCk7Ci0gICAgZWxzZQorICAgICAgICBtX3BsYXli YWNrTW9kZWwgPSBudWxscHRyOworICAgICAgICBtX2Z1bGxzY3JlZW5Nb2RlbCA9IG51bGxwdHI7 CisgICAgfSBlbHNlCiAgICAgICAgIGRpc3BhdGNoX3N5bmMoZGlzcGF0Y2hfZ2V0X21haW5fcXVl dWUoKSwgV1RGTW92ZShub3RpZnlDbGllbnRzTW9kZWxXYXNEZXN0cm95ZWQpKTsKIH0KIAo=