diff --git a/crates/ide-host/src/ops/java.rs b/crates/ide-host/src/ops/java.rs index 8f05763d4..adb8c63d3 100644 --- a/crates/ide-host/src/ops/java.rs +++ b/crates/ide-host/src/ops/java.rs @@ -235,11 +235,18 @@ pub async fn java_build_workspace( host: &Host, window: &str, module: PathBuf, + request: Option, ) -> IdeResult { host.require_trust(window)?; let project = host.project(window)?; let module = project.root.join(module); - project.lsp.java_build(&module, &CancellationToken::new()).await + // HotSwap may cancel the compile (the request id is allocated by the caller). + let cancel = request.map_or_else(CancellationToken::new, |id| host.begin_request(id)); + let result = project.lsp.java_build(&module, &cancel).await; + if let Some(id) = request { + host.end_request(id); + } + result } /// Commands of this module the remote server answers. @@ -249,7 +256,9 @@ pub(crate) fn register(d: &mut Dispatcher) { .await }); d.add("java_debug_server", |c| async move { java_debug_server(&c.host, &c.window, c.arg("module")?).await }); - d.add("java_build_workspace", |c| async move { java_build_workspace(&c.host, &c.window, c.arg("module")?).await }); + d.add("java_build_workspace", |c| async move { + java_build_workspace(&c.host, &c.window, c.arg("module")?, c.arg("request")?).await + }); } #[cfg(test)] diff --git a/src-tauri/src/commands/java.rs b/src-tauri/src/commands/java.rs index c27fa0dd3..cc148d4fc 100644 --- a/src-tauri/src/commands/java.rs +++ b/src-tauri/src/commands/java.rs @@ -39,6 +39,7 @@ pub async fn java_build_workspace( window: Window, state: State<'_, AppState>, module: PathBuf, + request: Option, ) -> IdeResult { - ide_host::ops::java::java_build_workspace(&state.host, window.label(), module).await + ide_host::ops::java::java_build_workspace(&state.host, window.label(), module, request).await } diff --git a/src/bindings.ts b/src/bindings.ts index 5000d6847..446f3f58a 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -780,7 +780,7 @@ export const commands = { /** Port of java-debug's DAP server inside the jdtls serving `module` (started on demand). */ javaDebugServer: (module: string) => typedError(__TAURI_INVOKE("java_debug_server", { module })), /** Incremental build of the jdtls workspace serving `module` (e.g. before HotSwap). */ - javaBuildWorkspace: (module: string) => typedError(__TAURI_INVOKE("java_build_workspace", { module })), + javaBuildWorkspace: (module: string, request: number | null) => typedError(__TAURI_INVOKE("java_build_workspace", { module, request })), /** Starts a session owned by this window: its events (and those of child sessions) go here only. */ debugStart: (config: LaunchConfig) => typedError(__TAURI_INVOKE("debug_start", { config })), debugSetBreakpoints: (path: string, breakpoints: Breakpoint[]) => typedError(__TAURI_INVOKE("debug_set_breakpoints", { path, breakpoints })), diff --git a/src/features/debugger/__tests__/hotswap.test.ts b/src/features/debugger/__tests__/hotswap.test.ts index 6ce4e5a6f..dc0b2ad25 100644 --- a/src/features/debugger/__tests__/hotswap.test.ts +++ b/src/features/debugger/__tests__/hotswap.test.ts @@ -4,7 +4,8 @@ const ipc = vi.hoisted(() => { const ok = (data: T) => Promise.resolve({ status: "ok" as const, data }); return { ok, - javaBuildWorkspace: vi.fn(() => ok("succeed" as string)), + cancelRequest: vi.fn((_request: number) => Promise.resolve()), + javaBuildWorkspace: vi.fn((_module: string, _request?: number) => ok("succeeded" as string)), debugRedefineClasses: vi.fn(() => ok({ changedClasses: ["com.x.A", "com.x.B"], errorMessage: null as string | null }), ), @@ -27,10 +28,16 @@ vi.mock("@/workbench/contributions", async (importOriginal) => { }; }); -import type { DebugCapabilities, DebugSessionId } from "@/platform/ipc"; +import { IpcError, type DebugCapabilities, type DebugSessionId } from "@/platform/ipc"; import { getContext } from "@/platform/context"; import { _resetI18nForTests, setLocale } from "@/platform/i18n"; -import { hotSwapMenuItems, hotSwapMessage, onHotCodeReplace, reloadChangedClasses } from "../hotswap"; +import { + cancelHotSwapBuild, + hotSwapMenuItems, + hotSwapMessage, + onHotCodeReplace, + reloadChangedClasses, +} from "../hotswap"; import { publishContext, useDebug, type DebugSession } from "../state"; function session(id: number, hotCodeReplace: boolean): DebugSession { @@ -67,8 +74,12 @@ function session(id: number, hotCodeReplace: boolean): DebugSession { describe("HotSwap", () => { beforeEach(() => { notes.length = 0; + ipc.cancelRequest.mockClear(); ipc.javaBuildWorkspace.mockClear(); - ipc.debugRedefineClasses.mockClear(); + ipc.debugRedefineClasses.mockReset(); + ipc.debugRedefineClasses.mockImplementation(() => + ipc.ok({ changedClasses: ["com.x.A", "com.x.B"], errorMessage: null }), + ); useDebug.setState({ sessions: { 1: session(1, true), 2: session(2, false) }, active: 1 }); publishContext(); }); @@ -89,7 +100,7 @@ describe("HotSwap", () => { it("builds, reloads in Java sessions and reports the count", async () => { await reloadChangedClasses(); - expect(ipc.javaBuildWorkspace).toHaveBeenCalledWith("/p/app"); + expect(ipc.javaBuildWorkspace).toHaveBeenCalledWith("/p/app", expect.any(Number)); expect(ipc.debugRedefineClasses).toHaveBeenCalledTimes(1); expect(ipc.debugRedefineClasses).toHaveBeenCalledWith(1); expect(notes.at(-1)).toMatchObject({ level: "info", title: "2 classes reloaded" }); @@ -105,6 +116,102 @@ describe("HotSwap", () => { expect(ipc.debugRedefineClasses).toHaveBeenCalledWith(2); }); + it("does not reload after a cancelled build", async () => { + ipc.javaBuildWorkspace.mockImplementationOnce(() => ipc.ok("cancelled")); + await reloadChangedClasses(); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(notes.at(-1)).toMatchObject({ level: "info", title: "Hot Swap cancelled" }); + }); + + it("treats a cancellation error as cancellation and requires a restart before reloading", async () => { + ipc.javaBuildWorkspace.mockRejectedValueOnce(new IpcError({ kind: "cancelled", message: "cancelled" })); + await reloadChangedClasses(); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(notes.at(-1)).toMatchObject({ level: "info", title: "Hot Swap cancelled" }); + expect(getContext().debugHotSwapBuilding).toBe(false); + cancelHotSwapBuild(); + expect(ipc.cancelRequest).not.toHaveBeenCalled(); + await reloadChangedClasses(); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(notes.at(-1)).toMatchObject({ + level: "error", + title: "Hot Swap failed", + body: expect.stringContaining("Restart"), + }); + useDebug.setState({ sessions: { 1: session(1, true) } }); + await reloadChangedClasses(); + expect(ipc.debugRedefineClasses).toHaveBeenCalledExactlyOnceWith(1); + }); + + it("keeps the first request when reloads overlap and stops at the cancelled module", async () => { + const two = session(3, true); + two.launch = { ...two.launch!, cwd: "/p/other" }; + useDebug.setState({ sessions: { 1: session(1, true), 3: two } }); + let finish!: (result: { status: "ok"; data: string }) => void; + ipc.javaBuildWorkspace.mockImplementationOnce(() => new Promise((resolve) => (finish = resolve))); + const pending = reloadChangedClasses(); + await vi.waitFor(() => expect(ipc.javaBuildWorkspace).toHaveBeenCalledTimes(1)); + const request = ipc.javaBuildWorkspace.mock.calls[0]![1]; + expect(getContext().debugHotSwapBuilding).toBe(true); + expect(notes.at(-1)).toMatchObject({ title: "Compiling changed classes…", actions: [expect.any(Object)] }); + await reloadChangedClasses(); + await reloadChangedClasses(false); + expect(ipc.javaBuildWorkspace).toHaveBeenCalledTimes(1); + cancelHotSwapBuild(); + cancelHotSwapBuild(); + expect(ipc.cancelRequest).toHaveBeenCalledExactlyOnceWith(request); + // A success response racing the cancel must not reload partially compiled classes. + finish({ status: "ok", data: "succeeded" }); + await pending; + expect(ipc.javaBuildWorkspace).toHaveBeenCalledTimes(1); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(notes.at(-1)).toMatchObject({ level: "info", title: "Hot Swap cancelled" }); + expect(getContext().debugHotSwapBuilding).toBe(false); + cancelHotSwapBuild(); + expect(ipc.cancelRequest).toHaveBeenCalledTimes(1); + await reloadChangedClasses(); + expect(ipc.javaBuildWorkspace).toHaveBeenCalledTimes(3); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(useDebug.getState().sessions[3]?.hotSwapNeedsRestart).toBe(true); + }); + + it("does not report a partial reload after edit, cancel, edit, cancel, reload", async () => { + // The cancelled builds wrote OrderBook.class, but the next build only reports OrderBookTest. + ipc.javaBuildWorkspace.mockResolvedValueOnce({ status: "ok", data: "cancelled" }); + await reloadChangedClasses(); + ipc.javaBuildWorkspace.mockResolvedValueOnce({ status: "ok", data: "cancelled" }); + await reloadChangedClasses(); + ipc.debugRedefineClasses.mockResolvedValueOnce({ + status: "ok", + data: { changedClasses: ["com.example.orders.OrderBookTest"], errorMessage: null }, + }); + await reloadChangedClasses(); + await reloadChangedClasses(false); + expect(ipc.javaBuildWorkspace).toHaveBeenCalledTimes(3); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(notes.slice(-2)).toEqual([ + expect.objectContaining({ level: "error", title: "Hot Swap failed", body: expect.stringContaining("Restart") }), + expect.objectContaining({ level: "error", title: "Hot Swap failed", body: expect.stringContaining("Restart") }), + ]); + }); + + it("invalidates all live Java sessions when a configuration's workspace build is cancelled", async () => { + const one = { ...session(1, true), runConfig: { id: "one" } as NonNullable }; + const two = { ...session(3, true), runConfig: { id: "two" } as NonNullable }; + useDebug.setState({ sessions: { 1: one, 2: session(2, false), 3: two } }); + ipc.javaBuildWorkspace.mockResolvedValueOnce({ status: "ok", data: "cancelled" }); + await reloadChangedClasses(true, "one"); + await reloadChangedClasses(false, "two"); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + expect(useDebug.getState().sessions[2]?.hotSwapNeedsRestart).toBeUndefined(); + useDebug.setState((state) => ({ sessions: { ...state.sessions, 4: session(4, true) } })); + await reloadChangedClasses(false); + expect(ipc.debugRedefineClasses).not.toHaveBeenCalled(); + useDebug.setState({ sessions: { 4: session(4, true) } }); + await reloadChangedClasses(false); + expect(ipc.debugRedefineClasses).toHaveBeenCalledExactlyOnceWith(4); + }); + it("does not reload after a failed build", async () => { ipc.javaBuildWorkspace.mockImplementationOnce(() => ipc.ok("failed")); await reloadChangedClasses(); diff --git a/src/features/debugger/actions.ts b/src/features/debugger/actions.ts index 05fb556b7..c7d1ccddb 100644 --- a/src/features/debugger/actions.ts +++ b/src/features/debugger/actions.ts @@ -29,7 +29,7 @@ import { removeAllSpecial, specialAt, toggleFieldAt, toggleMethodAt } from "./br import { fieldAt, methodAt } from "./breakpoints/symbols"; import { toggleBreakpointSmart } from "./breakpoints/toggle"; import { isJavaAttachTarget, isJavaDebuggable, javaAttachConfig, javaLaunchConfig } from "./java"; -import { reloadChangedClasses } from "./hotswap"; +import { cancelHotSwapBuild, reloadChangedClasses } from "./hotswap"; import { debugUnsupportedReason, launchConfigFor } from "./launch"; import { isPythonAttachTarget, @@ -449,6 +449,15 @@ export function registerDebugActions(): () => void { : undefined, ), }, + { + id: "debug.cancelHotSwap", + title: "Cancel Hot Swap Build", + description: "Stop the compile a Reload Changed Classes action is waiting on", + category: "Debug", + icon: "stop", + when: "debugHotSwapBuilding", + run: () => cancelHotSwapBuild(), + }, { id: "debug.rerun", title: "Rerun", diff --git a/src/features/debugger/hotswap.ts b/src/features/debugger/hotswap.ts index e4e039232..38a13114b 100644 --- a/src/features/debugger/hotswap.ts +++ b/src/features/debugger/hotswap.ts @@ -5,7 +5,17 @@ * a save) java-debug reports `BUILD_COMPLETE` and, like IDEA's default * "Reload classes after compilation: Ask", we offer the reload. */ -import { call, commands, isTauri, type DebugEvent, type DebugSessionId } from "@/platform/ipc"; +import { + call, + commands, + isTauri, + isCancelled, + nextRequestId, + type DebugEvent, + type DebugSessionId, + type RequestId, +} from "@/platform/ipc"; +import { setContext } from "@/platform/context"; import { maybeEditorService, notifications } from "@/workbench/contributions"; import { isLive, useDebug, type DebugSession } from "./state"; import { tx } from "@/platform/i18n"; @@ -14,6 +24,16 @@ import type { MenuItem } from "@/ui/Menu"; const KEY = "debug.hotswap"; /** `BUILD_COMPLETE` events of our own build are not worth a prompt. */ let quietUntil = 0; +/** The compile request HotSwap is waiting on, so it can be cancelled. */ +let activeBuild: { request: RequestId; cancelled: boolean } | null = null; +let reloading = false; + +/** Cancels a running HotSwap compile; the reload is aborted rather than applied to stale classes. */ +export function cancelHotSwapBuild(): void { + if (activeBuild === null || activeBuild.cancelled) return; + activeBuild.cancelled = true; + void commands.cancelRequest(activeBuild.request).catch(() => {}); +} export function hotSwapSessions(): DebugSession[] { return Object.values(useDebug.getState().sessions).filter((s) => isLive(s) && !!s.caps?.hotCodeReplace); @@ -40,11 +60,13 @@ export function hotSwapMessage( /** Compiles (unless `build` is false) and reloads changed classes into every Java debug session. */ export async function reloadChangedClasses(build = true, configId?: string): Promise { + if (reloading) return; const sessions = hotSwapSessions().filter((s) => !configId || s.runConfig?.id === configId); if (!sessions.length || !isTauri()) { notifications().notify({ group: "run", level: "info", key: KEY, title: "No debug session can reload classes" }); return; } + reloading = true; quietUntil = Date.now() + 60_000; try { if (build) { @@ -55,8 +77,42 @@ export async function reloadChangedClasses(build = true, configId?: string): Pro } const modules = new Set(sessions.map((s) => s.launch?.cwd ?? "")); for (const module of modules) { - const status = await call(commands.javaBuildWorkspace(module)).catch(() => null); - if (status === "failed" || status === "withErrors") { + const request = nextRequestId(); + const compile = { request, cancelled: false }; + activeBuild = compile; + setContext("debugHotSwapBuilding", true); + notifications().notify({ + group: "run", + level: "info", + key: KEY, + title: "Compiling changed classes…", + sticky: true, + actions: [{ label: "Cancel Hot Swap Build", run: cancelHotSwapBuild }], + }); + let status: "cancelled" | "failed" | "succeeded" | "withErrors" | null = null; + try { + status = await call(commands.javaBuildWorkspace(module, request)); + } catch (e) { + status = isCancelled(e) ? "cancelled" : null; + } finally { + activeBuild = null; + setContext("debugHotSwapBuilding", false); + } + if (compile.cancelled || status === "cancelled") { + // JDT can write class files before cancellation without publishing their + // build deltas. java-debug cannot recover those classes on the next build. + useDebug.setState((state) => ({ + sessions: Object.fromEntries( + Object.entries(state.sessions).map(([id, s]) => [ + id, + isLive(s) && s.caps?.hotCodeReplace ? { ...s, hotSwapNeedsRestart: true } : s, + ]), + ), + })); + notifications().notify({ group: "run", level: "info", key: KEY, title: "Hot Swap cancelled" }); + return; + } + if (status === "failed" || status === "withErrors" || status === null) { notifications().notify({ group: "run", level: "error", @@ -68,6 +124,16 @@ export async function reloadChangedClasses(build = true, configId?: string): Pro } } } + if (sessions.some((s) => useDebug.getState().sessions[s.id]?.hotSwapNeedsRestart)) { + notifications().notify({ + group: "run", + level: "error", + key: KEY, + title: "Hot Swap failed", + body: "A cancelled build may have left classes out of the reload. Restart the debug session to apply all changes.", + }); + return; + } const changed = new Set(); const errors: string[] = []; for (const s of sessions) { @@ -82,6 +148,7 @@ export async function reloadChangedClasses(build = true, configId?: string): Pro const m = hotSwapMessage(changed.size, errors); notifications().notify({ group: "run", key: KEY, ...m }); } finally { + reloading = false; quietUntil = Date.now() + 2_000; } } @@ -91,7 +158,7 @@ export function onHotCodeReplace(session: DebugSessionId, event: Extract