[shoumikhin]

What is wrong today

Two threads can hang forever when a backend logs while holding a lock. XNNPACK does this when a load fails.

During a binding call, each runtime log line goes to sys.stderr at once, and a Python write can let another thread take the GIL (Python's global interpreter lock). If that thread then waits for that lock, the first thread never gets the GIL back:

thread A: load_method -> takes lock L -> logs -> sys.stderr.write releases the GIL
thread B: takes the GIL -> execute -> waits for lock L, holding the GIL
thread A: waits for the GIL forever

What this change does

The log function no longer calls Python. While sys.stderr is the stream Python opened on fd 2, each line goes straight there through C stdio, so it shows at once, even before a crash.

When Python has replaced sys.stderr (capsys, redirect_stderr, Jupyter), a call keeps its lines and writes them there as it returns, with no backend lock held. On Linux and macOS, it does the same once fd 2 is a pipe that stays full for 100 ms, so a pipe that a Python thread drains (wurlitzer.pipes()) cannot hang the call. On macOS, pipes up to 16 KiB count as full at 512 bytes, because macOS can report room they do not have. A call keeps at most its newest 64 KiB. If writing to sys.stderr fails, the lines go to fd 2 instead. A fatal line and the lines waiting before it go out while there is room, since an abort follows.

On Linux with Python before 3.13.8, a daemon thread never writes through Python, since Python can end it at exit mid-write and crash the process, as it does on main.

Tests

New tests cover the hang, daemon threads at exit, failed and interrupted writes, fatal lines, a busy terminal, a full pipe drained by a thread, the 64 KiB cap, and cut error messages. Ran the pybindings tests on Linux x86_64 (the quantized-ops tests cannot run in this build) and the XNNPACK case from two threads.

Also fixed: an error message with invalid UTF-8, such as a corrupt program's identifier or a path cut inside a character, raised UnicodeDecodeError instead of RuntimeError; such bytes are now escaped. The docs now name the right namespace for registering a PAL (platform abstraction layer).

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24393

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

⏳ No Failures, 102 Pending

As of commit 5e58f45 with merge base 822cb60 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[shoumikhin]

@claude /pr-review

[claude]

Claude finished @shoumikhin's task in 20m 51s —— View job

---

PR Review: #24393

Needs Discussion: this removes the two-thread deadlock, but the new fd-2 sink can still hang a Jupyter kernel on Linux or macOS when one call logs more than the pipe holds, and it loses Python-side log capture in more places than the README says; writing the calling thread's lines out after the call returns would fix both.

Detailed review

Summary

The PR writes runtime logs to the process's stderr with C stdio instead of sys.stderr, so a backend that logs while holding its own lock can no longer hand the GIL to a thread that waits for that lock. Four problems remain:

• another way to hang, in Jupyter on Linux and macOS
• Python-side log capture is lost in more places than documented
• a docs page is now out of date
• the new test can miss failures on its worker thread

Thread Safety

• extension/pybindings/pybindings.cpp:5346: in Jupyter on Linux and macOS, this write can still block while the GIL is held.
  • ipykernel 6+ points fd 2 at a pipe. A Python thread (OutStream._watch_pipe_fd) empties that pipe, and it needs the GIL after every read.
  • The bindings hold the GIL for the whole load_method/execute. Two things follow:
    • Runtime lines reach the notebook only after the call returns, and may show up after output the cell prints later. They do not show "right away" as the description says.
    • If one call logs more than the pipe holds (64 KiB by default on Linux), fwrite blocks in write(2) forever. While blocked it holds the GIL, the stdio lock on stderr, and in the motivating case the backend lock. Holding the stdio lock means every other native thread that logs blocks too. This hangs even a single-threaded notebook.
  • What triggers it: a Debug-log build, or a delegate that forwards verbose library logs to ET_LOG.
  • Other tools that hang the same way: wurlitzer, or any capture that pairs a pipe with a Python reader thread. The README's advice to "capture file descriptor 2" points users toward these. capfd is safe, because it writes to a temp file.
  • The old path went through OutStream.write, which only buffers in memory.
  • Suggested direction:
    • While runtime_error_scopes != 0, have emit_log_message append the formatted lines to a plain thread_local buffer, the way it already fills runtime_errors.
    • Write those lines to sys.stderr from the outermost ~RuntimeErrorScope. The call guard is destroyed after the bound function returns, so at that point the GIL is held and no backend lock or ObjectLock is.
    • This avoids both the original deadlock and this one, and keeps Python-side capture working (see Backward Compatibility).
    • Costs:
      • Lines appear when the call returns, not while it runs.
      • kFatal lines should still go straight to fd 2, because an abort follows.
      • The destructor can run during stack unwinding, so it must swallow Python errors.
  • I did not reproduce this. It is based on how ipykernel's OutStream captures fds. Fix this →

Backward Compatibility

• extension/pybindings/pybindings.cpp:5346, extension/pybindings/README.md:238-242: Python-side capture is lost in more places than the description and README say.
  • The description names only contextlib.redirect_stderr. These break the same way:
    • pytest capsys
    • unittest --buffer: logs are no longer buffered, and they are not attached to failure reports
    • IPython %%capture: in a Linux or macOS notebook, the fd watcher forwards the lines, so they bypass %%capture and still print
    • anything else that installs its own sys.stderr
  • Jupyter on Windows is affected too. ipykernel does not capture fds there, so runtime logs go to the server's console instead of the notebook.
  • In all of these cases, Info and Warning lines are gone. The exception keeps only the text of Error-level messages, cut off at kRuntimeErrorsCapacity (1024 bytes; pybindings.cpp:138, :5333-5345). The README's "includes the error lines" does not mention this limit.
  • If the fd-2 design stays, the README note should list these cases and the 1 KiB limit. Fix this →

API Design

• docs/source/using-executorch-runtime-integration.md:42-64 is now out of date. It still presents the pybindings sink as the example "where logs are redirected to show up properly in a Python notebook environment". Its sample writes to std::cerr, which the new comment at pybindings.cpp:5319-5322 rules out in a Python process. The PR updates the README but not this page. Fix this →

Testing

• extension/pybindings/test/test_pybindings.py:431-468: failures on the worker thread are not reported.
  • run() runs on a bare threading.Thread, and nothing collects its exception.
  • Suppose method(inputs) raises after the load has returned, for example because the failed init left the shared workspace unusable. The script still prints "no deadlock" and the test passes.
  • Suppose instead it raises while SlowStderr is installed. The traceback goes into SlowStderr, which throws the text away, so the test fails showing only a writes count.
  • test_torch_inputs_are_safe_across_threads in the same file reports worker errors through future.result(). Do the same here, and have SlowStderr keep its text so the assertion message can show it. Fix this →

Recommendation

Needs Discussion

The fd-2 sink fixes the two-thread deadlock, but it leaves a hang in a single call in Linux and macOS notebooks. It also loses Python-side capture in more places than documented. Writing the calling thread's lines from the outermost ~RuntimeErrorScope would fix both without releasing the GIL. If the fd-2 design stays, the README and the docs page need the corrections above.

---

• Gather context (diff, commits, CLAUDE.md, review checklist)
• Run three independent reviewers over the diff
• Consolidate findings
• Fact-check findings
• Post review

Not done: I did not build or run anything. The ipykernel fd-capture behavior comes from its implementation and was not reproduced here.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[shoumikhin]

Thanks, the latest push does this. Fatal lines and lines logged outside a call still go straight to the process's stderr. The stderr redirect tests are back to their original form, and the docs page and the deadlock test are updated as suggested.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[shoumikhin]

@claude /pr-review

[claude]

Claude finished @shoumikhin's task in 31m 19s —— View job

---

PR Review: #24393

Request Changes: buffering a call's log lines fixes the deadlock, but a call that crashes or hangs now loses its logs even in a plain terminal, where nothing needs the buffering; buffering only when sys.stderr has been replaced would keep both.

Detailed review

Summary

The PR keeps the runtime log lines of each binding call and writes them to sys.stderr when the outermost call returns. A backend that logs while holding its own lock therefore can no longer hand the GIL to a thread that waits for that lock.

One major problem: a call that crashes or hangs loses its lines, even when nothing captures sys.stderr. The rest are minor:

• The lines of an interrupted write are either lost or written twice, depending on a condition unrelated to the write.
• An out-of-memory guard does not cover the allocation most likely to fail.
• Several new paths are untested.
• The README's 64 KiB figure is misleading.
• A daemon thread may abort the process at exit (low confidence).

Code Quality

• extension/pybindings/pybindings.cpp:251-254: the lines of an interrupted write are either lost or written twice.
  • Lost: with no exception saved earlier, a KeyboardInterrupt from sys.stderr.write sets written = true. If the writer raised before writing anything, the lines reach no stream at all. test_pybindings.py:555-558 asserts exactly this.
  • Written twice: if PyErr_CheckSignals() already saved an exception (one Ctrl-C during the call, a second during the write), the condition is false and written stays false. Line 261 then writes every line to fd 2, even if the interrupted write already printed some. The comment at 248-250 says those lines are not written again.
  • Fix: decide written separately from saving the exception, and use one rule for both cases. For failure logs, a possible duplicate costs less than a loss. Fix this →
• extension/pybindings/pybindings.cpp:228-231: new (std::nothrow) does not make this construction out-of-memory safe.
  • It covers only the outer allocation. pybind11's error_already_set() (2.10 and later) also allocates its fetched-error state with a plain new inside the constructor.
  • Line 228 is outside the try, in a noexcept function, so a bad_alloc from that inner allocation still ends in std::terminate.
  • Fix: drop std::nothrow and the nullptr branch. That is simpler, which CLAUDE.md prefers, and no worse.
  • This is based on pybind11's source; the submodule is not checked out here. Fix this →

Testing

• extension/pybindings/test/test_pybindings.py:380-558: most of the new paths have no test.
  • Ctrl-C during a call (PyErr_CheckSignals() at pybindings.cpp:227-232). The description promises "A Ctrl-C during a call reaches the caller after the call's lines", but the new test only covers a write that raises.
  • The 64 KiB cut and its left-out note (:165-175, :235-237).
  • write_fatal_context (:183-195, :5499-5504), which is now the only way a call's lines survive an abort.
  • The backslashreplace decoding (:240-241). This one is cheap to cover:
    • _load_program_from_buffer(b"\xff" * 128) logs the raw bytes through Program identifier '%.4s' (runtime/executor/program.cpp:170).
    • With a StringIO installed as sys.stderr, the test should get those bytes back escaped. Fix this →

API Design

• extension/pybindings/README.md:240-241: "keeps its last 64 KiB" overstates what is kept.
  • Whenever the lines pass 64 KiB, keep_line (pybindings.cpp:165-175) cuts them to at most 32 KiB.
  • So a call that logs more than 64 KiB keeps between about 32 and 64 KiB. A call that logs 66 KiB shows only about its last 33 KiB. Fix this →

Thread Safety

• extension/pybindings/pybindings.cpp:245, :256, :260: a daemon thread can abort the process at exit (low confidence).

  • What changed: ~RuntimeErrorScope now runs code that releases the GIL:

    • the sys.stderr.write at 245, since a file write drops the GIL around the system call;
    • the py::gil_scoped_release at 260.

    Before this PR, the destructor only copied runtime_errors back.

  • Why it aborts: on CPython 3.13 and earlier, a thread that retakes the GIL after another thread has started Py_Finalize is ended with pthread_exit, which glibc runs as a forced unwind.

    • At 245, the catch (...) at 256 swallows that unwind, and glibc then aborts with FATAL: exception not rethrown.
    • At 260, the unwind starts inside ~gil_scoped_release, which is noexcept, so it calls std::terminate.

  • Trigger: a daemon thread whose binding call logged something is inside that write when the main thread exits. CPython 3.14 hangs such threads instead of ending them.

  • Confidence: not reproduced; this comes from CPython's take_gil and glibc's pthread_exit. I don't see a simple fix inside a noexcept destructor, so this may only need a note.

Backward Compatibility

• extension/pybindings/pybindings.cpp:5495-5505, README.md:244: a call that crashes or hangs now loses its log lines, even when nothing captures sys.stderr.

  • What changed: during a binding call, every non-fatal line goes only into PendingLogs and leaves at line 293, when the call returns. Before this PR it went to sys.stderr at once, so a plain python script.py, pytest -s or CI log showed it right away.

  • Triggers:

    • a delegate logs an error, then segfaults or aborts without ET_LOG(Fatal) (for example a vendor SDK abort(), or std::terminate);
    • the process is killed;
    • the call hangs.

    These are the usual situations in which someone reads a delegate's log.

  • Why it is avoidable: the buffering only matters when a line has to go through Python. While sys.stderr is still the original stream, lines can go to fd 2 directly. That is the C stdio write the PR already uses for lines logged outside a call, and it cannot hand off the GIL.

  • Suggested fix:

    • When the outermost RuntimeErrorScope starts (GIL held), compare PySys_GetObject("stderr") with PySys_GetObject("__stderr__"). Both are borrowed lookups that run no Python code.
    • If they match, write each line to fd 2 as it is logged.
    • Otherwise sys.stderr has been replaced, and only then should lines be buffered. capsys, redirect_stderr, unittest --buffer, %%capture and ipykernel's OutStream all replace it, so Python-side capture and the Jupyter pipe case keep the buffered path.
    • This still leaves one exposure: fd-level capture that leaves sys.stderr alone, such as wurlitzer.pipes(), would take the direct path. Lines logged outside a call already do.

  • The earlier review suggested buffering every line and missed this cost. Fix this →

Recommendation

Request Changes

Losing the logs of a call that crashes or hangs in a plain terminal is avoidable: buffer only when sys.stderr has been replaced. The other items are minor.

---

• Gather context (diff, commits, CLAUDE.md, review checklist)
• Run three independent reviewers over the diff
• Consolidate findings
• Fact-check findings
• Post review

Not done: I did not build or run anything. pybind11 is not checked out here, so the pybind11, CPython and glibc behavior cited above comes from their sources, not from a run.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[shoumikhin]

Thanks, reworked along those lines. While sys.stderr is the process's own stderr, each runtime log line now goes straight to it as it is logged, so a crash or hang no longer loses it. Lines wait for the call to return only when Python has replaced sys.stderr, or, on Linux and macOS, when the process's stderr is a pipe that stays full for a tenth of a second, as when a Python thread in the same process drains it under wurlitzer.pipes().

A write that fails or is cut short now always falls back to the process's stderr, even if that repeats some lines, and a Ctrl-C raised in it is raised again once the call returns. The saved exception and its allocation are gone. A fatal line is now just that line, written at once while stderr has room. The README states what a long call keeps. Tests cover the direct writes, a full pipe, the cap with its note and escaping, a failed or interrupted write, and the fatal line, which a too-small ETDump debug buffer triggers.

The daemon thread crash at exit is real. On Linux, Python 3.13 and earlier end a daemon thread at exit with pthread_exit when it waits for the GIL. If that happens inside a write through Python, the unwinding releases Python objects without the GIL. The main branch crashes this way too, with a segfault in its log function. So a daemon thread now never writes its lines through Python; they go straight to the process's stderr. In a script whose daemon threads keep logging while Python exits, the main branch crashed in all 30 runs with sys.stderr left alone and in 10 of 30 with it replaced. With this change, none of the 60 runs crashed, and a test covers it.

[shoumikhin]

@claude /pr-review

[claude]

Claude finished @shoumikhin's task in 46m 51s —— View job

---

Request Changes: the daemon-thread rule applies on platforms and Python versions where the crash it guards against cannot happen, a fatal line leaves behind the kept lines that explain it, and the new UTF-8 escaping leaves the main error message raw; each needs a small change.

Detailed review

PR Review: #24393

Summary

The PR stops the pybindings log callback from calling Python. A backend that logs while holding its own lock can no longer deadlock with a thread that waits for that lock. Lines now go to fd 2 through C stdio, or wait until the outermost binding call returns. Three problems below need small changes before merge; five more are minor.

Code Quality

• A fatal line goes out alone, and the kept lines that explain it die with the abort (extension/pybindings/pybindings.cpp:5522-5526). The call's lines start to wait once sys.stderr is replaced or a pipe fills. A later fatal line then goes to fd 2 at once while there is room. The earlier lines stay in pending_logs, and the abort() that usually follows loses them, although they carry the context for the abort. The README accepts that a crash loses waiting lines, but here the code knows the abort is coming. Fix: before the fatal line, write the kept lines to fd 2 while there is room, as the daemon branch does (252-261). Fix this →

• The UTF-8 escaping covers only the appended runtime errors (pybindings.cpp:287-301). result(message) stays raw. THROW_IF_ERROR and THROW_INDEX_IF_ERROR (77-95) cut the message at 128 bytes with snprintf. pybind11 sets the exception message with PyErr_SetString, which decodes strictly. So a non-ASCII path cut mid-character still raises UnicodeDecodeError instead of RuntimeError or IndexError. Example: loader_from_file (2241-2245) with a long CJK path that does not exist. The last commit fixes this same error, but only for the appended part. Fix: decode the whole message once with backslashreplace. Fix this →

• One 100 ms wait on a full pipe holds every later line of the call (pybindings.cpp:5522). direct = false stays set for the rest of the call. Under 2>&1 | tee or a CI log pipe, a single stall by the reader is enough. The later lines then wait in memory: past 64 KiB the oldest are dropped, and a crash or hang in the call loses all of them. On main they were written. Fix: when a full pipe caused the wait, rather than a replaced sys.stderr, check again with a zero timeout on later lines. Once there is room, write the kept lines and go back to direct writes. Fix this →

• A KeyboardInterrupt from the deferred write is replayed as a new SIGINT, not re-raised (pybindings.cpp:272-277).

  • PyErr_SetInterrupt runs the Python SIGINT handler again, so a custom handler that raises runs twice for one Ctrl-C.
  • If SIGINT was ignored when the process started, for example in a background job (cmd &) of a non-interactive shell, Python installs no handler and the call does nothing. In such an environment the test's assert interrupted (test_pybindings.py:607) fails.
  • Other exceptions a signal handler raises in that write are still swallowed, as on main. One example is SystemExit from a SIGTERM handler that calls sys.exit().

  Fix (low confidence, since pybind11 is not checked out here): keep the error_already_set and rethrow it from ~RuntimeErrorScope when no exception is unwinding. Fix this →

API Design

• The runtime-integration guide repeats the bindings' routing rules, and already gets daemon threads wrong (docs/source/using-executorch-runtime-integration.md:74-80). It says a waiting line "goes to sys.stderr". On a daemon thread it goes to fd 2 instead. A second copy of these rules will drift from the README Note and from emit_log_message. Fix: keep the deadlock rationale and leave the routing details to the existing README link. Fix this →

Backward Compatibility

• The daemon-thread rule applies on every OS and Python version, but the crash it avoids needs Linux and Python 3.13 or earlier (pybindings.cpp:187-198, used by RuntimeErrorScope).

  • The code comment and the README limit the crash to Linux.
  • Python 3.14 hangs such a thread instead of calling pthread_exit (pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful python/cpython#87135), and ExecuTorch supports 3.11 to 3.15 (pyproject.toml:53-60).
  • Everywhere else, a daemon thread's lines skip a replaced sys.stderr. They escape capsys and redirect_stderr. On Windows, where ipykernel does not capture fd 2, they never reach the notebook.
  • Daemon threads are common: multiprocessing.pool.ThreadPool workers, ThreadingHTTPServer handlers, and Thread(daemon=True) in notebooks.
  • Their kept lines reach fd 2 only while it has room (252-261). The rest are dropped without the note that keep_line adds.
  • test_pybindings.py:553 asserts writes through Python: 0 on every platform, which locks this in.

  Fix: return false except on Linux with Python < 3.14, and scope the README and the test to match. If the rule is meant to be broader, the comment should give a reason that holds on the other platforms. Fix this →

• Lines logged outside a binding call now skip sys.stderr, even while the thread holds the GIL (pybindings.cpp:5515).

  • On main they went through sys.stderr.write. Now they escape capsys, never reach a notebook on Windows, and are written with no room check.
  • On Linux and macOS, that write can block on a full kernel pipe while it holds the GIL. The README documents this as a hang.
  • The README lists destructors and other modules as "outside a call". The ExecuTorchResult and CUDA stream bindings (5063-5085, 5097-5181) also omit call_guard, and the README does not mention them.
  • to_host and from_host reach CUDAGuard::create, which can log (extension/cuda/device_guard.cpp:25-34).

  Fix: guard those bindings. For a line logged outside a call on a thread that holds the GIL, drop the line rather than block when the pipe has no room. Fix this →

• Non-ASCII runtime lines on a Windows console now show in the console's code page (pybindings.cpp:160-163). write_to_stderr hands UTF-8 bytes to fwrite. On main, a line logged with the GIL held went through sys.stderr, whose console writer handles Unicode. Non-ASCII text, such as a path in a loader error, is now garbled. The README documents this, but it is still a regression. Fix: when fd 2 is a console, convert to UTF-16 and write with WriteConsoleW. Fix this →

Recommendation

Request Changes. Three fixes are needed before merge, and each is a few lines: limit the daemon-thread rule to Linux with Python < 3.14, write the kept lines before a fatal line, and escape the whole exception message. The other five items can land in a follow-up.

• Gather context
• Run three independent reviewers
• Consolidate findings
• Fact-check findings
• Post review

Not done: I did not build or run anything. pybind11 is not checked out here, so the pybind11, CPython and ipykernel behavior cited above comes from their sources and docs rather than this checkout.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[shoumikhin]

Thanks. Changed in this push:

• Daemon threads: the rule now applies only on Linux with Python before 3.13.8, checked at run time. Later versions leave such a thread waiting instead of ending it, and other platforms do not unwind it. The README and the test match.
• Fatal lines: the lines a call kept before a fatal line now go to the process's stderr first, while there is room. The test checks the line logged just before the fatal one.
• Exception text: the whole message is now escaped. A new test loads paths whose message is cut inside a character.
• The runtime-integration guide keeps only the reason and links to the README.
• macOS: when pipe memory runs low, macOS gives a new pipe as little as 512 bytes but still reports room in it, so a write there can wait forever. That likely hung the daemon test on macOS. A pipe that has not grown past 16 KiB now counts as full once it holds 512 bytes.
• The interrupted-write test installs Python's SIGINT handler itself, so it also passes where SIGINT starts ignored.

Not changed:

• After a stall, the kept lines still arrive when the call returns. Going back to direct writes needs a second state and a per-line check.
• Re-raising the original exception needs a destructor that throws. The main branch swallows every exception raised in that write.
• Writing lines from outside a call through Python can deadlock when a backend logs under its lock. Dropping them after a wait loses lines whenever a reader stalls. The CUDA stream and result bindings log only when a CUDA call fails, a few lines at most.
• The Windows console code page is out of scope here; the README documents it.

[shoumikhin]

@claude /pr-review

[claude]

PR Review: #24393

• Gather context (diff, commits, CLAUDE.md, review checklist)
• Run three independent reviewers over the diff
• Consolidate findings
• Fact-check findings
• Post review

View job run