From b208186f341a44f04dd9de2e1dd23aaabbcc7d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Desgroppes?= Date: Wed, 19 Aug 2026 08:32:06 +0200 Subject: [PATCH] fix(toolchain): fix crash on Windows when precompiling is enabled `python/private/py_executable.bzl`'s `_maybe_add_test_main_validation` fix (#4079) noted `precompile.bzl` as a remaining user of the same `exec_interpreter` relocation issue, needing the same migration. There was no existing test exercising `_precompile`'s action at all: `tests/base_rules/precompile`'s suite is `analysis_test`-only, checking declared providers, never actually running the precompiler. Reproducing this on Windows therefore required a real `bazel build`, via the new `test_precompile_enabled_succeeds`: ``` bazel test \ //tests/base_rules/precompile:test_precompile_enabled_succeeds ... ERROR: .../tests/base_rules/precompile/BUILD.bazel:3:22: Python precompiling .../test_precompile_enabled_succeeds_main.py into .../test_precompile_enabled_succeeds_main.cpython-311.pyc failed: Worker process did not return a WorkResponse: ---8<---8<--- Start of log, file at .../multiplex-worker-1-PyCompile.log ---8<---8<--- (empty) ---8<---8<--- End of log ---8<---8<--- ``` The worker crashes at startup, unable to find its DLLs, before it can write anything to its own log or respond over the worker protocol. `_precompile` now uses `actions_run()` with `exec_runtime`, exactly as `_maybe_add_test_main_validation` does, instead of `exec_tools_info.exec_interpreter[DefaultInfo].files_to_run`. Reproducing and fixing this also uncovered two more problems, both specific to the precompiler's worker mode and unrelated to `exec_interpreter`. First, `tools/precompiler/precompiler.py`'s persistent worker reads each JSON request as a single line via `asyncio.StreamReader`, whose default 64KiB limit is exceeded once every interpreter distribution file, previously hidden by relocation into a much smaller symlink tree, shows up as an actual, individually-digested action input: ``` ValueError: Separator is not found, and chunk exceed the limit ``` A CPython 3.11 distribution's ~2,260 inputs measure ~470KiB this way; `1 << 22` (4MiB) leaves ample headroom. Second, the worker's default implementation, `_AsyncPersistentWorker`, can't start on Windows at all: `asyncio`'s `ProactorEventLoop` fails to wrap `stdin`/`stdout` as pipe transports, with: ``` OSError: [WinError 6] The handle is invalid ``` Bazel gives workers anonymous pipes (`CreatePipe`) for stdio, which never support overlapped I/O, so `asyncio`'s `ProactorEventLoop` can't register them with an I/O completion port. This is unrelated to precompiling's relocation bug: nothing exercises this worker on Windows today. `_SerialPersistentWorker`, the blocking-I/O alternative already present in the file, has no such issue, so `--worker_impl` now defaults to `serial` on Windows. `tests/base_rules/precompile:test_precompile_enabled_succeeds` is a real, executing `py_test` with `precompile = "enabled"`, added alongside the analysis-only suite to close this gap: it forces the precompiler action to actually run, and needs no CI wiring since it carries no tag excluding it from the existing Windows job's default test sweep. --- news/4082.fixed.md | 3 ++ python/private/precompile.bzl | 41 +++---------------- .../precompile/precompile_tests.bzl | 19 +++++++++ tools/precompiler/precompiler.py | 10 ++++- 4 files changed, 36 insertions(+), 37 deletions(-) create mode 100644 news/4082.fixed.md diff --git a/news/4082.fixed.md b/news/4082.fixed.md new file mode 100644 index 0000000000..12c6a51e17 --- /dev/null +++ b/news/4082.fixed.md @@ -0,0 +1,3 @@ +(toolchain) Fixed a crash on Windows when precompiling is enabled: +the precompiler's interpreter couldn't find its DLLs once relocated +([#4082](https://github.com/bazel-contrib/rules_python/issues/4082)). diff --git a/python/private/precompile.bzl b/python/private/precompile.bzl index c12882bf82..898dc3ee2a 100644 --- a/python/private/precompile.bzl +++ b/python/private/precompile.bzl @@ -15,6 +15,7 @@ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load(":attributes.bzl", "PrecompileAttr", "PrecompileInvalidationModeAttr", "PrecompileSourceRetentionAttr") +load(":common.bzl", "actions_run") load(":flags.bzl", "PrecompileFlag") load(":py_interpreter_program.bzl", "PyInterpreterProgramInfo") load(":toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE", "TARGET_TOOLCHAIN_TYPE") @@ -108,25 +109,8 @@ def _precompile(ctx, src, *, use_pycache): exec_tools_info = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE].exec_tools target_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE].py3_runtime - # These args control starting the precompiler, e.g., when run as a worker, - # these args are only passed once. - precompiler_startup_args = ctx.actions.args() - - env = {} - tools = [] - precompiler = exec_tools_info.precompiler - if PyInterpreterProgramInfo in precompiler: - precompiler_executable = exec_tools_info.exec_interpreter[DefaultInfo].files_to_run - program_info = precompiler[PyInterpreterProgramInfo] - env.update(program_info.env) - precompiler_startup_args.add_all(program_info.interpreter_args) - default_info = precompiler[DefaultInfo] - precompiler_startup_args.add(default_info.files_to_run.executable) - tools.append(default_info.files_to_run) - elif precompiler[DefaultInfo].files_to_run: - precompiler_executable = precompiler[DefaultInfo].files_to_run - else: + if PyInterpreterProgramInfo not in precompiler and not precompiler[DefaultInfo].files_to_run: fail(("Unrecognized precompiler: target '{}' does not provide " + "PyInterpreterProgramInfo nor appears to be executable").format( precompiler, @@ -159,12 +143,6 @@ def _precompile(ctx, src, *, use_pycache): else: invalidation_mode = PrecompileInvalidationModeAttr.CHECKED_HASH - # Though --modify_execution_info exists, it can only set keys with - # empty values, which doesn't work for persistent worker settings. - execution_requirements = {} - if testing.ExecutionInfo in precompiler: - execution_requirements.update(precompiler[testing.ExecutionInfo].requirements) - # These args are passed for every precompilation request, e.g. as part of # a request to a worker process. precompile_request_args = ctx.actions.args() @@ -188,20 +166,13 @@ def _precompile(ctx, src, *, use_pycache): python_version = "{}.{}".format(version_info.major, version_info.minor) precompile_request_args.add("--python_version", python_version) - ctx.actions.run( - executable = precompiler_executable, - arguments = [precompiler_startup_args, precompile_request_args], + actions_run( + ctx, + executable = precompiler, + arguments = [precompile_request_args], inputs = [src], outputs = [pyc], mnemonic = "PyCompile", progress_message = "Python precompiling %{input} into %{output}", - tools = tools, - env = env | { - "PYTHONHASHSEED": "0", # Helps avoid non-deterministic behavior - "PYTHONNOUSERSITE": "1", # Helps avoid non-deterministic behavior - "PYTHONSAFEPATH": "1", # Helps avoid incorrect import issues - }, - execution_requirements = execution_requirements, - toolchain = EXEC_TOOLS_TOOLCHAIN_TYPE, ) return pyc diff --git a/tests/base_rules/precompile/precompile_tests.bzl b/tests/base_rules/precompile/precompile_tests.bzl index bff994aa1a..d2c1da6b8f 100644 --- a/tests/base_rules/precompile/precompile_tests.bzl +++ b/tests/base_rules/precompile/precompile_tests.bzl @@ -14,6 +14,7 @@ """Tests for precompiling behavior.""" +load("@bazel_skylib//rules:write_file.bzl", "write_file") load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:truth.bzl", "matching") @@ -510,6 +511,24 @@ def _test_precompile_attr_inherit_pyc_collection_disabled_precompile_flag_enable _tests.append(_test_precompile_attr_inherit_pyc_collection_disabled_precompile_flag_enabled) +# buildifier: disable=function-docstring-header +def _test_precompile_enabled_succeeds(name): + """Verify that a `py_test` target actually builds and runs with + precompiling (the above `analysis_test`s only check declared providers). + """ + write_file( + name = name + "_main", + out = name + "_main.py", + ) + py_test( + name = name, + srcs = [name + "_main.py"], + main = name + "_main.py", + precompile = "enabled", + ) + +_tests.append(_test_precompile_enabled_succeeds) + def runfiles_contains_at_least_predicates(runfiles, predicates): for predicate in predicates: runfiles.contains_predicate(predicate) diff --git a/tools/precompiler/precompiler.py b/tools/precompiler/precompiler.py index f83dd15951..7c44a32631 100644 --- a/tools/precompiler/precompiler.py +++ b/tools/precompiler/precompiler.py @@ -34,7 +34,11 @@ def _create_parser() -> "argparse.Namespace": parser.add_argument("--persistent_worker", action="store_true") parser.add_argument("--log_level", default="ERROR") - parser.add_argument("--worker_impl", default="async") + # Bazel workers use anonymous pipes for stdio, which don't support + # overlapped I/O required by asyncio on Windows. + parser.add_argument( + "--worker_impl", default="serial" if sys.platform == "win32" else "async" + ) return parser @@ -167,7 +171,9 @@ async def _connect_streams( outstream: "typing.TextIO", # noqa: F821 ) -> "tuple[asyncio.StreamReader, asyncio.StreamWriter]": loop = asyncio.get_event_loop() - reader = asyncio.StreamReader() + # Cap reader at 4 MiB, leaving enough headroom over the default 64 KiB + # for request lines with numerous inputs (~470 KiB as of CPython 3.11). + reader = asyncio.StreamReader(limit=1 << 22) protocol = asyncio.StreamReaderProtocol(reader) await loop.connect_read_pipe(lambda: protocol, instream)