-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Refactor emulator start and stop functions for clarity and efficiency #22861
base: main
Are you sure you want to change the base?
Changes from 15 commits
91d7fc3
d9af6f0
85780c3
afa5e9a
1ceeb24
bf977a9
a5a45ca
db94c41
5c5b014
a405d62
b9ea99c
5f0f8a8
01e3edd
4ff05f7
02d619f
f6a9526
2f3ddf1
b3c7cd4
fbf5bcc
b98dbd3
342e42c
a36f12f
3c73a6e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import collections | ||
import contextlib | ||
import datetime | ||
import os | ||
import signal | ||
import subprocess | ||
import time | ||
|
@@ -105,8 +106,13 @@ def _stop_process_with_pid(pid: int): | |
|
||
|
||
def start_emulator( | ||
sdk_tool_paths: SdkToolPaths, avd_name: str, extra_args: typing.Optional[typing.Sequence[str]] = None | ||
sdk_tool_paths: SdkToolPaths, avd_name: str, extra_args: typing.Optional[typing.Sequence[str]] = None, | ||
timeout_minutes: int = 20 | ||
) -> subprocess.Popen: | ||
jchen351 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if is_emulator_running_by_avd(avd_name=avd_name): | ||
raise RuntimeError( | ||
f"An emulator with avd_name{avd_name} is already running. Please close it before starting a new one." | ||
) | ||
with contextlib.ExitStack() as emulator_stack, contextlib.ExitStack() as waiter_stack: | ||
emulator_args = [ | ||
sdk_tool_paths.emulator, | ||
|
@@ -118,10 +124,13 @@ def start_emulator( | |
"America/Los_Angeles", | ||
"-no-snapstorage", | ||
"-no-audio", | ||
"-no-snapshot", # No bluetooth | ||
"-no-boot-anim", | ||
jchen351 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"-gpu", | ||
"guest", | ||
"-delay-adb", | ||
"-prop persist.sys.disable_bluetooth=true", | ||
"-verbose", | ||
] | ||
|
||
# For Linux CIs we must use "-no-window" otherwise you'll get | ||
|
@@ -155,9 +164,9 @@ def start_emulator( | |
waiter_stack.callback(_stop_process, waiter_process) | ||
|
||
# poll subprocesses. | ||
# allow 20 minutes for startup as some CIs are slow. TODO: Make timeout configurable if needed. | ||
# allow 20 minutes for startup as some CIs are slow. | ||
sleep_interval_seconds = 10 | ||
end_time = datetime.datetime.now() + datetime.timedelta(minutes=20) | ||
end_time = datetime.datetime.now() + datetime.timedelta(minutes=timeout_minutes) | ||
|
||
while True: | ||
waiter_ret, emulator_ret = waiter_process.poll(), emulator_process.poll() | ||
|
@@ -204,14 +213,121 @@ def start_emulator( | |
|
||
_log.debug(f"sys.boot_completed='{getprop_value}'. Sleeping for {sleep_interval_seconds} before retrying.") | ||
time.sleep(sleep_interval_seconds) | ||
|
||
# Verify if the emulator is now running | ||
jchen351 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if not is_emulator_running_by_avd(avd_name=avd_name): | ||
raise RuntimeError("Emulator failed to start.") | ||
return emulator_process | ||
|
||
|
||
def stop_emulator(emulator_proc_or_pid: typing.Union[subprocess.Popen, int]): | ||
def is_emulator_running_by_avd(avd_name: str) -> bool: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we call these something like The 'by' sounds like we could be checked how it was started rather than if it's currently running. |
||
""" | ||
Check if an emulator is running based on the provided AVD name. | ||
:param avd_name: Name of the Android Virtual Device (AVD) to check. | ||
:return: True if an emulator with the given AVD name is running, False otherwise. | ||
""" | ||
try: | ||
# Step 1: List running devices | ||
result = subprocess.check_output(["adb", "devices"], text=True).strip() | ||
_log.info(f"adb devices output:\n{result}") | ||
running_emulators = [line.split("\t")[0] for line in result.splitlines()[1:] if "emulator" in line] | ||
|
||
if not running_emulators: | ||
_log.debug("No emulators running.") | ||
return False # No emulators running | ||
|
||
# Step 2: Check each running emulator's AVD name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we have step 3 of checking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it is necessary, because It is used by
|
||
for emulator in running_emulators: | ||
try: | ||
avd_info = subprocess.check_output(["adb", "-s", emulator, "emu", "avd", "name"], text=True).strip() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the expected output of this command? I tried with both the phones connected to the mac mini but got no output so wondering if it differs for a real device vs an emulator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be only for emulator. Generated by ChatGPT. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got
when I run this command locally. |
||
if avd_info == avd_name: | ||
return True | ||
except subprocess.SubprocessError: | ||
_log.warning(f"Error checking AVD name for emulator: {emulator}") | ||
continue # Skip if there's an issue querying a specific emulator | ||
_log.warning(f"No emulator running with AVD name: {avd_name}") | ||
jchen351 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return False # No matching AVD name found | ||
except subprocess.SubprocessError as e: | ||
_log.warning(f"Error checking emulator status: {e}") | ||
return False | ||
|
||
|
||
def is_emulator_running_by_proc(emulator_proc: subprocess.Popen) -> bool: | ||
"""Check if the emulator process is running based on a Popen instance.""" | ||
return emulator_proc.poll() is None | ||
|
||
|
||
def is_emulator_running_by_pid(emulator_pid: int) -> bool: | ||
"""Check if the emulator process is running based on PID.""" | ||
try: | ||
os.kill(emulator_pid, 0) # Signal 0 checks process existence | ||
|
||
return True | ||
except OSError: | ||
return False | ||
|
||
|
||
def stop_emulator_by_proc(emulator_proc: subprocess.Popen, timeout: int = 120): | ||
jchen351 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
Stops the emulator process using a subprocess.Popen instance. | ||
:param emulator_proc: The emulator process as a subprocess.Popen instance. | ||
:param timeout: Maximum time (in seconds) to wait for the emulator to stop. | ||
""" | ||
if not is_emulator_running_by_proc(emulator_proc): | ||
_log.warning("The specified emulator process is not running.") | ||
return | ||
|
||
_log.info("Stopping emulator using subprocess.Popen instance.") | ||
_stop_process(emulator_proc) | ||
|
||
# Wait for the process to stop | ||
interval = 5 | ||
end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout) | ||
|
||
while is_emulator_running_by_proc(emulator_proc): | ||
if datetime.datetime.now() > end_time: | ||
raise RuntimeError(f"Failed to stop the emulator within the specified timeout = {timeout} seconds.") | ||
_log.debug("Emulator still running. Checking again in 5 seconds...") | ||
time.sleep(interval) | ||
|
||
_log.info("Emulator stopped successfully.") | ||
|
||
|
||
def stop_emulator_by_pid(emulator_pid: int, timeout: int = 120): | ||
""" | ||
Stops the emulator process using a PID. | ||
:param emulator_pid: The emulator process PID. | ||
:param timeout: Maximum time (in seconds) to wait for the emulator to stop. | ||
""" | ||
if not is_emulator_running_by_pid(emulator_pid): | ||
_log.warning(f"No emulator process with PID {emulator_pid} is currently running.") | ||
return | ||
|
||
_log.info(f"Stopping emulator with PID: {emulator_pid}") | ||
_stop_process_with_pid(emulator_pid) | ||
|
||
# Wait for the process to stop | ||
interval = 5 | ||
end_time = datetime.datetime.now() + datetime.timedelta(seconds=timeout) | ||
|
||
while is_emulator_running_by_pid(emulator_pid): | ||
if datetime.datetime.now() > end_time: | ||
raise RuntimeError( | ||
f"Failed to stop the emulator with PID {emulator_pid} within the specified timeout = {timeout} seconds." | ||
) | ||
_log.debug("Emulator still running. Checking again in 5 seconds...") | ||
time.sleep(interval) | ||
|
||
_log.info("Emulator stopped successfully.") | ||
|
||
|
||
def stop_emulator(emulator_proc_or_pid: typing.Union[subprocess.Popen, int], timeout: int = 120): | ||
""" | ||
Stops the emulator process, checking its running status before and after stopping. | ||
:param emulator_proc_or_pid: The emulator process (subprocess.Popen) or PID (int). | ||
:param timeout: Maximum time (in seconds) to wait for the emulator to stop. | ||
""" | ||
if isinstance(emulator_proc_or_pid, subprocess.Popen): | ||
_stop_process(emulator_proc_or_pid) | ||
stop_emulator_by_proc(emulator_proc_or_pid, timeout) | ||
elif isinstance(emulator_proc_or_pid, int): | ||
_stop_process_with_pid(emulator_proc_or_pid) | ||
stop_emulator_by_pid(emulator_proc_or_pid, timeout) | ||
else: | ||
raise ValueError("Expected either a PID or subprocess.Popen instance.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also update the versions in onnxruntime-extensions and onnxruntime-genai once the changes are checked in.