184445 2018-04-10 04:08:58 -0700 [GTK][WPE] Race condition when destroying webprocesses 2018-04-16 11:34:35 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=183348 P2 Normal --- 1 magomez magomez bugs-noreply cgarcia commit-queue mcatanzaro oldest_to_newest 1413249 0 magomez 2018-04-10 04:08:58 -0700 When a WebPageProxy gets destroyed, this is that ideally happens: * WebPageProxy::close() is called: * Sends a WebPage::Close() message to the webprocess, so it can properly close its page before shutting down the process * Calls WebProcessProxy::removeWebPage() which, in the end, closes the IPC connection to the webprocess * At the same time, when the webprocess executes WebPage::close() in response to the WebPage::Close() message * releases the page resources * at the end calls WebProcess::removeWebPage(), which causes the web process termination But there's the possibility that the webprocess never processes the WebPage::Close() message. This happens when it detects that the IPC connection has been closed before processing the message. In that case, the webprocess exits without properly closing its webpages, causing problems on situations where a proper deinitialization is required (for example deinitializing a hardware decoder used by the media player). We need to ensure that the WebPage instances owned by the WebProcess are always properly closed before exiting. 1413253 1 magomez 2018-04-10 05:15:15 -0700 One option to fix this could be making the WebPage::Close() message a sync one, and make the ui process wait for the web process to properly close before shutting down, but this would delay closing the browser and that is a problem for the Apple ports. So, if we cannot wait for the web process to end, we should ensure that it will close its WebPages on exit. The easiest way is to use the code that does that on debug inside WebProcess::didClose(). 1413255 2 cgarcia 2018-04-10 05:37:49 -0700 (In reply to Miguel Gomez from comment #1) > One option to fix this could be making the WebPage::Close() message a sync > one, and make the ui process wait for the web process to properly close > before shutting down, but this would delay closing the browser and that is a > problem for the Apple ports. > > So, if we cannot wait for the web process to end, we should ensure that it > will close its WebPages on exit. The easiest way is to use the code that > does that on debug inside WebProcess::didClose(). Yes, I would move that part out of the debug ifdef, at least for GTK and WPE. 1413256 3 337601 magomez 2018-04-10 06:23:32 -0700 Created attachment 337601 Patch 1413261 4 337603 magomez 2018-04-10 06:53:15 -0700 Created attachment 337603 Patch 1413269 5 337603 cgarcia 2018-04-10 08:11:04 -0700 Comment on attachment 337603 Patch Ok, let's try with this. 1413285 6 337603 commit-queue 2018-04-10 09:26:47 -0700 Comment on attachment 337603 Patch Clearing flags on attachment: 337603 Committed r230481: <https://trac.webkit.org/changeset/230481> 1413286 7 commit-queue 2018-04-10 09:26:48 -0700 All reviewed patches have been landed. Closing bug. 1413320 8 mcatanzaro 2018-04-10 10:44:08 -0700 IMO this is wrong, this code should not be platform-specific. If it's not good enough for Apple, why should it be good enough for us? The web process is *expected* to crash, and WebKit is designed around that expectation. It must not be expected to release any system resources, or your application is going to be in big trouble when it does crash. Sounds like you need to find some other solution to handle these hardware media decoders, if they are not robust to process crashes. (In reply to Miguel Gomez from comment #1) > One option to fix this could be making the WebPage::Close() message a sync > one, and make the ui process wait for the web process to properly close > before shutting down, but this would delay closing the browser and that is a > problem for the Apple ports. That would be a problem for us too, because we know that in practice the web process often hangs on exit. So let's not try that, either. 337601 2018-04-10 06:23:32 -0700 2018-04-10 06:53:11 -0700 Patch bug-184445-20180410152331.patch text/plain 1571 magomez U3VidmVyc2lvbiBSZXZpc2lvbjogMjMwNDc5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDFiYjgxMGMyOGRkMWQyYzA0 ZjJmZjBlNTA1MjhhZmIyYzA4YzgxMWIuLjdjMzY5NjA0ZWFjOTU0YzI0ZTkwOTdmNDU3ZTAwMWRh NTYxZDNlNGQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTgtMDQtMTAgIE1pZ3VlbCBH b21leiAgPG1hZ29tZXpAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXVtXUEVdIFJhY2UgY29u ZGl0aW9uIHdoZW4gZGVzdHJveWluZyB3ZWJwcm9jZXNzZXMKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4NDQ0NQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEVuc3VyZSB0aGF0IHRoZSBXZWJQcm9jZXNzIGlz IHByb3Blcmx5IGNsb3NpbmcgaXRzIHBhZ2VzIHdoZW4gaXQncyBleGl0aW5nIGJlY2F1c2UKKyAg ICAgICAgdGhlIFVJUHJvY2VzcyBoYXMgaW52YWxpZGF0ZWQgdGhlIElQQyBjb25uZWN0aW9uLgor CisgICAgICAgICogV2ViUHJvY2Vzcy9XZWJQcm9jZXNzLmNwcDoKKyAgICAgICAgKFdlYktpdDo6 V2ViUHJvY2Vzczo6ZGlkQ2xvc2UpOgorCiAyMDE4LTA0LTA5ICBXZW5zb24gSHNpZWggIDx3ZW5z b25faHNpZWhAYXBwbGUuY29tPgogCiAgICAgICAgIEFkZCBtaXNzaW5nIGF2YWlsYWJpbGl0eSBt YWNyb3MgYWZ0ZXIgcjIzMDQ2MgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9XZWJQcm9jZXNz L1dlYlByb2Nlc3MuY3BwIGIvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1dlYlByb2Nlc3MuY3Bw CmluZGV4IDRjY2M1ZTFmYTA5NzM5OTIyMzBlMDQ1YWE2MTczNmYwYmE2NGY1YjYuLjc1MTNlYjU4 YzkwMjY1NDU0OWEwMDllYzU5NTRhMWFlMTNmNDUwNTcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvV2ViUHJvY2Vzcy9XZWJQcm9jZXNzLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nl c3MvV2ViUHJvY2Vzcy5jcHAKQEAgLTY4NiwxMCArNjg2LDEyIEBAIHZvaWQgV2ViUHJvY2Vzczo6 ZGlkUmVjZWl2ZU1lc3NhZ2UoSVBDOjpDb25uZWN0aW9uJiBjb25uZWN0aW9uLCBJUEM6OkRlY29k ZXImIGRlCiAKIHZvaWQgV2ViUHJvY2Vzczo6ZGlkQ2xvc2UoSVBDOjpDb25uZWN0aW9uJikKIHsK LSNpZm5kZWYgTkRFQlVHCisjaWZuZGVmIE5ERUJVRyB8fCBQTEFURk9STShHVEspIHx8IFBMQVRG T1JNKFdQRSkKICAgICBmb3IgKGF1dG8mIHBhZ2UgOiBjb3B5VG9WZWN0b3IobV9wYWdlTWFwLnZh bHVlcygpKSkKICAgICAgICAgcGFnZS0+Y2xvc2UoKTsKKyNlbmRpZgogCisjaWZuZGVmIE5ERUJV RwogICAgIEdDQ29udHJvbGxlcjo6c2luZ2xldG9uKCkuZ2FyYmFnZUNvbGxlY3RTb29uKCk7CiAg ICAgRm9udENhY2hlOjpzaW5nbGV0b24oKS5pbnZhbGlkYXRlKCk7CiAgICAgTWVtb3J5Q2FjaGU6 OnNpbmdsZXRvbigpLnNldERpc2FibGVkKHRydWUpOwo= 337603 2018-04-10 06:53:15 -0700 2018-04-10 09:26:47 -0700 Patch bug-184445-20180410155313.patch text/plain 1577 magomez U3VidmVyc2lvbiBSZXZpc2lvbjogMjMwNDc5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDFiYjgxMGMyOGRkMWQyYzA0 ZjJmZjBlNTA1MjhhZmIyYzA4YzgxMWIuLjdjMzY5NjA0ZWFjOTU0YzI0ZTkwOTdmNDU3ZTAwMWRh NTYxZDNlNGQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTgtMDQtMTAgIE1pZ3VlbCBH b21leiAgPG1hZ29tZXpAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1RLXVtXUEVdIFJhY2UgY29u ZGl0aW9uIHdoZW4gZGVzdHJveWluZyB3ZWJwcm9jZXNzZXMKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4NDQ0NQorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEVuc3VyZSB0aGF0IHRoZSBXZWJQcm9jZXNzIGlz IHByb3Blcmx5IGNsb3NpbmcgaXRzIHBhZ2VzIHdoZW4gaXQncyBleGl0aW5nIGJlY2F1c2UKKyAg ICAgICAgdGhlIFVJUHJvY2VzcyBoYXMgaW52YWxpZGF0ZWQgdGhlIElQQyBjb25uZWN0aW9uLgor CisgICAgICAgICogV2ViUHJvY2Vzcy9XZWJQcm9jZXNzLmNwcDoKKyAgICAgICAgKFdlYktpdDo6 V2ViUHJvY2Vzczo6ZGlkQ2xvc2UpOgorCiAyMDE4LTA0LTA5ICBXZW5zb24gSHNpZWggIDx3ZW5z b25faHNpZWhAYXBwbGUuY29tPgogCiAgICAgICAgIEFkZCBtaXNzaW5nIGF2YWlsYWJpbGl0eSBt YWNyb3MgYWZ0ZXIgcjIzMDQ2MgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9XZWJQcm9jZXNz L1dlYlByb2Nlc3MuY3BwIGIvU291cmNlL1dlYktpdC9XZWJQcm9jZXNzL1dlYlByb2Nlc3MuY3Bw CmluZGV4IDRjY2M1ZTFmYTA5NzM5OTIyMzBlMDQ1YWE2MTczNmYwYmE2NGY1YjYuLjg3YWI3MzZi ODc3MTkyNjA5ZjIyMzVkYWEzYjQ0ZTdmYzBjNzhlM2YgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvV2ViUHJvY2Vzcy9XZWJQcm9jZXNzLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1dlYlByb2Nl c3MvV2ViUHJvY2Vzcy5jcHAKQEAgLTY4NiwxMCArNjg2LDEyIEBAIHZvaWQgV2ViUHJvY2Vzczo6 ZGlkUmVjZWl2ZU1lc3NhZ2UoSVBDOjpDb25uZWN0aW9uJiBjb25uZWN0aW9uLCBJUEM6OkRlY29k ZXImIGRlCiAKIHZvaWQgV2ViUHJvY2Vzczo6ZGlkQ2xvc2UoSVBDOjpDb25uZWN0aW9uJikKIHsK LSNpZm5kZWYgTkRFQlVHCisjaWYgIWRlZmluZWQoTkRFQlVHKSB8fCBQTEFURk9STShHVEspIHx8 IFBMQVRGT1JNKFdQRSkKICAgICBmb3IgKGF1dG8mIHBhZ2UgOiBjb3B5VG9WZWN0b3IobV9wYWdl TWFwLnZhbHVlcygpKSkKICAgICAgICAgcGFnZS0+Y2xvc2UoKTsKKyNlbmRpZgogCisjaWZuZGVm IE5ERUJVRwogICAgIEdDQ29udHJvbGxlcjo6c2luZ2xldG9uKCkuZ2FyYmFnZUNvbGxlY3RTb29u KCk7CiAgICAgRm9udENhY2hlOjpzaW5nbGV0b24oKS5pbnZhbGlkYXRlKCk7CiAgICAgTWVtb3J5 Q2FjaGU6OnNpbmdsZXRvbigpLnNldERpc2FibGVkKHRydWUpOwo=