diff --git a/mateclaw-server/src/main/java/vip/mate/tool/browser/BrowserLauncher.java b/mateclaw-server/src/main/java/vip/mate/tool/browser/BrowserLauncher.java index 2a4335a2..99e08315 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/browser/BrowserLauncher.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/browser/BrowserLauncher.java @@ -282,14 +282,14 @@ public class BrowserLauncher { ? browser.newContext() : browser.contexts().get(0); Page page = context.pages().isEmpty() ? context.newPage() : context.pages().get(0); - // The temp profile dir is owned by the spawned Chrome for the lifetime of the - // browser; clean up when the JVM exits since we don't track per-session shutdown - // here. (BrowserUseTool stops the browser explicitly on stop, so the JVM hook is - // a backstop for crashes.) - registerProfileDirCleanup(userDataDir); long elapsed = System.currentTimeMillis() - t0; trace.add(Attempt.ok(Strategy.EXTERNAL_CDP, browserBin + " + connectOverCDP(" + cdpBase + ")", elapsed)); - return Result.success(browser, context, page, true, cdpBase, Strategy.EXTERNAL_CDP, trace); + // Hand ownership of `proc` and `userDataDir` to the caller — the session that + // consumes this Result is responsible for destroyForcibly() on the process and + // deleteQuietly() on the dir when it stops. Spring's @PreDestroy on BrowserUseTool + // closes every active session on shutdown, so a clean JVM exit will not leak. + return Result.successOwned(browser, context, page, cdpBase, Strategy.EXTERNAL_CDP, trace, + userDataDir, proc); } catch (Exception e) { proc.destroyForcibly(); deleteQuietly(userDataDir); @@ -299,8 +299,28 @@ public class BrowserLauncher { } } - private static void deleteQuietly(Path dir) { + /** + * Best-effort recursive delete with a single short retry. Per-file failures are + * swallowed so a single locked file (Windows: Chrome leaves lockfiles open briefly + * after exit) does not abort cleanup of the rest of the directory. + * + *

The retry is necessary because Chrome's GPU process and segmentation_platform + * DB take a few hundred milliseconds longer than the parent chrome.exe to flush + * and release file handles even after destroyForcibly() — without the retry, + * ~6 files (ShaderCache, ukm_db) are typically left on disk per session on Windows. + * + *

Public so {@code BrowserSession.close()} can reuse it without duplicating logic. + */ + public static void deleteQuietly(Path dir) { if (dir == null) return; + deleteOnce(dir); + if (!Files.exists(dir)) return; + // Second pass: give lingering Chrome subprocesses ~500ms to release locks. + try { Thread.sleep(500); } catch (InterruptedException e) { Thread.currentThread().interrupt(); return; } + deleteOnce(dir); + } + + private static void deleteOnce(Path dir) { try { if (!Files.exists(dir)) return; try (var stream = Files.walk(dir)) { @@ -311,11 +331,6 @@ public class BrowserLauncher { } catch (Exception ignored) {} } - private static void registerProfileDirCleanup(Path dir) { - Runtime.getRuntime().addShutdownHook(new Thread(() -> deleteQuietly(dir), - "mateclaw-cdp-profile-cleanup")); - } - // ==================== Helpers ==================== private BrowserType.LaunchOptions baseLaunchOptions(boolean headed) { @@ -551,10 +566,25 @@ public class BrowserLauncher { private final List attempts; private final boolean success; private final String failureSummary; + /** + * Temp profile directory created for {@link Strategy#EXTERNAL_CDP}. + * Null for every other strategy (caller does not own the user data). + * The session that consumes this Result is responsible for deleting the + * directory after it stops the browser; {@link BrowserLauncher#deleteQuietly} + * is provided as the canonical implementation. + */ + private final Path userDataDir; + /** + * The Chrome subprocess we spawned for {@link Strategy#EXTERNAL_CDP}. + * Null otherwise. The session must {@code destroyForcibly()} it on stop — + * closing the Playwright {@code Browser} only severs the CDP connection. + */ + private final Process ownedProcess; private Result(Browser browser, BrowserContext context, Page page, boolean connectedViaCdp, String cdpUrl, Strategy strategy, - List attempts, boolean success, String failureSummary) { + List attempts, boolean success, String failureSummary, + Path userDataDir, Process ownedProcess) { this.browser = browser; this.context = context; this.page = page; @@ -564,15 +594,24 @@ public class BrowserLauncher { this.attempts = attempts; this.success = success; this.failureSummary = failureSummary; + this.userDataDir = userDataDir; + this.ownedProcess = ownedProcess; } static Result success(Browser browser, BrowserContext context, Page page, boolean cdp, String cdpUrl, Strategy strategy, List attempts) { - return new Result(browser, context, page, cdp, cdpUrl, strategy, attempts, true, null); + return new Result(browser, context, page, cdp, cdpUrl, strategy, attempts, true, null, null, null); + } + + static Result successOwned(Browser browser, BrowserContext context, Page page, + String cdpUrl, Strategy strategy, List attempts, + Path userDataDir, Process ownedProcess) { + return new Result(browser, context, page, true, cdpUrl, strategy, attempts, true, null, + userDataDir, ownedProcess); } static Result failure(List attempts, String summary) { - return new Result(null, null, null, false, null, null, attempts, false, summary); + return new Result(null, null, null, false, null, null, attempts, false, summary, null, null); } } } diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/BrowserUseTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/BrowserUseTool.java index 51760895..d8c57a90 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/BrowserUseTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/BrowserUseTool.java @@ -257,7 +257,8 @@ public class BrowserUseTool { } BrowserSession session = new BrowserSession(r.getBrowser(), r.getContext(), r.getPage(), - headed, r.isConnectedViaCdp(), r.getCdpUrl()); + headed, r.isConnectedViaCdp(), r.getCdpUrl(), + r.getUserDataDir(), r.getOwnedProcess()); sessions.put(sessionKey, session); scheduleIdleCheck(sessionKey); @@ -297,8 +298,10 @@ public class BrowserUseTool { return error("Failed to connect to CDP at " + cdpUrl + ": " + r.getFailureSummary()); } + // action=connect_cdp attaches to a user-managed Chrome — we did not spawn it, + // so userDataDir / ownedProcess stay null and close() will only disconnect. BrowserSession session = new BrowserSession(r.getBrowser(), r.getContext(), r.getPage(), - true, true, r.getCdpUrl()); + true, true, r.getCdpUrl(), null, null); sessions.put(sessionKey, session); scheduleIdleCheck(sessionKey); @@ -791,18 +794,33 @@ public class BrowserUseTool { final boolean headed; final boolean connectedViaCdp; final String cdpUrl; + /** + * Temp profile directory we created for the EXTERNAL_CDP fallback. + * Null when the session connected to a user-managed Chrome (CONFIG_CDP / + * action=connect_cdp) or used a non-CDP launch strategy. + */ + final java.nio.file.Path userDataDir; + /** + * Chrome subprocess we spawned ourselves for EXTERNAL_CDP. Null otherwise. + * Closing the Playwright {@code Browser} only severs the CDP socket; the + * actual Chrome process keeps running until we destroyForcibly() it here. + */ + final Process ownedProcess; volatile long lastActivity; /** 空闲看门狗定时任务(stop 时取消,避免泄漏) */ volatile ScheduledFuture idleWatchdog; BrowserSession(Browser browser, BrowserContext context, Page page, - boolean headed, boolean connectedViaCdp, String cdpUrl) { + boolean headed, boolean connectedViaCdp, String cdpUrl, + java.nio.file.Path userDataDir, Process ownedProcess) { this.browser = browser; this.context = context; this.page = page; this.headed = headed; this.connectedViaCdp = connectedViaCdp; this.cdpUrl = cdpUrl; + this.userDataDir = userDataDir; + this.ownedProcess = ownedProcess; this.lastActivity = System.currentTimeMillis(); } @@ -815,15 +833,40 @@ public class BrowserUseTool { } /** - * 关闭浏览器会话(不关闭共享 Playwright)。 - * CDP 模式:仅断开连接,Chrome 进程继续运行。 - * Launch 模式:关闭 context + browser(终止 Chromium 进程)。 + * Close the session (does not touch the shared Playwright driver). + *

*/ void close() { if (connectedViaCdp) { try { if (browser != null) browser.close(); } catch (Exception ignored) {} + if (ownedProcess != null) { + try { + // Snapshot descendants BEFORE killing the parent. Chrome on Windows + // spawns ~6 child processes (renderer, GPU, network service, ...) + // that hold open handles inside the user-data-dir. destroyForcibly() + // only sends TerminateProcess to the parent; killing children must + // be done separately, otherwise the temp dir cannot be deleted and + // the orphaned chrome.exe instances keep running. + java.util.List children = ownedProcess.descendants() + .toList(); + ownedProcess.destroyForcibly(); + for (ProcessHandle h : children) { + try { h.destroyForcibly(); } catch (Exception ignored) {} + } + ownedProcess.waitFor(5, TimeUnit.SECONDS); + for (ProcessHandle h : children) { + try { h.onExit().get(2, TimeUnit.SECONDS); } catch (Exception ignored) {} + } + } catch (Exception ignored) {} + BrowserLauncher.deleteQuietly(userDataDir); + } } else { try { if (context != null) context.close();