Summary: | [GTK][WPE] Race condition when destroying webprocesses | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Miguel Gomez <magomez> | ||||||
Component: | WebKitGTK | Assignee: | Miguel Gomez <magomez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bugs-noreply, cgarcia, commit-queue, mcatanzaro | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=183348 | ||||||||
Attachments: |
|
Description
Miguel Gomez
2018-04-10 04:08:58 PDT
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(). (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. Created attachment 337601 [details]
Patch
Created attachment 337603 [details]
Patch
Comment on attachment 337603 [details]
Patch
Ok, let's try with this.
Comment on attachment 337603 [details] Patch Clearing flags on attachment: 337603 Committed r230481: <https://trac.webkit.org/changeset/230481> All reviewed patches have been landed. Closing bug. 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. |