Avoid subprocess memory copy when c library supports posix_spawn (#87958)

* use posix spawn on alpine

* Avoid subprocess memory copy when c library supports posix_spawn

By default python 3.10 will use the fork() which has to
copy all the memory of the parent process (in our case
this can be huge since Home Assistant core can use
hundreds of megabytes of RAM). By using posix_spawn
this is avoided.

In python 3.11 vfork will also be available
https://github.com/python/cpython/issues/80004#issuecomment-1093810689
https://github.com/python/cpython/pull/11671 but we won't
always be able to use it and posix_spawn is considered safer
https://bugzilla.kernel.org/show_bug.cgi?id=215813#c14

The subprocess library doesn't know about musl though
even though it supports posix_spawn https://git.musl-libc.org/cgit/musl/log/src/process/posix_spawn.c
so we have to teach it since it only has checks for glibc
1b736838e6/Lib/subprocess.py (L745)

The constant is documented as being able to be flipped here:
https://docs.python.org/3/library/subprocess.html#disabling-use-of-vfork-or-posix-spawn

* Avoid subprocess memory copy when c library supports posix_spawn

By default python 3.10 will use the fork() which has to
copy memory of the parent process (in our case
this can be huge since Home Assistant core can use
hundreds of megabytes of RAM). By using posix_spawn
this is avoided and subprocess creation does not
get discernibly slow the larger the Home Assistant
python process grows.

In python 3.11 vfork will also be available
https://github.com/python/cpython/issues/80004#issuecomment-1093810689
https://github.com/python/cpython/pull/11671 but we won't
always be able to use it and posix_spawn is considered safer
https://bugzilla.kernel.org/show_bug.cgi?id=215813#c14

The subprocess library doesn't know about musl though
even though it supports posix_spawn https://git.musl-libc.org/cgit/musl/log/src/process/posix_spawn.c
so we have to teach it since it only has checks for glibc
1b736838e6/Lib/subprocess.py (L745)

The constant is documented as being able to be flipped here:
https://docs.python.org/3/library/subprocess.html#disabling-use-of-vfork-or-posix-spawn

* missed some

* adjust more tests

* coverage
This commit is contained in:
J. Nick Koston 2023-02-13 08:02:51 -06:00 committed by GitHub
parent cf2fcdfba1
commit 03eea7bd3f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 84 additions and 9 deletions

View file

@ -68,6 +68,7 @@ class CommandLineAuthProvider(AuthProvider):
*self.config[CONF_ARGS],
env=env,
stdout=asyncio.subprocess.PIPE if self.config[CONF_META] else None,
close_fds=False, # required for posix_spawn
)
stdout, _ = await process.communicate()
except OSError as err:

View file

@ -18,7 +18,10 @@ def call_shell_with_timeout(
try:
_LOGGER.debug("Running command: %s", command)
subprocess.check_output(
command, shell=True, timeout=timeout # nosec # shell by design
command,
shell=True, # nosec # shell by design
timeout=timeout,
close_fds=False, # required for posix_spawn
)
return 0
except subprocess.CalledProcessError as proc_exception:
@ -41,7 +44,10 @@ def check_output_or_log(command: str, timeout: int) -> str | None:
"""Run a shell command with a timeout and return the output."""
try:
return_value = subprocess.check_output(
command, shell=True, timeout=timeout # nosec # shell by design
command,
shell=True, # nosec # shell by design
timeout=timeout,
close_fds=False, # required for posix_spawn
)
return return_value.strip().decode("utf-8")
except subprocess.CalledProcessError as err:

View file

@ -52,6 +52,7 @@ class CommandLineNotificationService(BaseNotificationService):
self.command,
universal_newlines=True,
stdin=subprocess.PIPE,
close_fds=False, # required for posix_spawn
shell=True, # nosec # shell by design
) as proc:
try:

View file

@ -227,6 +227,7 @@ class PingDataSubProcess(PingData):
stdin=None,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
close_fds=False, # required for posix_spawn
)
try:
out_data, out_error = await asyncio.wait_for(

View file

@ -55,7 +55,10 @@ class HostSubProcess:
def ping(self):
"""Send an ICMP echo request and return True if success."""
with subprocess.Popen(
self._ping_cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL
self._ping_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.DEVNULL,
close_fds=False, # required for posix_spawn
) as pinger:
try:
pinger.communicate(timeout=1 + PING_TIMEOUT)

View file

@ -32,7 +32,10 @@ _LOGGER = logging.getLogger(__name__)
def kill_raspistill(*args):
"""Kill any previously running raspistill process.."""
with subprocess.Popen(
["killall", "raspistill"], stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT
["killall", "raspistill"],
stdout=subprocess.DEVNULL,
stderr=subprocess.STDOUT,
close_fds=False, # required for posix_spawn
):
pass
@ -132,7 +135,10 @@ class RaspberryCamera(Camera):
# Therefore it must not be wrapped with "with", since that
# waits for the subprocess to exit before continuing.
subprocess.Popen( # pylint: disable=consider-using-with
cmd_args, stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT
cmd_args,
stdout=subprocess.DEVNULL,
stderr=subprocess.STDOUT,
close_fds=False, # required for posix_spawn
)
def camera_image(

View file

@ -130,7 +130,10 @@ class ImageProcessingSsocr(ImageProcessingEntity):
img.save(self.filepath, "png")
with subprocess.Popen(
self._command, stdout=subprocess.PIPE, stderr=subprocess.PIPE
self._command,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
close_fds=False, # Required for posix_spawn
) as ocr:
out = ocr.communicate()
if out[0] != b"":

View file

@ -65,6 +65,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
stdin=None,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
close_fds=False, # required for posix_spawn
)
else:
# Template used. Break into list and use create_subprocess_exec
@ -76,6 +77,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
stdin=None,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
close_fds=False, # required for posix_spawn
)
process = await create_process

