Skip to content

gh-129200: Add locking to the iOS testbed startup sequence. #130564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

freakboy3742
Copy link
Contributor

@freakboy3742 freakboy3742 commented Feb 26, 2025

In order to stream iOS testbed output, it is necessary to run a log stream on the iOS system log. This requires identifying the simulator whose logs need to be streamed.

The Xcode project is responsible for starting the simulator, and doesn't report the identity of the simulator. To work around this, a polling mechanism is used to identify when a new simulator is started. However, this polling mechanism is fragile if 2 testbed runs are started on the same machine at near the same time, as 2 new simulator instances may be created in rapid succession. This causes the log streamer to be unable to identify which simulator is associated with which testbed run.

This PR adds a system-wide locking mechanism around the creation of simulator instances. The lock is acquired just before the first call to obtain the list of simulators; it is released as soon as a new simulator is identified. If two test runs are started at the same time, one will acquire the lock; the other will wait until the first run releases the lock. In this way, multiple testbed runs can be started at the same time, at the cost of a slightly delayed startup of the second testbed run. In my testing, the delay is approximately 1 minute when 2 runs are started; this reflects the time to compile the testbed and start a simulator device. A timeout of 10 minutes is enforced on waiting for the lock.

There is a related race condition in the make testios target; if two tests are started in the same second, they'll generate the same temporary filename for the cloned testbed project. This PR adds the parent PID to the testbed clone name to ensure that tests started in the same second are unique.

@freakboy3742
Copy link
Contributor Author

!buildbot iOS

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit bc647db 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130564%2Fmerge

The command will test the builders whose names match following regular expression: iOS

The builders matched are:

  • iOS ARM64 Simulator PR

@freakboy3742 freakboy3742 requested review from ned-deily and removed request for erlend-aasland February 26, 2025 09:24
class SimulatorLock:
# An fcntl-based filesystem lock that can be used to ensure that
def __init__(self, timeout):
self.filename = Path(tempfile.gettempdir()) / "python-ios-testbed"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the lock need to be system-wide rather than just for a particular user? If so, the use of tempfile.gettempdir() can be problematic here. As documented, it checks first for a TMPDIR environment variable and it appears that usually on macOS TMPDIR is set to a user-specific temp directory, for example:

>>> tempfile.gettempdir()
'/var/folders/sn/0m4rnbyj2z1byjs68sz838300000gn/T'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As best as I can make out, the lock only needs to be per user. If I start a simulator on one user's account, then use Fast User Switching to move to a second user's account, the simulator on the first user's account doesn't show on the second.

@freakboy3742 freakboy3742 merged commit 9211b3d into python:main Feb 27, 2025
50 checks passed
@miss-islington-app
Copy link

Thanks @freakboy3742 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 27, 2025
…thonGH-130564)

Add a lock to ensure that only one iOS testbed per user can start at a time, so
that the simulator discovery process doesn't collide between instances.
(cherry picked from commit 9211b3d)

Co-authored-by: Russell Keith-Magee <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 27, 2025

GH-130657 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Feb 27, 2025
@freakboy3742 freakboy3742 deleted the testbed-parallel-lock branch February 27, 2025 23:33
freakboy3742 added a commit that referenced this pull request Feb 28, 2025
…H-130564) (#130657)

Add a lock to ensure that only one iOS testbed per user can start at a time, so
that the simulator discovery process doesn't collide between instances.
(cherry picked from commit 9211b3d)

Co-authored-by: Russell Keith-Magee <[email protected]>
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
…thon#130564)

Add a lock to ensure that only one iOS testbed per user can start at a time, so
that the simulator discovery process doesn't collide between instances.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants