Bug 188880 - Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated
Summary: Assert in NetworkBlobRegistry::unregisterBlobURL after network process had te...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 188426
  Show dependency treegraph
 
Reported: 2018-08-22 22:55 PDT by Ryosuke Niwa
Modified: 2018-08-23 17:44 PDT (History)
8 users (show)

See Also:


Attachments
Removes the assertion (8.96 KB, patch)
2018-08-22 23:01 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Removes the assertion (8.96 KB, patch)
2018-08-22 23:07 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.44 MB, application/zip)
2018-08-22 23:57 PDT, EWS Watchlist
no flags Details
Patch for landing (10.37 KB, patch)
2018-08-23 00:04 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2018-08-22 22:55:25 PDT
When a network process terminates, it obviously loses track of the registered blobs.
When WebContent process then tries to unregister a blob, we hit a debug assertion in NetworkBlobRegistry::unregisterBlobURL.
Comment 1 Ryosuke Niwa 2018-08-22 23:01:07 PDT
Created attachment 347902 [details]
Removes the assertion
Comment 2 EWS Watchlist 2018-08-22 23:03:03 PDT
Attachment 347902 [details] did not pass style-queue:


ERROR: LayoutTests/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ryosuke Niwa 2018-08-22 23:07:10 PDT
Created attachment 347903 [details]
Removes the assertion
Comment 4 Saam Barati 2018-08-22 23:38:43 PDT
Comment on attachment 347903 [details]
Removes the assertion

r=me
Comment 5 EWS Watchlist 2018-08-22 23:57:10 PDT
Comment on attachment 347903 [details]
Removes the assertion

Attachment 347903 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8954594

New failing tests:
fast/files/blob-network-process-crash.html
Comment 6 EWS Watchlist 2018-08-22 23:57:12 PDT
Created attachment 347911 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 Ryosuke Niwa 2018-08-22 23:59:58 PDT
Oh oops, clearly we have to skip this test in Wk1.
Comment 8 Ryosuke Niwa 2018-08-23 00:04:13 PDT
Created attachment 347913 [details]
Patch for landing
Comment 9 Ryosuke Niwa 2018-08-23 00:04:25 PDT
Comment on attachment 347913 [details]
Patch for landing

Waiting for EWS
Comment 10 EWS Watchlist 2018-08-23 00:06:45 PDT
Attachment 347913 [details] did not pass style-queue:


ERROR: LayoutTests/platform/wk2/TestExpectations:744:  expecting "[", "#", or end of line instead of "Pass"  [test/expectations] [5]
ERROR: LayoutTests/platform/wk2/TestExpectations:744:  Path does not exist.  [test/expectations] [5]
ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/wk2/TestExpectations:744:  expecting "[", "#", or end of line instead of "Pass"  [test/expectations] [5]
ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/wk2/TestExpectations:744:  Path does not exist.  [test/expectations] [5]
Total errors found: 4 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Alex Christensen 2018-08-23 10:15:31 PDT
Comment on attachment 347913 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=347913&action=review

> LayoutTests/TestExpectations:393
> +# testRunner.terminateNetworkProcess() only works in WebKit2
> +fast/files/blob-network-process-crash.html [ Skip ]

We could've just put if (testRunner.terminateNetworkProcess) instead of this.
Comment 12 Ryosuke Niwa 2018-08-23 13:49:29 PDT
(In reply to Alex Christensen from comment #11)
> Comment on attachment 347913 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347913&action=review
> 
> > LayoutTests/TestExpectations:393
> > +# testRunner.terminateNetworkProcess() only works in WebKit2
> > +fast/files/blob-network-process-crash.html [ Skip ]
> 
> We could've just put if (testRunner.terminateNetworkProcess) instead of this.

But then the test doesn't do anything useful.
Comment 13 Ryosuke Niwa 2018-08-23 13:49:54 PDT
(In reply to Ryosuke Niwa from comment #12)
> (In reply to Alex Christensen from comment #11)
> > Comment on attachment 347913 [details]
> > Patch for landing
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=347913&action=review
> > 
> > > LayoutTests/TestExpectations:393
> > > +# testRunner.terminateNetworkProcess() only works in WebKit2
> > > +fast/files/blob-network-process-crash.html [ Skip ]
> > 
> > We could've just put if (testRunner.terminateNetworkProcess) instead of this.
> 
> But then the test doesn't do anything useful.

That is, since this test isn't useful in DRT, there is no point in running it there.
Comment 14 Ryosuke Niwa 2018-08-23 13:51:22 PDT
Committed r235243: <https://trac.webkit.org/changeset/235243>
Comment 15 Radar WebKit Bug Importer 2018-08-23 13:52:19 PDT
<rdar://problem/43656323>
Comment 16 Ryan Haddad 2018-08-23 16:02:53 PDT
The layout test added with this change is timing out on the bots:

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r235245%20(6350)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ffiles%2Fblob-network-process-crash.html

--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/files/blob-network-process-crash-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/files/blob-network-process-crash-actual.txt
@@ -1,2 +1,6 @@
-This tests unregistering a Blob object after the network process to which it was registered crashed.
-WebKit should not hit a debug assertion in the network process.
+CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: WebKit encountered an internal error
+#PID UNRESPONSIVE - com.apple.WebKit.WebContent.Development (pid 38955)
+FAIL: Timed out waiting for notifyDone to be called
+
+#EOF
+#EOF
Comment 17 Ryosuke Niwa 2018-08-23 16:59:17 PDT
Investigating...
Comment 18 Ryosuke Niwa 2018-08-23 17:44:08 PDT
This is super weird. Timer sometimes never fires in this test.
Tracking this bug in https://bugs.webkit.org/show_bug.cgi?id=188911.

Temporarily worked around the issue in <https://trac.webkit.org/changeset/235264 to unflake the test.