View file

@ -6,6 +6,7 @@ from asyncio import events
import dataclasses
import logging
import os
import subprocess
import threading
import traceback
from typing import Any
@ -28,6 +29,7 @@ from .util.thread import deadlock_safe_shutdown
#
MAX_EXECUTOR_WORKERS = 64
TASK_CANCELATION_TIMEOUT = 5
ALPINE_RELEASE_FILE = "/etc/alpine-release"
_LOGGER = logging.getLogger(__name__)
@ -153,8 +155,22 @@ async def setup_and_run_hass(runtime_config: RuntimeConfig) -> int:
return await hass.async_run()
def _enable_posix_spawn() -> None:
"""Enable posix_spawn on Alpine Linux."""
# pylint: disable=protected-access
if subprocess._USE_POSIX_SPAWN:
return
# The subprocess module does not know about Alpine Linux/musl
# and will use fork() instead of posix_spawn() which significantly
# less efficient. This is a workaround to force posix_spawn()
# on Alpine Linux which is supported by musl.
subprocess._USE_POSIX_SPAWN = os.path.exists(ALPINE_RELEASE_FILE)
def run(runtime_config: RuntimeConfig) -> int:
"""Run Home Assistant."""
_enable_posix_spawn()
asyncio.set_event_loop_policy(HassEventLoopPolicy(runtime_config.debug))
# Backport of cpython 3.9 asyncio.run with a _cancel_all_tasks that times out
loop = asyncio.new_event_loop()

View file

@ -94,7 +94,14 @@ def install_package(
args += ["--user"]
env["PYTHONUSERBASE"] = os.path.abspath(target)
_LOGGER.debug("Running pip command: args=%s", args)
with Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env) as process:
with Popen(
args,
stdin=PIPE,
stdout=PIPE,
stderr=PIPE,
env=env,
close_fds=False, # required for posix_spawn
) as process:
_, stderr = process.communicate()
if process.returncode != 0:
_LOGGER.error(
@ -121,6 +128,7 @@ async def async_get_user_site(deps_dir: str) -> str:
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.DEVNULL,
env=env,
close_fds=False, # required for posix_spawn
)
stdout, _ = await process.communicate()
lib_dir = stdout.decode().strip()

View file

@ -73,7 +73,10 @@ async def test_poll_when_cover_has_command_state(hass: HomeAssistant) -> None:
async_fire_time_changed(hass, dt_util.utcnow() + SCAN_INTERVAL)
await hass.async_block_till_done()
check_output.assert_called_once_with(
"echo state", shell=True, timeout=15 # nosec # shell by design
"echo state",
shell=True, # nosec # shell by design
timeout=15,
close_fds=False,
)

View file

@ -97,6 +97,7 @@ async def test_template_render_with_quote(hass: HomeAssistant) -> None:
'echo "template_value" "3 4"',
shell=True, # nosec # shell by design
timeout=15,
close_fds=False,
)

View file

@ -137,3 +137,22 @@ async def test_unhandled_exception_traceback(hass, caplog):
assert "Task exception was never retrieved" in caplog.text
assert "This is unhandled" in caplog.text
assert "_unhandled_exception" in caplog.text
def test__enable_posix_spawn():
"""Test that we can enable posix_spawn on Alpine."""
def _mock_alpine_exists(path):
return path == "/etc/alpine-release"
with patch.object(runner.subprocess, "_USE_POSIX_SPAWN", False), patch.object(
runner.os.path, "exists", _mock_alpine_exists
):
runner._enable_posix_spawn()
assert runner.subprocess._USE_POSIX_SPAWN is True
with patch.object(runner.subprocess, "_USE_POSIX_SPAWN", False), patch.object(
runner.os.path, "exists", return_value=False
):
runner._enable_posix_spawn()
assert runner.subprocess._USE_POSIX_SPAWN is False

View file

@ -95,6 +95,7 @@ def test_install(mock_sys, mock_popen, mock_env_copy, mock_venv):
stdout=PIPE,
stderr=PIPE,
env=env,
close_fds=False,
)
assert mock_popen.return_value.communicate.call_count == 1
@ -118,6 +119,7 @@ def test_install_upgrade(mock_sys, mock_popen, mock_env_copy, mock_venv):
stdout=PIPE,
stderr=PIPE,
env=env,
close_fds=False,
)
assert mock_popen.return_value.communicate.call_count == 1
@ -142,7 +144,7 @@ def test_install_target(mock_sys, mock_popen, mock_env_copy, mock_venv):
assert package.install_package(TEST_NEW_REQ, False, target=target)
assert mock_popen.call_count == 2
assert mock_popen.mock_calls[0] == call(
args, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env
args, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env, close_fds=False
)
assert mock_popen.return_value.communicate.call_count == 1
@ -185,6 +187,7 @@ def test_install_constraint(mock_sys, mock_popen, mock_env_copy, mock_venv):
stdout=PIPE,
stderr=PIPE,
env=env,
close_fds=False,
)
assert mock_popen.return_value.communicate.call_count == 1
@ -211,6 +214,7 @@ def test_install_find_links(mock_sys, mock_popen, mock_env_copy, mock_venv):
stdout=PIPE,
stderr=PIPE,
env=env,
close_fds=False,
)
assert mock_popen.return_value.communicate.call_count == 1
@ -233,6 +237,7 @@ async def test_async_get_user_site(mock_env_copy):
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.DEVNULL,
env=env,
close_fds=False,
)
assert ret == os.path.join(deps_dir, "lib_dir")