Bug 196694 - iOS 12 Simulator Release WK2 frequently timing out while running layout tests
Summary: iOS 12 Simulator Release WK2 frequently timing out while running layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-08 09:25 PDT by Jonathan Bedard
Modified: 2019-04-09 09:27 PDT (History)
7 users (show)

See Also:

Attachments
Patch (1.89 KB, patch)
2019-04-08 09:33 PDT, Jonathan Bedard 		no flags Details | Formatted Diff | Diff
Patch (2.32 KB, patch)
2019-04-08 13:23 PDT, Jonathan Bedard 		no flags Details | Formatted Diff | Diff
Patch (1.23 KB, patch)
2019-04-08 15:34 PDT, Jonathan Bedard 		no flags Details | Formatted Diff | Diff
Patch (6.21 KB, patch)
2019-04-08 17:07 PDT, Jonathan Bedard 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2019-04-08 09:25:17 PDT 
This is related to communication between ImageDiff and webkitpy, as killing ImageDiff causes tests to continue running.
Comment 1 Jonathan Bedard 2019-04-08 09:25:40 PDT 
<rdar://problem/48388734>
Comment 2 Jonathan Bedard 2019-04-08 09:33:36 PDT 
Created attachment 366941 [details]
Patch
Comment 3 Jonathan Bedard 2019-04-08 09:34:07 PDT 
Comment on attachment 366941 [details]
Patch

This is a speculative change, I'm testing it now.
Comment 4 Alexey Proskuryakov 2019-04-08 09:39:08 PDT 
Comment on attachment 366941 [details]
Patch

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

> Tools/ChangeLog:10
> +        (main): Only flush stdout after writing to it.

This doesn't sound like it should make a difference.
Comment 5 Jonathan Bedard 2019-04-08 13:23:05 PDT 
Created attachment 366974 [details]
Patch
Comment 6 Jonathan Bedard 2019-04-08 13:26:12 PDT 
The problem here looks to be a deadlock filling up buffers when reading/writing from and to ImageDiff. I have a few ideas, not sure if the fix should be in ImageDiff or webkitpy.
Comment 7 Jonathan Bedard 2019-04-08 15:34:23 PDT 
Created attachment 366990 [details]
Patch
Comment 8 Jonathan Bedard 2019-04-08 15:36:32 PDT 
Comment on attachment 366990 [details]
Patch

This patch seems to stop the problem, but also defeats the point of using ImageDiff as a server process. I think the right way to fix this will basically be 'check if there is stdout available, if there is, stop ImageDiff, otherwise, use the existing process'
Comment 9 Jonathan Bedard 2019-04-08 17:07:46 PDT 
Created attachment 367002 [details]
Patch
Comment 10 Jonathan Bedard 2019-04-08 18:02:44 PDT 
Comment on attachment 367002 [details]
Patch

Verified that this approach solves the problem. It's probably worth investigating how ImageDiff can get into this state, but once it is in this state, we should throw away the process.
Comment 11 Aakash Jain 2019-04-09 08:49:30 PDT 
Comment on attachment 367002 [details]
Patch

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

rs=me

> Tools/Scripts/webkitpy/port/image_diff.py:50
> +        if tolerance != self._tolerance or self._process and self._process.has_available_stdout():

Can we use parenthesis here to make the code more readable?
Comment 12 Jonathan Bedard 2019-04-09 09:14:45 PDT 
Committed r244080: <https://trac.webkit.org/changeset/244080>
Comment 13 Jonathan Bedard 2019-04-09 09:16:42 PDT 
(In reply to Jonathan Bedard from comment #12)
> Committed r244080: <https://trac.webkit.org/changeset/244080>

Not done with this bug yet, but at least change patch will stop the bleeding.
Comment 14 Jonathan Bedard 2019-04-09 09:27:09 PDT 
It seems like it's possible for ImageDiff to dump twice? That's probably where the root cause of this bug is.