Bug 228752

Summary: [webkitpy] Narrow set of terminated simulator processes
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, dewei_zhu, ews-watchlist, glenn, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ews-feeder: commit-queue-

Description Jonathan Bedard 2021-08-03 14:06:12 PDT 
We are terminating too many simulator processes in our simulator management code, and we're not recovering when failing to delete a simulated device.
Comment 1 Radar WebKit Bug Importer 2021-08-03 14:08:41 PDT 
<rdar://problem/81479508>
Comment 2 Jonathan Bedard 2021-08-03 14:11:39 PDT 
Created attachment 434861 [details]
Patch
Comment 3 Aakash Jain 2021-08-03 14:28:23 PDT 
Comment on attachment 434861 [details]
Patch

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

> Tools/Scripts/webkitpy/xcode/simulated_device.py:-508
> -        if SimulatedDeviceManager._managing_simulator_app:

why don't we need this anymore?
Comment 4 Aakash Jain 2021-08-03 14:29:33 PDT 
Overall, how do you come up with this? How do we know that we won't be killing too less (or different thing than needed) after this?
Comment 5 Jonathan Bedard 2021-08-03 14:40:15 PDT 
Comment on attachment 434861 [details]
Patch

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

>> Tools/Scripts/webkitpy/xcode/simulated_device.py:-508
>> -        if SimulatedDeviceManager._managing_simulator_app:
> 
> why don't we need this anymore?

Because CoreSimulator is mature enough now that we don't need to do this any more, we end up killing processes before they finish tearing themselves down.
Comment 6 Jonathan Bedard 2021-08-03 14:46:28 PDT 
(In reply to Aakash Jain from comment #4)
> Overall, how do you come up with this? How do we know that we won't be
> killing too less (or different thing than needed) after this?

We will be killing less, in every case. That's why I did Simulator.app instead of Simulator. I started to play around killing less processes and seeing what that did, it made interacting with simctl much more stable. Which isn't particularly surprising, we shouldn't really need to kill CoreSimulator processes.
Comment 7 Alexey Proskuryakov 2021-08-03 14:58:42 PDT 
Comment on attachment 434861 [details]
Patch

Seems worth trying!
Comment 8 EWS 2021-08-03 15:17:20 PDT 
Committed r280618 (240231@main): <https://commits.webkit.org/240231@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 434861 [details].