fix(session): use forceDestroy() to reap orphaned Chromium on init failure#630
Closed
m7fz7 wants to merge 10 commits into
Closed
fix(session): use forceDestroy() to reap orphaned Chromium on init failure#630m7fz7 wants to merge 10 commits into
m7fz7 wants to merge 10 commits into
Conversation
…ilure start()'s crash-recovery path tore down a half-built engine with a graceful destroy(), but a failed engine.initialize() usually means the underlying browser/CDP connection is already broken (e.g. a TargetCloseError: Target closed mid script-injection). destroy() has nothing live to talk to in that case, so it can only time out via teardownEngineSafely's 10s race, leaving the Chromium process alive and orphaned. Every such crash left one more process behind, eventually starving the host of memory. Use forceDestroy() instead, the same SIGKILL-the-process recovery POST /:id/force-kill already uses for a wedged engine -- the exact state this catch block is handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
SessionService.start()'s crash-recovery path tore down a half-built engine with a gracefuldestroy(), but a failedengine.initialize()usually means the browser/CDP connection is already broken (e.g.TargetCloseError: Target closedmid script-injection).destroy()has nothing live to talk to in that case — it can only time out viateardownEngineSafely's 10s race and give up, leaving the Chromium process alive and orphaned. Every such crash leaves one more zombie process behind, eventually exhausting host memory and causing subsequent start attempts to fail too (including"The browser is already running for <userDataDir>"from the still-held profile lock).Switched that one teardown call to
forceDestroy()— the same SIGKILL-the-process recoveryPOST /:id/force-killalready uses for a wedged engine, which is exactly the state this catch block is handling.Type of Change
Checklist
Screenshots (if applicable)
N/A — backend-only change, no UI surface.