Bug 170741 - webkitpy: Ignore previously launched pid when system is under stress
Summary: webkitpy: Ignore previously launched pid when system is under stress
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: iPhone / iPad All
: P1 Critical
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-04-11 11:11 PDT by Jonathan Bedard
Modified: 2017-04-18 10:35 PDT (History)
9 users (show)

See Also:


Attachments
Patch (2.20 KB, patch)
2017-04-11 12:00 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (2.67 KB, patch)
2017-04-13 15:36 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2017-04-11 11:11:06 PDT
When launching apps on an iOS Simulator, xcrun simctl launch will sometimes return the pid of the previous process if that process is still being torn down.  When the system is stressed, the race conditions which exist here can get worse.  Mitigate these issues by checking if the returned PID is a new PID in SimulatorProcess.
Comment 1 Radar WebKit Bug Importer 2017-04-11 11:12:57 PDT
<rdar://problem/31560432>
Comment 2 Jonathan Bedard 2017-04-11 12:00:59 PDT
Created attachment 306835 [details]
Patch
Comment 3 David Kilzer (:ddkilzer) 2017-04-13 15:31:25 PDT
Comment on attachment 306835 [details]
Patch

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

r=me

> Tools/Scripts/webkitpy/port/simulator_process.py:96
> +            raise Exception('Faild to launch {}, kept receiving old PID'.format(os.path.basename(self._cmd[0])))

Jonathan says this should be a RuntimeException.

Adding a blank space before the "try:" line would make this easier to read.
Comment 4 Jonathan Bedard 2017-04-13 15:36:11 PDT
Created attachment 307036 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2017-04-13 16:20:07 PDT
Comment on attachment 307036 [details]
Patch for landing

Clearing flags on attachment: 307036

Committed r215346: <http://trac.webkit.org/changeset/215346>
Comment 6 WebKit Commit Bot 2017-04-13 16:20:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Jonathan Bedard 2017-04-14 08:14:03 PDT
Follow up fix in <http://trac.webkit.org/changeset/215361>.
Comment 8 Jonathan Bedard 2017-04-14 10:10:32 PDT
Another follow up fix in <http://trac.webkit.org/changeset/215363>.
Comment 9 Ryan Haddad 2017-04-14 10:57:50 PDT
Reverted r215363 for reason:

This change causes LayoutTests to exit early with crashes.

Committed r215367: <http://trac.webkit.org/changeset/215367>
Comment 10 Jonathan Bedard 2017-04-14 13:53:00 PDT
Addressed Ryan's rollout in <http://trac.webkit.org/changeset/215374>.
Comment 11 Ryan Haddad 2017-04-14 16:26:58 PDT
Reverted r215374 for reason:

This change causes LayoutTests to exit early with crashes on Sierra.

Committed r215380: <http://trac.webkit.org/changeset/215380>
Comment 12 Jonathan Bedard 2017-04-17 09:17:11 PDT
If <https://trac.webkit.org/changeset/215416/webkit> fixes this problem, I will roll-out all of the other changes.

I think <https://trac.webkit.org/changeset/215416/webkit> is the root cause.
Comment 13 Jonathan Bedard 2017-04-18 10:35:26 PDT
Reverted r215346 and r215361 for reason:

The problem these changes were fixing was addressed in <https://trac.webkit.org/changeset/215416/webkit>.

Committed r215470: <http://trac.webkit.org/changeset/215470>