175721 2017-08-18 09:20:15 -0700 REGRESSION (r220601): Crash when closing google doc after switching the order of tabs in safari 2020-05-29 11:45:04 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 212537 1 sam cdumez cdumez commit-queue ggaren sam webkit-bug-importer oldest_to_newest 1340096 0 sam 2017-08-18 09:20:15 -0700 https://trac.webkit.org/changeset/220601 caused the following crash on iOS. Thread 0 Crashed: 0 WebKit 0x00000001005ece90 WTF::Function<void ()>::CallableWrapper<WebKit::WebProcess::markAllLayersVolatile(WTF::Function<void ()>&&)::$_7>::call() + 28 1 WebKit 0x00000001005576f0 WebKit::WebPage::callVolatilityCompletionHandlers() + 72 2 WebKit 0x00000001005578f8 WebKit::WebPage::markLayersVolatile(WTF::Function<void ()>&&) + 488 3 WebKit 0x00000001005e8f78 WebKit::WebProcess::markAllLayersVolatile(WTF::Function<void ()>&&) + 292 4 WebKit 0x00000001005e8d78 WebKit::WebProcess::actualPrepareToSuspend(WebKit::WebProcess::ShouldAcknowledgeWhenReadyToSuspend) + 112 5 WebKit 0x00000001005e91a4 WebKit::WebProcess::prepareToSuspend() + 160 6 WebKit 0x00000001003da8b0 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 164 7 WebKit 0x00000001003dd2a4 IPC::Connection::dispatchOneMessage() + 232 8 JavaScriptCore 0x0000000101e16bb0 WTF::RunLoop::performWork() + 196 9 JavaScriptCore 0x0000000101e16dcc WTF::RunLoop::performWork(void*) + 36 10 CoreFoundation 0x00000001858a4358 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 24 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CF/CF-1443/RunLoop.subproj/CFRunLoop.c:1982) 11 CoreFoundation 0x00000001858a42d8 __CFRunLoopDoSource0 + 88 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CF/CF-1443/RunLoop.subproj/CFRunLoop.c:2017) 12 CoreFoundation 0x00000001858a3b60 __CFRunLoopDoSources0 + 204 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CF/CF-1443/RunLoop.subproj/CFRunLoop.c:2053) 13 CoreFoundation 0x00000001858a1738 __CFRunLoopRun + 1048 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CF/CF-1443/RunLoop.subproj/CFRunLoop.c:2920) 14 CoreFoundation 0x00000001857c22d8 CFRunLoopRunSpecific + 436 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/CF/CF-1443/RunLoop.subproj/CFRunLoop.c:3245) 15 Foundation 0x00000001861ea6e4 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 304 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/Foundation/Foundation-1444.12/Soil.subproj/NSRunLoop.m:367) 16 Foundation 0x000000018623c62c -[NSRunLoop(NSRunLoop) run] + 88 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/Foundation/Foundation-1444.12/Soil.subproj/NSRunLoop.m:389) 17 libxpc.dylib 0x00000001855702b0 _xpc_objc_main + 516 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/libxpc/libxpc-1205.20.17/src/main.m:167) 18 libxpc.dylib 0x000000018557233c xpc_main + 180 (/BuildRoot/Library/Caches/com.apple.xbs/Sources/libxpc/libxpc-1205.20.17/src/init.c:1470) 19 com.apple.WebKit.WebContent 0x0000000100187594 0x100184000 + 13716 20 libdyld.dylib 0x00000001852e656c start + 4 It is being tracked in radar with <rdar://problem/33928369>. The likely culprit is WebProcess::markAllLayersVolatile() which is moving a WTF::Function multiple times (for each page, see lines 1392-1393) leading nulled out functions being called. It looks like this was previously broken, but silent due to to the old nature of WTF::Function silently ignoring calls to null functions. 1340144 1 cdumez 2017-08-18 11:01:11 -0700 <rdar://problem/33928369> 1340181 2 318527 cdumez 2017-08-18 12:29:46 -0700 Created attachment 318527 Patch 1340190 3 318527 ggaren 2017-08-18 12:41:54 -0700 Comment on attachment 318527 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318527&action=review > Source/WebKit/WebProcess/WebProcess.cpp:1388 > + m_pageMarkingLayersAsVolatileCounter = std::make_unique<PageMarkingLayersAsVolatileCounter>([this, completionHandler = WTFMove(completionHandler)] (RefCounterEvent) { How do we know that the WebProcess object will still be alive when our callback fires? 1340191 4 318527 ggaren 2017-08-18 12:43:19 -0700 Comment on attachment 318527 Patch r=me Still a little sketched out by the refcounting here. 1340195 5 318527 cdumez 2017-08-18 12:47:28 -0700 Comment on attachment 318527 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318527&action=review >> Source/WebKit/WebProcess/WebProcess.cpp:1388 >> + m_pageMarkingLayersAsVolatileCounter = std::make_unique<PageMarkingLayersAsVolatileCounter>([this, completionHandler = WTFMove(completionHandler)] (RefCounterEvent) { > > How do we know that the WebProcess object will still be alive when our callback fires? If the WebProcess does away then so does the RefCounter object, since it is a data member. If the RefCounter object is destroyed, the token gets disconnected from the RefCounter object and the valueDidChange callback cannot get called. 1340197 6 ggaren 2017-08-18 12:51:03 -0700 > If the WebProcess does away then so does the RefCounter object, since it is > a data member. Oh, duh. :P 1340221 7 318527 commit-queue 2017-08-18 13:53:36 -0700 Comment on attachment 318527 Patch Clearing flags on attachment: 318527 Committed r220931: <http://trac.webkit.org/changeset/220931> 1340222 8 commit-queue 2017-08-18 13:53:38 -0700 All reviewed patches have been landed. Closing bug. 1340229 9 sam 2017-08-18 14:10:45 -0700 Heh. I guess I should have assigned this to myself when I filed it if I didn't want you to fix it before I did :). Thanks for tackling it! 1340235 10 cdumez 2017-08-18 14:27:05 -0700 (In reply to Sam Weinig from comment #9) > Heh. I guess I should have assigned this to myself when I filed it if I > didn't want you to fix it before I did :). Thanks for tackling it! Oh yes :) I saw you did not assign it to yourself and cc'd me and took it a a sign you wanted me to take it if I had time. 1657360 11 318527 dbates 2020-05-29 11:23:56 -0700 Comment on attachment 318527 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318527&action=review > Source/WebKit/WebProcess/WebProcess.cpp:1390 > + return; Completion handler never called. 1657369 12 318527 cdumez 2020-05-29 11:45:04 -0700 Comment on attachment 318527 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318527&action=review >> Source/WebKit/WebProcess/WebProcess.cpp:1390 >> + return; > > Completion handler never called. Well, the idea is that it is supposed to get called below when m_pageMarkingLayersAsVolatileCounter->value() becomes 0. 318527 2017-08-18 12:29:46 -0700 2017-08-18 13:53:36 -0700 Patch bug-175721-20170818122946.patch text/plain 4572 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwOTIyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGQ0NWM1MDdiNjJkZmU0NTJl YzVmZWIwNDZlM2VkYjY0ZDNjODkxNTUuLmJmYjFjNThlOGMxYTZhNWVjZGNlNzllMzAwYTE4ZWEy YTFmM2YzNzIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjMgQEAKIDIwMTctMDgtMTggIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAorICAgICAgICBSRUdSRVNTSU9OIChyMjIwNjAxKTog Q3Jhc2ggd2hlbiBjbG9zaW5nIGdvb2dsZSBkb2MgYWZ0ZXIgc3dpdGNoaW5nIHRoZSBvcmRlciBv ZiB0YWJzIGluIHNhZmFyaQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTc1NzIxCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8zMzkyODM2OT4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIHN1cmUgV2Vi UHJvY2Vzczo6bWFya0FsbExheWVyc1ZvbGF0aWxlKCkgZG9lcyBub3QgY2FsbCBXVEZNb3ZlKCkg bXVsdGlwbGUgdGltZXMKKyAgICAgICAgb24gdGhlIHNhbWUgY29tcGxldGlvbiBoYW5kbGVyLiBV c2UgYSBSZWZDb3VudGVyIHRvIGhvbGQgb24gdG8gdGhlIGNvbXBsZXRpb24gaGFuZGxlcgorICAg ICAgICBhbmQgbWFrZSBzdXJlIHRoZSBoYW5kbGVyIGdldHMgY2FsbGVkIHdoZW4gdGhlIFJlZkNv dW50ZXIncyB2YWx1ZSBiZWNvbWVzIDAuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlByb2Nl c3MuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJQcm9jZXNzOjpjYW5jZWxQcmVwYXJlVG9TdXNw ZW5kKToKKyAgICAgICAgKFdlYktpdDo6V2ViUHJvY2Vzczo6bWFya0FsbExheWVyc1ZvbGF0aWxl KToKKyAgICAgICAgKFdlYktpdDo6V2ViUHJvY2Vzczo6Y2FuY2VsTWFya0FsbExheWVyc1ZvbGF0 aWxlKToKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlByb2Nlc3MuaDoKKworMjAxNy0wOC0xOCAg Q2hyaXMgRHVtZXogIDxjZHVtZXpAYXBwbGUuY29tPgorCiAgICAgICAgIFtCZWFjb25dIEFkZCBz dXBwb3J0IGZvciBxdW90YSBsaW1pdGF0aW9uCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xNzU0NDMKICAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzMzNzI5 MDAyPgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1dlYlByb2Nlc3MuY3Bw IGIvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1dlYlByb2Nlc3MuY3BwCmluZGV4IDU0ZDMxYzUz NDUwZTQwZTNmNWY3NTUxYTA4MzY2NTI2MzRlMjM1ZjEuLjBhNGVmOGJjYzlkYTZhYTI5MDA0OTg2 Zjg0MjRlNDczMDRiNzI1MmUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvV2ViUHJvY2Vzcy9X ZWJQcm9jZXNzLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nlc3MvV2ViUHJvY2Vzcy5j cHAKQEAgLTEzNzIsNyArMTM3Miw3IEBAIHZvaWQgV2ViUHJvY2Vzczo6Y2FuY2VsUHJlcGFyZVRv U3VzcGVuZCgpCiAKICAgICAvLyBJZiB3ZSd2ZSBhbHJlYWR5IGZpbmlzaGVkIGNsZWFuaW5nIHVw IGFuZCBzZW50IFByb2Nlc3NSZWFkeVRvU3VzcGVuZCwgd2UKICAgICAvLyBzaG91bGRuJ3Qgc2Vu ZCBEaWRDYW5jZWxQcm9jZXNzU3VzcGVuc2lvbjsgdGhlIFVJIHByb2Nlc3Mgc3RyaWN0bHkgZXhw ZWN0cyBvbmUgb3IgdGhlIG90aGVyLgotICAgIGlmICghbV9wYWdlc01hcmtpbmdMYXllcnNBc1Zv bGF0aWxlKQorICAgIGlmICghbV9wYWdlTWFya2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyKQog ICAgICAgICByZXR1cm47CiAKICAgICBjYW5jZWxNYXJrQWxsTGF5ZXJzVm9sYXRpbGUoKTsKQEAg LTEzODQsMjggKzEzODQsMjcgQEAgdm9pZCBXZWJQcm9jZXNzOjpjYW5jZWxQcmVwYXJlVG9TdXNw ZW5kKCkKIHZvaWQgV2ViUHJvY2Vzczo6bWFya0FsbExheWVyc1ZvbGF0aWxlKFdURjo6RnVuY3Rp b248dm9pZCgpPiYmIGNvbXBsZXRpb25IYW5kbGVyKQogewogICAgIFJFTEVBU0VfTE9HKFByb2Nl c3NTdXNwZW5zaW9uLCAiJXAgLSBXZWJQcm9jZXNzOjptYXJrQWxsTGF5ZXJzVm9sYXRpbGUoKSIs IHRoaXMpOwotICAgIG1fcGFnZXNNYXJraW5nTGF5ZXJzQXNWb2xhdGlsZSA9IG1fcGFnZU1hcC5z aXplKCk7Ci0gICAgaWYgKCFtX3BhZ2VzTWFya2luZ0xheWVyc0FzVm9sYXRpbGUpIHsKKyAgICBB U1NFUlQoIW1fcGFnZU1hcmtpbmdMYXllcnNBc1ZvbGF0aWxlQ291bnRlcik7CisgICAgbV9wYWdl TWFya2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyID0gc3RkOjptYWtlX3VuaXF1ZTxQYWdlTWFy a2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyPihbdGhpcywgY29tcGxldGlvbkhhbmRsZXIgPSBX VEZNb3ZlKGNvbXBsZXRpb25IYW5kbGVyKV0gKFJlZkNvdW50ZXJFdmVudCkgeworICAgICAgICBp ZiAobV9wYWdlTWFya2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyLT52YWx1ZSgpKQorICAgICAg ICAgICAgcmV0dXJuOworCiAgICAgICAgIGNvbXBsZXRpb25IYW5kbGVyKCk7Ci0gICAgICAgIHJl dHVybjsKLSAgICB9Ci0gICAgZm9yIChhdXRvJiBwYWdlIDogbV9wYWdlTWFwLnZhbHVlcygpKSB7 Ci0gICAgICAgIHBhZ2UtPm1hcmtMYXllcnNWb2xhdGlsZShbdGhpcywgY29tcGxldGlvbkhhbmRs ZXIgPSBXVEZNb3ZlKGNvbXBsZXRpb25IYW5kbGVyKV0gewotICAgICAgICAgICAgQVNTRVJUKG1f cGFnZXNNYXJraW5nTGF5ZXJzQXNWb2xhdGlsZSk7Ci0gICAgICAgICAgICBpZiAoIS0tbV9wYWdl c01hcmtpbmdMYXllcnNBc1ZvbGF0aWxlKQotICAgICAgICAgICAgICAgIGNvbXBsZXRpb25IYW5k bGVyKCk7Ci0gICAgICAgIH0pOwotICAgIH0KKyAgICAgICAgbV9wYWdlTWFya2luZ0xheWVyc0Fz Vm9sYXRpbGVDb3VudGVyID0gbnVsbHB0cjsKKyAgICB9KTsKKyAgICBhdXRvIHRva2VuID0gbV9w YWdlTWFya2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyLT5jb3VudCgpOworICAgIGZvciAoYXV0 byYgcGFnZSA6IG1fcGFnZU1hcC52YWx1ZXMoKSkKKyAgICAgICAgcGFnZS0+bWFya0xheWVyc1Zv bGF0aWxlKFt0b2tlbl0geyB9KTsKIH0KIAogdm9pZCBXZWJQcm9jZXNzOjpjYW5jZWxNYXJrQWxs TGF5ZXJzVm9sYXRpbGUoKQogewotICAgIGlmICghbV9wYWdlc01hcmtpbmdMYXllcnNBc1ZvbGF0 aWxlKQorICAgIGlmICghbV9wYWdlTWFya2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyKQogICAg ICAgICByZXR1cm47CiAKKyAgICBtX3BhZ2VNYXJraW5nTGF5ZXJzQXNWb2xhdGlsZUNvdW50ZXIg PSBudWxscHRyOwogICAgIGZvciAoYXV0byYgcGFnZSA6IG1fcGFnZU1hcC52YWx1ZXMoKSkKICAg ICAgICAgcGFnZS0+Y2FuY2VsTWFya0xheWVyc1ZvbGF0aWxlKCk7Ci0gICAgbV9wYWdlc01hcmtp bmdMYXllcnNBc1ZvbGF0aWxlID0gMDsKIH0KIAogdm9pZCBXZWJQcm9jZXNzOjpzZXRBbGxMYXll clRyZWVTdGF0ZXNGcm96ZW4oYm9vbCBmcm96ZW4pCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0 L1dlYlByb2Nlc3MvV2ViUHJvY2Vzcy5oIGIvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1dlYlBy b2Nlc3MuaAppbmRleCAxMGZlZDNlMzJmNTBmOTJlMjliZmExNTMyM2YyMTFkZTQzM2Q0ZThiLi5i ZWE2Y2RiMmMwNjM3YzE2NWQ3ZTA4MzM2MmY3YTFhNWRjMDA1MWFjIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViS2l0L1dlYlByb2Nlc3MvV2ViUHJvY2Vzcy5oCisrKyBiL1NvdXJjZS9XZWJLaXQvV2Vi UHJvY2Vzcy9XZWJQcm9jZXNzLmgKQEAgLTQwLDYgKzQwLDcgQEAKICNpbmNsdWRlIDx3dGYvRm9y d2FyZC5oPgogI2luY2x1ZGUgPHd0Zi9IYXNoTWFwLmg+CiAjaW5jbHVkZSA8d3RmL0hhc2hTZXQu aD4KKyNpbmNsdWRlIDx3dGYvUmVmQ291bnRlci5oPgogI2luY2x1ZGUgPHd0Zi90ZXh0L0F0b21p Y1N0cmluZy5oPgogI2luY2x1ZGUgPHd0Zi90ZXh0L0F0b21pY1N0cmluZ0hhc2guaD4KIApAQCAt NDE0LDcgKzQxNSwxMCBAQCBwcml2YXRlOgogICAgIFdlYlNRTGl0ZURhdGFiYXNlVHJhY2tlciBt X3dlYlNRTGl0ZURhdGFiYXNlVHJhY2tlcjsKICNlbmRpZgogCi0gICAgdW5zaWduZWQgbV9wYWdl c01hcmtpbmdMYXllcnNBc1ZvbGF0aWxlIHsgMCB9OworICAgIGVudW0gUGFnZU1hcmtpbmdMYXll cnNBc1ZvbGF0aWxlQ291bnRlclR5cGUgeyB9OworICAgIHVzaW5nIFBhZ2VNYXJraW5nTGF5ZXJz QXNWb2xhdGlsZUNvdW50ZXIgPSBSZWZDb3VudGVyPFBhZ2VNYXJraW5nTGF5ZXJzQXNWb2xhdGls ZUNvdW50ZXJUeXBlPjsKKyAgICBzdGQ6OnVuaXF1ZV9wdHI8UGFnZU1hcmtpbmdMYXllcnNBc1Zv bGF0aWxlQ291bnRlcj4gbV9wYWdlTWFya2luZ0xheWVyc0FzVm9sYXRpbGVDb3VudGVyOworCiAg ICAgYm9vbCBtX3N1cHByZXNzTWVtb3J5UHJlc3N1cmVIYW5kbGVyIHsgZmFsc2UgfTsKICNpZiBQ TEFURk9STShNQUMpCiAgICAgc3RkOjp1bmlxdWVfcHRyPFdlYkNvcmU6OkNQVU1vbml0b3I+IG1f Y3B1TW9uaXRvcjsK