181771 2018-01-17 16:36:49 -0800 REGRESSION (r223149): WebProcessProxy::didClose() no longer refs WebPageProxy objects 2018-01-22 12:20:08 -0800 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=181863 InRadar, Regression P2 Normal --- 178102 1 cdumez cdumez beidson commit-queue darin dbates ggaren sam webkit-bug-importer oldest_to_newest 1390285 0 cdumez 2018-01-17 16:36:49 -0800 WebProcessProxy::didClose() no longer refs WebPageProxy objects, leading to crashes: Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000200000353) [ 0] 0x00000001945ccf58 WebKit`WebKit::WebPageProxy::processDidTerminate(WebKit::ProcessTerminationReason) [inlined] WebKit::WebProcessProxy::isUnderMemoryPressure() const at WebProcessProxy.h:184:49 0x00000001945ccf48: add x29, sp, #0x60 ; =0x60 0x00000001945ccf4c: mov x20, x1 0x00000001945ccf50: mov x19, x0 0x00000001945ccf54: ldr x8, [x19, #0xc0] -> 0x00000001945ccf58: ldrb w8, [x8, #0x2d0] 0x00000001945ccf5c: cbz w8, 0x23d0a0 ; <+356> at WebPageProxy.cpp:5523 0x00000001945ccf60: add x8, sp, #0x8 ; =0x8 0x00000001945ccf64: mov x0, x19 0x00000001945ccf68: bl 0x22e304 ; WebKit::WebPageProxy::currentURL at WebPageProxy.cpp:5503 [ 0] 0x00000001945ccf58 WebKit`WebKit::WebPageProxy::processDidTerminate(WebKit::ProcessTerminationReason) + 28 at WebPageProxy.cpp:5515 [ 1] 0x000000019464062b WebKit`WebKit::WebProcessProxy::didClose(IPC::Connection&) + 523 at WebProcessProxy.cpp:643:15 [ 2] 0x000000019464062b WebKit`WebKit::WebProcessProxy::didClose(IPC::Connection&) + 523 at WebProcessProxy.cpp:643:15 [ 3] 0x000000018c179dc7 JavaScriptCore`WTF::RunLoop::performWork() [inlined] WTF::Function<void ()>::operator()() const + 15 at Function.h:56:35 [ 3] 0x000000018c179db8 JavaScriptCore`WTF::RunLoop::performWork() + 252 at RunLoop.cpp:106 [ 4] 0x000000018c17a087 JavaScriptCore`WTF::RunLoop::performWork(void*) + 35 at RunLoopCF.cpp:38:37 [ 5] 0x000000018435ca23 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 23 at CFRunLoop.c:1982:9 [ 6] 0x000000018435c24b CoreFoundation`__CFRunLoopDoSources0 [inlined] __CFRunLoopDoSource0 + 67 at CFRunLoop.c:2017:13 [ 6] 0x000000018435c208 CoreFoundation`__CFRunLoopDoSources0 + 208 at CFRunLoop.c:2053 [ 7] 0x0000000184359dbb CoreFoundation`__CFRunLoopRun + 1203 at CFRunLoop.c:2920:41 [ 8] 0x000000018427a5d7 CoreFoundation`CFRunLoopRunSpecific + 551 at CFRunLoop.c:3245:18 [ 9] 0x000000018623b023 GraphicsServices`GSEventRunModal + 99 at GSEvent.c:2245:9 [ 10] 0x000000018e705a63 UIKit`UIApplicationMain + 235 at UIApplication.m:3956:5 [ 11] 0x000000010086e90b MobileSafari`main + 1927 [ 12] 0x0000000183d19faf libdyld.dylib`start + 3 1390286 1 cdumez 2018-01-17 16:37:02 -0800 <rdar://problem/36566237> 1390288 2 331559 cdumez 2018-01-17 16:41:15 -0800 Created attachment 331559 Patch 1390334 3 beidson 2018-01-17 18:41:39 -0800 "...no longer refs WebPageProxy objects..." This means it used to. Why did it stop doing so? 1390347 4 cdumez 2018-01-17 19:01:43 -0800 (In reply to Brady Eidson from comment #3) > "...no longer refs WebPageProxy objects..." > > This means it used to. > > Why did it stop doing so? Sorry, the explanation was only in the radar. In WebProcessProxy::didClose(): Vector<RefPtr<WebPageProxy>> pages; copyValuesToVector(m_pageMap, pages); became (in r223149) auto pages = copyToVector(m_pageMap.values()); “auto” here resolves to Vector<WebPageProxy*> so we no longer ref the pages. This is because m_pages is of type HashMap<uint64_t, WebPageProxy*>. 1390558 5 cdumez 2018-01-18 10:19:28 -0800 For the record, I tried writing an API test. The issue is that I cannot get the view to get destroyed in the processDidTerminate delegates. This is because all API tests run in an autorelease pool. Even though I release the webviews in the delegate, they only get deallocated later, at the end of the test, when the pool is drained. 1390622 6 331559 cdumez 2018-01-18 12:11:03 -0800 Comment on attachment 331559 Patch Clearing flags on attachment: 331559 Committed r227157: <https://trac.webkit.org/changeset/227157> 1390623 7 cdumez 2018-01-18 12:11:05 -0800 All reviewed patches have been landed. Closing bug. 1391559 8 darin 2018-01-21 23:40:32 -0800 Did we audit all the other loops we changed to see if any others need this sort of fix to restore the old behavior? 1391671 9 cdumez 2018-01-22 09:30:38 -0800 (In reply to Darin Adler from comment #8) > Did we audit all the other loops we changed to see if any others need this > sort of fix to restore the old behavior? Daniel Bates landed a follow-up patch in https://bugs.webkit.org/show_bug.cgi?id=178102. 1391727 10 dbates 2018-01-22 11:24:16 -0800 (In reply to Chris Dumez from comment #9) > (In reply to Darin Adler from comment #8) > > Did we audit all the other loops we changed to see if any others need this > > sort of fix to restore the old behavior? > > Daniel Bates landed a follow-up patch in > https://bugs.webkit.org/show_bug.cgi?id=178102. I take it you meant to write that the follow up patch is on bug #181863. 1391738 11 cdumez 2018-01-22 12:20:08 -0800 (In reply to Daniel Bates from comment #10) > (In reply to Chris Dumez from comment #9) > > (In reply to Darin Adler from comment #8) > > > Did we audit all the other loops we changed to see if any others need this > > > sort of fix to restore the old behavior? > > > > Daniel Bates landed a follow-up patch in > > https://bugs.webkit.org/show_bug.cgi?id=178102. > > I take it you meant to write that the follow up patch is on bug #181863. Yes indeed. 331559 2018-01-17 16:41:15 -0800 2018-01-18 12:11:03 -0800 Patch bug-181771-20180117164115.patch text/plain 1420 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjI3MDg3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGE2NTAxYWJmNTNlYTM3MTlm ODcxZDgyOTg3YzdjYzc1NTI0YWE1ZjIuLmJkMzhmODRlMTE0Nzg0MzgxYWM1Zjc5ODNkMjlhZjY1 MWRmNjRhNTYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTgtMDEtMTcgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBSZWdyZXNzaW9uKHIyMjMxNDkpOiBX ZWJQcm9jZXNzUHJveHk6OmRpZENsb3NlKCkgbm8gbG9uZ2VyIHJlZnMgV2ViUGFnZVByb3h5IG9i amVjdHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4 MTc3MQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMzY1NjYyMzc+CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViUHJvY2Vzc1By b3h5LmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUHJvY2Vzc1Byb3h5OjpkaWRDbG9zZSk6Cisg ICAgICAgIFVzZSBjb3B5VG9WZWN0b3JPZjxSZWZQdHI8V2ViUGFnZVByb3h5Pj4oKSB0byBtYWlu dGFpbiBwcmUtcjIyMzE0OSBiZWhhdmlvcgorICAgICAgICBhbmQgcmVmIHRoZSBwYWdlcy4KKwog MjAxOC0wMS0xNyAgQ2hyaXMgRHVtZXogIDxjZHVtZXpAYXBwbGUuY29tPgogCiAgICAgICAgIFdl IHNob3VsZCBiZSBhYmxlIHRvIHRlcm1pbmF0ZSBzZXJ2aWNlIHdvcmtlcnMgdGhhdCBhcmUgdW5y ZXNwb25zaXZlCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9XZWJQcm9jZXNz UHJveHkuY3BwIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvV2ViUHJvY2Vzc1Byb3h5LmNwcApp bmRleCBjMzU1NGFjNGIzNDIyZDIyOTJjOGNhN2U1YmUwNDliYzM1ZTk5YWI1Li4zYTI3MWJlY2Yy NWY3MWZkYzE2ZDYzZDcxZTQ0ODZmMzc5NDFjZTU1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 L1VJUHJvY2Vzcy9XZWJQcm9jZXNzUHJveHkuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvVUlQcm9j ZXNzL1dlYlByb2Nlc3NQcm94eS5jcHAKQEAgLTYyOCw3ICs2MjgsNyBAQCB2b2lkIFdlYlByb2Nl c3NQcm94eTo6ZGlkQ2xvc2UoSVBDOjpDb25uZWN0aW9uJikKIAogICAgIHdlYkNvbm5lY3Rpb24o KS0+ZGlkQ2xvc2UoKTsKIAotICAgIGF1dG8gcGFnZXMgPSBjb3B5VG9WZWN0b3IobV9wYWdlTWFw LnZhbHVlcygpKTsKKyAgICBhdXRvIHBhZ2VzID0gY29weVRvVmVjdG9yT2Y8UmVmUHRyPFdlYlBh Z2VQcm94eT4+KG1fcGFnZU1hcC52YWx1ZXMoKSk7CiAKICAgICBzaHV0RG93bigpOwogCg==