From e017154c75bebae30524da331163eae23ab1f932 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:46:58 +0800 Subject: [PATCH] fix(debugger): Java source paths on Windows and breakpoints before session start (#387) java-debug treated forward-slash Windows drive paths as URIs, so breakpoints never verified; source paths are now sent with native separators and mapped back for stack frames and breakpoint events. Debug sessions also flush every file's pending breakpoint updates before starting, so configurationDone never runs with an empty breakpoint snapshot (all adapters, launch and attach). Not verified on a real Windows machine; the path conversion is covered by unit tests. Fixes #380 Refs #374 --- crates/ide-dap/src/session.rs | 24 +++- crates/ide-dap/src/tests.rs | 62 ++++++++- .../__tests__/startupBreakpoints.test.ts | 131 ++++++++++++++++++ src/features/debugger/breakpoints/store.ts | 36 +++-- src/features/debugger/state.ts | 3 +- 5 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 src/features/debugger/__tests__/startupBreakpoints.test.ts diff --git a/crates/ide-dap/src/session.rs b/crates/ide-dap/src/session.rs index 345b8d90b..2ec511e83 100644 --- a/crates/ide-dap/src/session.rs +++ b/crates/ide-dap/src/session.rs @@ -104,6 +104,11 @@ impl Session { /// `path` as sent to the adapter (resolved for [`RESOLVES_PATHS`] adapters). pub fn adapter_path(&self, path: &Path) -> PathBuf { + if self.adapter == "java-debug" { + let native = java_source_path(&path.to_string_lossy()); + self.resolved_paths.lock().insert(native.clone(), path.to_path_buf()); + return native; + } if !RESOLVES_PATHS.contains(&self.adapter.as_str()) { return path.to_path_buf(); } @@ -118,7 +123,11 @@ impl Session { /// A path from the adapter as the IDE knows it (undoes [`Session::adapter_path`]). pub fn client_path(&self, path: &str) -> Option { - self.resolved_paths.lock().get(Path::new(path)).cloned() + let key = if self.adapter == "java-debug" { java_source_path(path) } else { PathBuf::from(path) }; + self.resolved_paths.lock().get(&key).cloned().or_else(|| { + (self.adapter == "java-debug" && is_windows_source_path(path)) + .then(|| PathBuf::from(path.replace('\\', "/"))) + }) } pub fn supports(&self, cap: &str) -> bool { @@ -245,6 +254,19 @@ impl Session { } } +// java-debug treats a forward-slash drive path as a URI (scheme "C"). +// Keep DAP path mode, but use native Windows separators for Java source lookup. +fn is_windows_source_path(path: &str) -> bool { + let bytes = path.as_bytes(); + (bytes.len() >= 3 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':' && matches!(bytes[2], b'/' | b'\\')) + || path.starts_with("//") + || path.starts_with(r"\\") +} + +fn java_source_path(path: &str) -> PathBuf { + if is_windows_source_path(path) { PathBuf::from(path.replace('/', r"\")) } else { PathBuf::from(path) } +} + fn with_conditions(v: &mut Value, condition: Option<&str>, hit_condition: Option<&str>) { if let Some(c) = condition.filter(|c| !c.trim().is_empty()) { v["condition"] = c.into(); diff --git a/crates/ide-dap/src/tests.rs b/crates/ide-dap/src/tests.rs index be4adefe6..e1247be71 100644 --- a/crates/ide-dap/src/tests.rs +++ b/crates/ide-dap/src/tests.rs @@ -15,6 +15,7 @@ struct Script { run_in_terminal: Option>, /// Send a `startDebugging` reverse request (child session) when a non-child `launch` arrives. start_debugging: bool, + source_path: Option, } struct FakeLauncher { @@ -139,14 +140,14 @@ async fn fake_adapter(stream json!({"type": "event", "event": "stopped", "body": {"reason": "breakpoint", "threadId": 1, "allThreadsStopped": true}}) ); send!( - json!({"type": "event", "event": "breakpoint", "body": {"reason": "changed", "breakpoint": {"id": 1, "verified": false, "message": "moved", "line": 9}}}) + json!({"type": "event", "event": "breakpoint", "body": {"reason": "changed", "breakpoint": {"id": 1, "verified": false, "message": "moved", "line": 9, "source": {"path": script.source_path}}}}) ); } "threads" => ok!(msg, json!({"threads": [{"id": 1, "name": "MainThread"}]})), "stackTrace" => ok!( msg, json!({"stackFrames": [ - {"id": 10, "name": "main", "line": 5, "column": 3, "source": {"path": "/p/a.py"}}, + {"id": 10, "name": "main", "line": 5, "column": 3, "source": {"path": script.source_path.as_deref().unwrap_or("/p/a.py")}}, {"id": 11, "name": "lib", "line": 1, "column": 1, "source": {"path": "/usr/lib/x.py", "presentationHint": "deemphasize"}}, ]}) ), @@ -1016,3 +1017,60 @@ fn root_and_address_placeholders() { assert_eq!(adapter::parse_address("[::1]:6006"), Some(("::1".into(), 6006))); assert_eq!(adapter::parse_address("6006"), None); } + +#[tokio::test] +async fn java_windows_source_paths_for_launch_and_attach() { + for request in ["launch", "attach"] { + for (client, native, returned) in [ + ("C:/work/Main.java", r"C:\work\Main.java", r"C:\work\Main.java"), + ("C:/work/Main.java", r"C:\work\Main.java", "C:/work/Main.java"), + (r"c:\work\Main.java", r"c:\work\Main.java", "c:/work/Main.java"), + (r"D:/my project\Main.java", r"D:\my project\Main.java", r"D:\my project\Main.java"), + ("//server/share/Main.java", r"\\server\share\Main.java", "//server/share/Main.java"), + (r"\\server\share/Main.java", r"\\server\share\Main.java", "//server/share/Main.java"), + ("/work/Main.java", "/work/Main.java", "/work/Main.java"), + ] { + let h = harness(Script { source_path: Some(returned.into()), ..Script::default() }); + let path = PathBuf::from(client); + h.manager.set_breakpoints(path.clone(), vec![bp(4)]).await.unwrap(); + let mut cfg = config(); + cfg.adapter = "java-debug".into(); + cfg.request = request.into(); + let id = h.start(cfg).await.unwrap(); + assert_eq!(h.request("initialize")["arguments"]["pathFormat"], "path"); + assert_eq!(h.request("setBreakpoints")["arguments"]["source"]["path"], native); + let commands = h.commands(); + assert!( + commands.iter().position(|c| c == "setBreakpoints").unwrap() + < commands.iter().position(|c| c == "configurationDone").unwrap() + ); + let frames = h.manager.stack_trace(id, 1).await.unwrap(); + assert_eq!(frames[0].path.as_deref(), Some(path.as_path())); + h.wait_event(|e| { + matches!(e, DebugEvent::BreakpointsChanged { path: p } if p.as_ref() == path.as_path()) + && h.manager.breakpoint_states(&path)[0].message.as_deref() == Some("moved") + }) + .await; + assert_eq!(h.manager.breakpoint_states(&path)[0].message.as_deref(), Some("moved")); + h.manager.stop(id).await.unwrap(); + } + } +} + +#[tokio::test] +async fn java_stack_frames_without_breakpoints_normalize_windows_paths() { + for (returned, expected) in [ + (r"C:\work\Other.java", "C:/work/Other.java"), + (r"\\server\share\Other.java", "//server/share/Other.java"), + ("C:/work/Other.java", "C:/work/Other.java"), + ("/work/Other.java", "/work/Other.java"), + ] { + let h = harness(Script { source_path: Some(returned.into()), ..Script::default() }); + let mut cfg = config(); + cfg.adapter = "java-debug".into(); + let id = h.start(cfg).await.unwrap(); + let frames = h.manager.stack_trace(id, 1).await.unwrap(); + assert_eq!(frames[0].path.as_deref(), Some(PathBuf::from(expected).as_path())); + h.manager.stop(id).await.unwrap(); + } +} diff --git a/src/features/debugger/__tests__/startupBreakpoints.test.ts b/src/features/debugger/__tests__/startupBreakpoints.test.ts new file mode 100644 index 000000000..452e61a66 --- /dev/null +++ b/src/features/debugger/__tests__/startupBreakpoints.test.ts @@ -0,0 +1,131 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { addBreakpoint, loadBreakpoints, removeAllBreakpoints, useBreakpoints } from "../breakpoints/store"; +import { startSession, useDebug } from "../state"; +import type { LaunchConfig } from "@/platform/ipc"; +import { + provideNotificationService, + provideToolWindowService, + type ToolWindowService, +} from "@/workbench/contributions"; + +const ipc = vi.hoisted(() => ({ + debugSetBreakpoints: vi.fn(), + debugStart: vi.fn(async () => ({ status: "ok", data: 42 })), + debugCapabilities: vi.fn(async () => ({ status: "ok", data: {} })), + debugExceptionFilters: vi.fn(async () => ({ status: "ok", data: [] })), +})); +vi.mock("@/platform/ipc", async (original) => { + const real = await original(); + return { ...real, isTauri: () => true, commands: { ...real.commands, ...ipc } }; +}); +const config = (request: string): LaunchConfig => ({ + name: "Java", + adapter: "java-debug", + request, + program: null, + args: [], + cwd: null, + env: {}, + extraJson: null, +}); +beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + localStorage.clear(); + useBreakpoints.setState({ root: "/p", byFile: {}, muted: false, status: {} }); + useDebug.setState({ sessions: {}, active: null, console: {} }); + provideNotificationService({ notify: vi.fn() }); + provideToolWindowService({ show: vi.fn() } as unknown as ToolWindowService); +}); +afterEach(() => vi.useRealTimers()); + +it.each(["launch", "attach"])("awaits every file before %s can send configurationDone", async (request) => { + const completed: string[] = []; + const release: Array<() => void> = []; + ipc.debugSetBreakpoints.mockImplementation( + (path: string) => + new Promise((resolve) => { + release.push(() => { + completed.push(path); + resolve({ status: "ok", data: [] }); + }); + }), + ); + addBreakpoint("/p/Main.java", 4); + addBreakpoint("/p/Other.java", 7); + const starting = startSession(config(request)); + await vi.advanceTimersByTimeAsync(0); + expect(ipc.debugSetBreakpoints).toHaveBeenCalledTimes(2); + expect(ipc.debugStart).not.toHaveBeenCalled(); + release[0]!(); + await vi.advanceTimersByTimeAsync(0); + expect(ipc.debugStart).not.toHaveBeenCalled(); + release[1]!(); + await starting; + expect(completed).toEqual(["/p/Main.java", "/p/Other.java"]); + expect(ipc.debugStart).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(120); + expect(ipc.debugSetBreakpoints).toHaveBeenCalledTimes(2); +}); + +it("drains an in-flight update and sends pending removals before startup", async () => { + let release!: () => void; + ipc.debugSetBreakpoints + .mockImplementationOnce( + () => + new Promise((resolve) => { + release = () => resolve({ status: "ok", data: [] }); + }), + ) + .mockResolvedValue({ status: "ok", data: [] }); + addBreakpoint("/p/Main.java", 4); + await vi.advanceTimersByTimeAsync(120); + removeAllBreakpoints(); + const starting = startSession(config("launch")); + await vi.advanceTimersByTimeAsync(0); + expect(ipc.debugStart).not.toHaveBeenCalled(); + release(); + await starting; + expect(ipc.debugSetBreakpoints).toHaveBeenLastCalledWith("/p/Main.java", []); + expect(ipc.debugStart).toHaveBeenCalledOnce(); +}); + +it("flushes persisted breakpoints immediately after project loading", async () => { + ipc.debugSetBreakpoints.mockResolvedValue({ status: "ok", data: [] }); + localStorage.setItem( + "openidea.debugger.breakpoints:/p", + JSON.stringify({ files: { "/p/Main.java": [{ line: 4 }] } }), + ); + loadBreakpoints("/p"); + await startSession(config("launch")); + expect(ipc.debugSetBreakpoints).toHaveBeenCalledOnce(); + expect(ipc.debugSetBreakpoints.mock.invocationCallOrder[0]).toBeLessThan(ipc.debugStart.mock.invocationCallOrder[0]!); +}); + +it("flushes edits made while another file is still syncing", async () => { + let release!: () => void; + ipc.debugSetBreakpoints + .mockImplementationOnce( + () => + new Promise((resolve) => { + release = () => resolve({ status: "ok", data: [] }); + }), + ) + .mockResolvedValue({ status: "ok", data: [] }); + addBreakpoint("/p/Main.java", 4); + const starting = startSession(config("launch")); + await vi.advanceTimersByTimeAsync(0); + addBreakpoint("/p/Other.java", 7); + release(); + await starting; + expect(ipc.debugSetBreakpoints).toHaveBeenCalledTimes(2); + expect(ipc.debugSetBreakpoints.mock.calls[1]![0]).toBe("/p/Other.java"); + expect(ipc.debugSetBreakpoints.mock.invocationCallOrder[1]).toBeLessThan(ipc.debugStart.mock.invocationCallOrder[0]!); +}); + +it("does not start if the initial breakpoint sync fails", async () => { + ipc.debugSetBreakpoints.mockRejectedValueOnce(new Error("breakpoint sync failed")); + addBreakpoint("/p/Main.java", 4); + expect(await startSession(config("launch"))).toBeNull(); + expect(ipc.debugStart).not.toHaveBeenCalled(); +}); diff --git a/src/features/debugger/breakpoints/store.ts b/src/features/debugger/breakpoints/store.ts index fcb0bf370..9899609ad 100644 --- a/src/features/debugger/breakpoints/store.ts +++ b/src/features/debugger/breakpoints/store.ts @@ -94,6 +94,7 @@ export function loadBreakpoints(root: string | null) { const timers = new Map>(); /** Ids in the order last sent per file (backend states are index-aligned). */ const sent = new Map(); +const inFlight = new Map>(); const SYNC_DELAY = 120; export function scheduleSync(path: string) { @@ -102,12 +103,35 @@ export function scheduleSync(path: string) { path, setTimeout(() => { timers.delete(path); - void syncFile(path); + void syncFile(path).catch((e) => console.warn("debugSetBreakpoints failed", e)); }, SYNC_DELAY), ); } -export async function syncFile(path: string) { +/** Drain debounced edits before the backend takes its startup breakpoint snapshot. */ +export async function flushBreakpoints() { + let paths = new Set([...timers.keys(), ...inFlight.keys(), ...Object.keys(useBreakpoints.getState().byFile)]); + while (paths.size) { + for (const timer of timers.values()) clearTimeout(timer); + timers.clear(); + await Promise.all([...paths].map((path) => syncFile(path))); + paths = new Set([...timers.keys(), ...inFlight.keys()]); + } +} + +export function syncFile(path: string): Promise { + const previous = inFlight.get(path) ?? Promise.resolve(); + const next = previous.catch(() => {}).then(() => sendFile(path)); + inFlight.set(path, next); + void next + .finally(() => { + if (inFlight.get(path) === next) inFlight.delete(path); + }) + .catch(() => {}); + return next; +} + +async function sendFile(path: string) { if (!isTauri()) return; const { byFile, muted } = useBreakpoints.getState(); const list = byFile[path] ?? []; @@ -115,12 +139,8 @@ export async function syncFile(path: string) { path, list.map((b) => b.id), ); - try { - const states = await call(commands.debugSetBreakpoints(path, toBackend(list, muted, path))); - applyStates(path, states); - } catch (e) { - console.warn("debugSetBreakpoints failed", e); - } + const states = await call(commands.debugSetBreakpoints(path, toBackend(list, muted, path))); + applyStates(path, states); } /** Re-reads verification states after a `breakpointsChanged` event. */ diff --git a/src/features/debugger/state.ts b/src/features/debugger/state.ts index 59e5f9888..5a8c36a6b 100644 --- a/src/features/debugger/state.ts +++ b/src/features/debugger/state.ts @@ -22,7 +22,7 @@ import { type Variable, } from "@/platform/ipc"; import { maybeEditorService, notifications, toolWindowService, useDebugRunSessions } from "@/workbench/contributions"; -import { breakpointsOf, consumeTemporary, refreshStates, resetStatuses } from "./breakpoints/store"; +import { breakpointsOf, consumeTemporary, flushBreakpoints, refreshStates, resetStatuses } from "./breakpoints/store"; import { interpolateLog, logParts, suspends } from "./breakpoints/model"; import { syncSpecial, useSpecialBreakpoints } from "./breakpoints/special"; import { refreshExceptionFilters } from "./breakpoints/exceptions"; @@ -306,6 +306,7 @@ export async function startSession( const startedAt = Date.now(); pendingStarts++; try { + await flushBreakpoints(); const id = await call(commands.debugStart(launch)); owned.add(id); useDebug.setState((s) => {