Bug 170741

Summary: webkitpy: Ignore previously launched pid when system is under stress
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Critical CC: aakash_jain, buildbot, commit-queue, ddkilzer, dean_johnson, glenn, lforschler, ryanhaddad, webkit-bug-importer
Priority: P1 Keywords: InRadar
Version: Other   
Hardware: iPhone / iPad   
OS: All   
Attachments:
Description Flags
Patch
none
Patch for landing none

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>