Bug 192486

Summary: Consecutive DumpRenderTree crashes are happening again on WinCairo BuildBots since r238903
Product: WebKit Reporter: Fujii Hironori <fujii>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, glenn, jbedard, lforschler, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=192161
https://bugs.webkit.org/show_bug.cgi?id=188036
Attachments:
Description Flags
Original Behavior
none
Possible Improvement
none
Patch for landing none

Fujii Hironori
Reported 2018-12-06 18:28:27 PST
Consecutive DumpRenderTree crashes are happening again on WinCairo BuildBots since r238903 WinCairo BuildBots is using WEBKIT_TEST_CHILD_PROCESSES to avoid Bug 192161. But, WEBKIT_TEST_CHILD_PROCESSES was removed in Bug 192161.
Attachments
Original Behavior (1.31 KB, patch)
2018-12-06 19:50 PST, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Possible Improvement (1.25 KB, patch)
2018-12-06 19:54 PST, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.42 KB, patch)
2018-12-06 21:28 PST, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2018-12-06 18:30:17 PST
(In reply to Fujii Hironori from comment #0) > WinCairo BuildBots is using WEBKIT_TEST_CHILD_PROCESSES to avoid Bug 192161. Oops. Wrong bug id. I meant this: Bug 188036 – Consecutive DumpRenderTree crash on WinCairo BuildBots
Jonathan Bedard
Comment 2 2018-12-06 19:38:35 PST
Didn't see WEBKIT_TEST_CHILD_PROCESSES being used anywhere, apparently didn't look hard enough. This is an easy enough fix.
Jonathan Bedard
Comment 3 2018-12-06 19:50:55 PST
Created attachment 356780 [details] Original Behavior
Jonathan Bedard
Comment 4 2018-12-06 19:54:19 PST
Created attachment 356781 [details] Possible Improvement
Jonathan Bedard
Comment 5 2018-12-06 19:58:11 PST
I don't have a great way to test this (nor am I familiar with the original bug) but I have 2 patches, either one will likely fix the issue. The first patch will return the original behavior and respect WEBKIT_TEST_CHILD_PROCESSES. The second what I think we actually want here, it modifies the maximum number of child processes for the WinCairo port, that way no one has to worry about setting up WEBKIT_TEST_CHILD_PROCESSES on machines running these tests.
Fujii Hironori
Comment 6 2018-12-06 20:57:52 PST
Thank you very much for working on this. I tested both patches, and confirmed both work fine. IMO, Original Behavior patch is somewhat more preferable because Bug 188036 is happening only on WinCairo test bot. WinCairo test bot is now running on 8-thread machine, but has limited RAM. run-webkit-tests trys to run 8 DumpRenderTree processes on it, but fails to spawning a new process due to memory shortage. It doesn't happen on my PC. And, we have a plan to add new bot machines. I don't know the spec yet. But, I hope it would be able to run more workers to shorten testing time.
Jonathan Bedard
Comment 7 2018-12-06 21:20:51 PST
(In reply to Fujii Hironori from comment #6) > Thank you very much for working on this. I tested both patches, > and confirmed both work fine. > > IMO, Original Behavior patch is somewhat more preferable because > Bug 188036 is happening only on WinCairo test bot. WinCairo test > bot is now running on 8-thread machine, but has limited RAM. > run-webkit-tests trys to run 8 DumpRenderTree processes on it, > but fails to spawning a new process due to memory shortage. It > doesn't happen on my PC. And, we have a plan to add new bot > machines. I don't know the spec yet. But, I hope it would be able > to run more workers to shorten testing time. For now, I'll land the first then. We've actually had this problem with simulators too, the 'right' solution is to have the port define default processes based on CPU and RAM.
Fujii Hironori
Comment 8 2018-12-06 21:27:23 PST
(In reply to Jonathan Bedard from comment #7) > For now, I'll land the first then. > > We've actually had this problem with simulators too, the 'right' solution is > to have the port define default processes based on CPU and RAM. It makes sense. I get the idea.
Jonathan Bedard
Comment 9 2018-12-06 21:28:03 PST
Created attachment 356782 [details] Patch for landing
WebKit Commit Bot
Comment 10 2018-12-06 22:17:53 PST
The commit-queue encountered the following flaky tests while processing attachment 356782 [details]: imported/w3c/web-platform-tests/WebCryptoAPI/generateKey/failures_RSASSA-PKCS1-v1_5.https.any.html bug 192488 The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 11 2018-12-06 22:18:41 PST
Comment on attachment 356782 [details] Patch for landing Clearing flags on attachment: 356782 Committed r238950: <https://trac.webkit.org/changeset/238950>
WebKit Commit Bot
Comment 12 2018-12-06 22:18:43 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 13 2018-12-06 22:19:30 PST
<rdar://problem/46546609>
Fujii Hironori
Comment 14 2018-12-07 01:16:38 PST
WinCairo test bots get back to normal. Thank you very much. I am going to do comment#7 task in Bug 192490.
Note You need to log in before you can comment on or make changes to this bug.