Bug 160782

Summary: REGRESSION (r204226): LayoutTest editing/deleting/delete-empty-line-breaks-at-end-of-textarea.html "crashing" without a crashlog
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, barraclough, cdumez, commit-queue, jbedard
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=157977
https://bugs.webkit.org/show_bug.cgi?id=122475
https://bugs.webkit.org/show_bug.cgi?id=157990
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Ryan Haddad 2016-08-11 14:04:13 PDT
LayoutTest editing/deleting/delete-empty-line-breaks-at-end-of-textarea.html "crashing" without a crashlog

Most recent "crash":
https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/builds/7275

Flakiness dashboard:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fdeleting%2Fdelete-empty-line-breaks-at-end-of-textarea.html
Comment 1 Jonathan Bedard 2016-08-12 08:46:38 PDT
Let's wait on https://bugs.webkit.org/show_bug.cgi?id=160779, I suspect that this is the same bug we were seeing there, although I can't be sure because the --no-sample-on-timeout disables the output we would see before the test gets marked as having crashed.

Even if https://bugs.webkit.org/show_bug.cgi?id=160779 is the issue we're seeing here, we are still going to have a timeout occurring.  Why is this suite disabling sample on timeout?
Comment 3 Jonathan Bedard 2016-08-16 15:37:16 PDT
I guess send that Radar back my way.  https://trac.webkit.org/changeset/204514 definitely fixed a problem, I guess it just wasn't this one.
Comment 4 Ryan Haddad 2016-08-16 15:47:18 PDT
(In reply to comment #2)
> It looks like this happened again after
> https://trac.webkit.org/changeset/204514 landed
> 
> https://build.webkit.org/builders/
> Apple%20El%20Capitan%20Release%20WK2%20(Tests)/builds/8541

Sorry, that link should have been:
https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/8539
Comment 5 Jonathan Bedard 2016-08-17 10:05:14 PDT
Taking a closer look at the history of editing/deleting/delete-empty-line-breaks-at-end-of-textarea.html, this seems to be a separate problem from radar 27786762.  I'm not sure what's causing this test to be marked as crashed, it's quite possible this is a true crash and it simply can't match the crash log.

Taking a look at https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/builds/8296/steps/layout-test/logs/stdio (which is where this bug first starts showing up) we can see process 78827 crashing, then 78817 is killed, then an attempt to find the crashlog of 78827 (which would seem to fail, otherwise the results would be attached) and then 78827 crashing for a second time in the next test.  This second piece should not be possible, as 78827 should have already crashed.
Comment 6 Jonathan Bedard 2016-08-17 10:54:27 PDT
https://bugs.webkit.org/show_bug.cgi?id=160943 should help debug this.
Comment 7 Jonathan Bedard 2016-08-17 16:29:31 PDT
https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/builds/8583 has this bug again.  None of the logging added in <http://trac.webkit.org/changeset/204577> caught it, so somehow, a test is being erroneously marked as crashed outside of the framework we are using for marking tests as crashed.
Comment 8 Jonathan Bedard 2016-08-18 10:16:06 PDT
Created attachment 286374 [details]
Patch
Comment 9 Alexey Proskuryakov 2016-08-18 10:25:26 PDT
Comment on attachment 286374 [details]
Patch

Discussed in person.
Comment 10 Jonathan Bedard 2016-08-18 16:17:45 PDT
Some additional information on this: This particular bug does not seem to be in the testing rig, but rather the test itself.  This particular error also seems to be related to the preceding test, editing/deleting/delete-emoji.html.  In short, the WebProcess is exiting unexpectedly, but with an exit() instead of an abort(), meaning no crash-log is generated.

On one occasion, I have managed to reproduced this error locally, but without the debugger enabled.
Comment 11 Ryan Haddad 2016-08-19 13:36:01 PDT
Marked test as flaky on El Capitan in http://trac.webkit.org/projects/webkit/changeset/204650 to get the bots back to green.
Comment 12 Alexey Proskuryakov 2016-08-23 09:34:27 PDT
What happens here is that editing/deleting/delete-emoji.html is extremely slow to repaint, and sometimes it repaints after finishing. When this happens, WebKitTestRunner cannot reset WebContent process for the next test, and attempts to terminate it, and to launch a new one.

However, calling WKPageTerminate() results in a processDidCrash() callback, and then WebKitTestRunner itself terminates with an exit(). It seems suspicious that we get a processDidCrash() when intentionally terminating.

    frame #12: 0x00007fff9857c767 libsystem_c.dylib`exit + 55
    frame #13: 0x00000001002f5b30 WebKitTestRunner`WTR::TestController::processDidCrash(this=0x00007fff5f9667a0) + 576 at TestController.cpp:1730
    frame #14: 0x00000001002daf2c WebKitTestRunner`WTR::TestController::processDidCrash(page=0x000061d000039880, clientInfo=0x00007fff5f9667a0) + 28 at TestController.cpp:1518
    frame #15: 0x00000001099ef958 WebKit`WKPageSetPageNavigationClient::NavigationClient::processDidCrash(this=0x0000610000049540, page=0x000061d000039898) + 264 at WKPage.cpp:2325
    frame #16: 0x000000010902b6c8 WebKit`WebKit::WebPageProxy::processDidCrash(this=0x000061d000039898) + 792 at WebPageProxy.cpp:5190
    frame #17: 0x00000001095cce52 WebKit`WebKit::WebProcessProxy::didClose(this=0x0000619000035780, (null)=0x000061400009d240) + 434 at WebProcessProxy.cpp:541
    frame #18: 0x00000001095d19d5 WebKit`WebKit::WebProcessProxy::requestTermination(this=0x0000619000035780) + 197 at WebProcessProxy.cpp:808
    frame #19: 0x0000000109000366 WebKit`WebKit::WebPageProxy::terminateProcess(this=0x000061d000039898) + 230 at WebPageProxy.cpp:2355
    frame #20: 0x00000001099bfdfd WebKit`::WKPageTerminate(pageRef=0x000061d000039880) + 29 at WKPage.cpp:409
    frame #21: 0x00000001002e2c8d WebKitTestRunner`WTR::TestController::terminateWebContentProcess(this=0x00007fff5f9667a0) + 109 at TestController.cpp:805
    frame #22: 0x00000001003389ec WebKitTestRunner`WTR::TestInvocation::invoke(this=0x0000611000079d00) + 2940 at TestInvocation.cpp:180
    frame #23: 0x00000001002ef2b9 WebKitTestRunner`WTR::TestController::runTest(this=<unavailable>, inputLine=<unavailable>) + 4873 at TestController.cpp:1098
    frame #24: 0x00000001002f1f5c WebKitTestRunner`WTR::TestController::runTestingServerLoop(this=0x00007fff5f9667a0) + 524 at TestController.cpp:1115
    frame #25: 0x00000001002d8a2f WebKitTestRunner`WTR::TestController::run(this=0x00007fff5f9667a0) + 159 at TestController.cpp:1123
    frame #26: 0x00000001002d7645 WebKitTestRunner`WTR::TestController::TestController(this=<unavailable>, argc=<unavailable>, argv=<unavailable>) + 8069 at TestController.cpp:126
    frame #27: 0x00000001002d8d73 WebKitTestRunner`WTR::TestController::TestController(this=0x00007fff5f9667a0, argc=2, argv=0x00007fff5f966a30) + 35 at TestController.cpp:123
Comment 13 Alexey Proskuryakov 2016-08-23 09:45:14 PDT
What's still unclear is why this only started to happen recently, around August 5th.
Comment 14 Alexey Proskuryakov 2016-08-23 10:18:44 PDT
Found it, regressed with <https://trac.webkit.org/changeset/204226>.

The crash a few hours prior to that was a real one, a gamepad patch was making lots of tests crash.
Comment 15 Jonathan Bedard 2016-08-23 11:07:20 PDT
Created attachment 286738 [details]
Patch
Comment 16 Chris Dumez 2016-08-23 11:08:41 PDT
Comment on attachment 286738 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        Backing out r204226.

r204243 is the last version of this change.
Comment 17 Chris Dumez 2016-08-23 11:09:30 PDT
Comment on attachment 286738 [details]
Patch

This should also remove the change to WebPageProxy, please see:
https://trac.webkit.org/changeset/204243
Comment 18 Chris Dumez 2016-08-23 11:11:26 PDT
I did a roll out in <http://trac.webkit.org/changeset/204841>. Just unskip the test.
Comment 19 Jonathan Bedard 2016-08-23 11:25:09 PDT
Created attachment 286747 [details]
Patch
Comment 20 WebKit Commit Bot 2016-08-23 11:58:40 PDT
Comment on attachment 286747 [details]
Patch

Clearing flags on attachment: 286747

Committed r204845: <http://trac.webkit.org/changeset/204845>
Comment 21 WebKit Commit Bot 2016-08-23 11:58:47 PDT
All reviewed patches have been landed.  Closing bug.