RESOLVED FIXED 196694
iOS 12 Simulator Release WK2 frequently timing out while running layout tests 
https://bugs.webkit.org/show_bug.cgi?id=196694
Summary iOS 12 Simulator Release WK2 frequently timing out while running layout tests
Jonathan Bedard
Reported 2019-04-08 09:25:17 PDT
This is related to communication between ImageDiff and webkitpy, as killing ImageDiff causes tests to continue running.
Attachments
Patch (1.89 KB, patch)
2019-04-08 09:33 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (2.32 KB, patch)
2019-04-08 13:23 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (1.23 KB, patch)
2019-04-08 15:34 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (6.21 KB, patch)
2019-04-08 17:07 PDT, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Jonathan Bedard
Comment 1 2019-04-08 09:25:40 PDT
<rdar://problem/48388734>
Jonathan Bedard
Comment 2 2019-04-08 09:33:36 PDT
Created attachment 366941 [details] Patch
Jonathan Bedard
Comment 3 2019-04-08 09:34:07 PDT
Comment on attachment 366941 [details] Patch This is a speculative change, I'm testing it now.
Alexey Proskuryakov
Comment 4 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.
Jonathan Bedard
Comment 5 2019-04-08 13:23:05 PDT
Created attachment 366974 [details] Patch
Jonathan Bedard
Comment 6 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.
Jonathan Bedard
Comment 7 2019-04-08 15:34:23 PDT
Created attachment 366990 [details] Patch
Jonathan Bedard
Comment 8 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'
Jonathan Bedard
Comment 9 2019-04-08 17:07:46 PDT
Created attachment 367002 [details] Patch
Jonathan Bedard
Comment 10 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.
Aakash Jain
Comment 11 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?
Jonathan Bedard
Comment 12 2019-04-09 09:14:45 PDT
Committed r244080: <https://trac.webkit.org/changeset/244080>
Jonathan Bedard
Comment 13 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.
Jonathan Bedard
Comment 14 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.
Note You need to log in before you can comment on or make changes to this bug.