mirror of
https://github.com/electron/electron.git
synced 2026-10-28 11:44:46 +08:00
claude[bot]andClaude Fable 5.1
c7e11acc11
* fix: don't reap utilityProcess children in kill()
UtilityProcessWrapper::Kill() opened a second base::Process for the
child, sent SIGTERM and then handed it to base::EnsureProcessTerminated,
which on POSIX reaps the child itself (immediately via kqueue on macOS,
after a 2s delay on Linux). content's BrowserChildProcessHostImpl reaps
the same child once its mojo pipe drops, via
base::GetKnownDeadTerminationStatus. Whichever side lost that race
failed: content's kill(SIGKILL) hit ESRCH and its waitpid() hit ECHILD,
logging "ERROR:base/process/kill_posix.cc:87 Unable to terminate
process" and "kill_posix.cc:33 waitpid(): No child processes" (about
1,200 lines per main CI run), and the exit code content reported was
derived from a status that was already gone.
content reaps reliably on its own, so only keep base's helper on
Windows. On POSIX send SIGTERM and post a delayed SIGKILL that fires
only if the wrapper has not yet seen the child terminate, matching the
2s grace period base::EnsureProcessTerminated used. waitpid now reports
the signal, which HandleTermination already maps to exit code 0.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT
* fix: run the utilityProcess SIGKILL fallback off the UI thread
Review feedback on the previous commit: the delayed SIGKILL was posted to
the UI thread, so it could not run while main-process JavaScript kept
that sequence busy, and a child that ignores SIGTERM could outlive
kill() indefinitely. The base::EnsureProcessTerminated call it replaced
enforced its two-second deadline on a background thread.
Post the fallback to a ThreadPool sequence (MayBlock, BEST_EFFORT)
instead. The task takes only the pid and the child's
base::Process::CreationTime(), read before SIGTERM while the child is
certainly alive, and re-reads the creation time when it fires: once
content has reaped the child the pid yields a null time, and a reused
pid yields a different one, so neither is signalled. A not-yet-reaped
zombie matches and accepts SIGKILL harmlessly. No wrapper state is
touched off the UI thread, so nothing has to be marshalled back, and
the ForceKill member goes away.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT
* fix: align utilityProcess kill() with child_process.kill()
Kill() now only delivers the signal: base::Process::Terminate() sends
SIGTERM on POSIX and calls TerminateProcess on Windows, and nothing else
happens. The base::EnsureProcessTerminated call introduced in 489d3ac
(#34980) is gone on every platform, and with it the ThreadPool SIGKILL
fallback from the previous commit. EnsureProcessTerminated reaped the
child itself and SIGKILLed it after 2s, which raced content's
BrowserChildProcessHost reap (its kill()/waitpid() then failed with
ESRCH/ECHILD and logged two ERROR lines per kill) and force-killed a
child that handles SIGTERM. Like Node's child_process.kill(), delivery
is now all kill() guarantees; content reaps the child once its mojo
pipe drops.
The docs for child.kill() describe the new contract, with an API
history entry, and a POSIX spec forks a child whose SIGTERM handler
exits with 42 after 2.5s and asserts that kill() lets it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT
* docs: record the utilityProcess kill() change in breaking-changes.md
Apply review nits: drop the API-history block under `child.kill()`,
note the behavior change under Breaking API Changes (46.0) in
docs/breaking-changes.md instead, and say that the child is reaped by
Electron on POSIX.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFucMLGyVVqSUAKXrqbaUT
---------
Co-authored-by: Claude <noreply@anthropic.com>