Bug 170741

Summary:

webkitpy: Ignore previously launched pid when system is under stress

Product:

WebKit

Reporter:

Jonathan Bedard <jbedard>

Component:

Tools / Tests

Assignee:

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:

Keywords:

InRadar

Version:

Other

Hardware:

iPhone / iPad

OS:

All

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Jonathan Bedard

Reported 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.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2017-04-11 11:12:57 PDT

<rdar://problem/31560432>

Jonathan Bedard

Comment 2 2017-04-11 12:00:59 PDT

Created attachment 306835 [details] Patch

David Kilzer (:ddkilzer)

Comment 3 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.

Jonathan Bedard

Comment 4 2017-04-13 15:36:11 PDT

Created attachment 307036 [details] Patch for landing

WebKit Commit Bot

Comment 5 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>

WebKit Commit Bot

Comment 6 2017-04-13 16:20:08 PDT

All reviewed patches have been landed. Closing bug.

Jonathan Bedard

Comment 7 2017-04-14 08:14:03 PDT

Follow up fix in <http://trac.webkit.org/changeset/215361>.

Jonathan Bedard

Comment 8 2017-04-14 10:10:32 PDT

Another follow up fix in <http://trac.webkit.org/changeset/215363>.

Ryan Haddad

Comment 9 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>

Jonathan Bedard

Comment 10 2017-04-14 13:53:00 PDT

Addressed Ryan's rollout in <http://trac.webkit.org/changeset/215374>.

Ryan Haddad

Comment 11 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>

Jonathan Bedard

Comment 12 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.

Jonathan Bedard

Comment 13 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>

Note You need to log in before you can comment on or make changes to this bug.