-
Notifications
You must be signed in to change notification settings - Fork 68
Fix/windows multi window run output leak #413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,12 @@ const LITHE_GITIGNORE_ENTRIES: &[&str] = &["run/local.json", "toolchains/local.j | |
|
|
||
| pub struct RunProcessManager; | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
| struct RunSessionKey { | ||
| window_label: String, | ||
| session_id: String, | ||
| } | ||
|
|
||
| struct RunningSession { | ||
| pid: u32, | ||
| stdin: Option<ChildStdin>, | ||
|
|
@@ -50,11 +56,18 @@ impl Default for RunProcessManager { | |
| } | ||
| } | ||
|
|
||
| fn sessions() -> &'static Mutex<HashMap<String, RunningSession>> { | ||
| static SESSIONS: OnceLock<Mutex<HashMap<String, RunningSession>>> = OnceLock::new(); | ||
| fn sessions() -> &'static Mutex<HashMap<RunSessionKey, RunningSession>> { | ||
| static SESSIONS: OnceLock<Mutex<HashMap<RunSessionKey, RunningSession>>> = OnceLock::new(); | ||
| SESSIONS.get_or_init(|| Mutex::new(HashMap::new())) | ||
| } | ||
|
|
||
| fn run_session_key(window_label: &str, session_id: &str) -> RunSessionKey { | ||
| RunSessionKey { | ||
| window_label: window_label.to_string(), | ||
| session_id: session_id.to_string(), | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct WriteGeneratedArgs { | ||
|
|
@@ -148,6 +161,7 @@ pub struct ResolvedLaunch { | |
| #[derive(Debug, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct StartProcessArgs { | ||
| pub window_label: String, | ||
| pub session_id: String, | ||
| pub executable: String, | ||
| pub arguments: Vec<String>, | ||
|
|
@@ -261,7 +275,10 @@ pub fn run_resolve_launch(args: ResolveLaunchArgs) -> Result<ResolvedLaunch, Str | |
|
|
||
| #[tauri::command] | ||
| pub fn run_start_process(app: AppHandle, args: StartProcessArgs) -> Result<(), String> { | ||
| stop_session(&args.session_id); | ||
| if args.window_label.trim().is_empty() { | ||
| return Err("A run process must be started from an active window.".into()); | ||
| } | ||
| stop_session(&args.window_label, &args.session_id); | ||
| let mut command = command_for_executable(&args.executable, &args.arguments); | ||
| command | ||
| .current_dir(&args.working_directory) | ||
|
|
@@ -277,15 +294,33 @@ pub fn run_start_process(app: AppHandle, args: StartProcessArgs) -> Result<(), S | |
| let stdin = child.stdin.take(); | ||
| let stdout = child.stdout.take(); | ||
| let stderr = child.stderr.take(); | ||
| let session_key = run_session_key(&args.window_label, &args.session_id); | ||
| sessions() | ||
| .lock() | ||
| .map_err(|_| "Run process state is unavailable".to_string())? | ||
| .insert(args.session_id.clone(), RunningSession { pid, stdin }); | ||
| .insert( | ||
| session_key, | ||
| RunningSession { | ||
| pid, | ||
| stdin, | ||
| }, | ||
| ); | ||
|
|
||
| let stdout_reader = spawn_output_reader(app.clone(), args.session_id.clone(), stdout); | ||
| let stderr_reader = spawn_output_reader(app.clone(), args.session_id.clone(), stderr); | ||
| let stdout_reader = spawn_output_reader( | ||
| app.clone(), | ||
| args.window_label.clone(), | ||
| args.session_id.clone(), | ||
| stdout, | ||
| ); | ||
| let stderr_reader = spawn_output_reader( | ||
| app.clone(), | ||
| args.window_label.clone(), | ||
| args.session_id.clone(), | ||
| stderr, | ||
| ); | ||
| spawn_exit_waiter( | ||
| app, | ||
| args.window_label, | ||
| args.session_id, | ||
| child, | ||
| pid, | ||
|
|
@@ -296,18 +331,18 @@ pub fn run_start_process(app: AppHandle, args: StartProcessArgs) -> Result<(), S | |
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn run_stop_process(session_id: String) -> Result<(), String> { | ||
| stop_session(&session_id); | ||
| pub fn run_stop_process(window_label: String, session_id: String) -> Result<(), String> { | ||
| stop_session(&window_label, &session_id); | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub fn run_write_stdin(session_id: String, input: String) -> Result<(), String> { | ||
| pub fn run_write_stdin(window_label: String, session_id: String, input: String) -> Result<(), String> { | ||
| let mut current = sessions() | ||
| .lock() | ||
| .map_err(|_| "Run process state is unavailable".to_string())?; | ||
| let session = current | ||
| .get_mut(&session_id) | ||
| .get_mut(&run_session_key(&window_label, &session_id)) | ||
| .ok_or_else(|| "The run process is no longer active.".to_string())?; | ||
| let stdin = session | ||
| .stdin | ||
|
|
@@ -1421,6 +1456,7 @@ fn runtime_version_parts(version: &str) -> Vec<u32> { | |
|
|
||
| fn spawn_output_reader<T: Read + Send + 'static>( | ||
| app: AppHandle, | ||
| window_label: String, | ||
| session_id: String, | ||
| stream: Option<T>, | ||
| ) -> thread::JoinHandle<()> { | ||
|
|
@@ -1434,7 +1470,8 @@ fn spawn_output_reader<T: Read + Send + 'static>( | |
| if !pending.is_empty() { | ||
| let chunk = decode_process_bytes(&pending); | ||
| if !chunk.is_empty() { | ||
| let _ = app.emit( | ||
| let _ = app.emit_to( | ||
| &window_label, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P0] 这里改成 触发场景:两个项目窗口都挂了 影响:会话复合键能避免进程互踩,但用户看到的「输出串到其他窗口」很可能还在。 建议:前端改成窗口作用域监听,和 menu 事件同一套做法,例如: const window = getCurrentWebviewWindow();
await window.listen("run-output", ...)
await window.listen("run-exit", ...)
|
||
| "run-output", | ||
| json!({ "sessionId": session_id, "chunk": chunk }), | ||
| ); | ||
|
|
@@ -1454,7 +1491,8 @@ fn spawn_output_reader<T: Read + Send + 'static>( | |
| if chunk.is_empty() { | ||
| continue; | ||
| } | ||
| let _ = app.emit( | ||
| let _ = app.emit_to( | ||
| &window_label, | ||
| "run-output", | ||
| json!({ "sessionId": session_id, "chunk": chunk }), | ||
| ); | ||
|
|
@@ -1467,6 +1505,7 @@ fn spawn_output_reader<T: Read + Send + 'static>( | |
|
|
||
| fn spawn_exit_waiter( | ||
| app: AppHandle, | ||
| window_label: String, | ||
| session_id: String, | ||
| mut child: Child, | ||
| pid: u32, | ||
|
|
@@ -1481,10 +1520,11 @@ fn spawn_exit_waiter( | |
| .unwrap_or(-1); | ||
| let _ = stdout_reader.join(); | ||
| let _ = stderr_reader.join(); | ||
| let session_key = run_session_key(&window_label, &session_id); | ||
| let stale = match sessions().lock() { | ||
| Ok(mut current) => match current.get(&session_id) { | ||
| Ok(mut current) => match current.get(&session_key) { | ||
| Some(session) if session.pid == pid => { | ||
| current.remove(&session_id); | ||
| current.remove(&session_key); | ||
| false | ||
| } | ||
| _ => true, | ||
|
|
@@ -1494,18 +1534,23 @@ fn spawn_exit_waiter( | |
| if stale { | ||
| return; | ||
| } | ||
| let _ = app.emit( | ||
| let _ = app.emit_to( | ||
| &window_label, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P0]
建议与 output 一并改成 |
||
| "run-exit", | ||
| json!({ "sessionId": session_id, "exitCode": exit_code }), | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| fn stop_session(session_id: &str) { | ||
| fn stop_session(window_label: &str, session_id: &str) { | ||
| let pid = sessions() | ||
| .lock() | ||
| .ok() | ||
| .and_then(|mut current| current.remove(session_id).map(|session| session.pid)); | ||
| .and_then(|mut current| { | ||
| current | ||
| .remove(&run_session_key(window_label, session_id)) | ||
| .map(|session| session.pid) | ||
| }); | ||
| if let Some(pid) = pid { | ||
| let mut command = Command::new("taskkill"); | ||
| command.args(["/F", "/T", "/PID", &pid.to_string()]); | ||
|
|
@@ -1878,10 +1923,18 @@ mod tests { | |
| assert!(looks_like_real_utf8(b"[INFO] BUILD SUCCESS")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn run_session_keys_are_scoped_by_window_label() { | ||
| let left = run_session_key("project-a", "primary"); | ||
| let right = run_session_key("project-b", "primary"); | ||
| assert_ne!(left, right); | ||
| assert_eq!(left, run_session_key("project-a", "primary")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stdin_write_rejects_an_inactive_session() { | ||
| let session_id = format!("missing-{}", std::process::id()); | ||
| let error = run_write_stdin(session_id, "input\n".to_string()).unwrap_err(); | ||
| let error = run_write_stdin("main".into(), session_id, "input\n".to_string()).unwrap_err(); | ||
| assert_eq!(error, "The run process is no longer active."); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { beforeEach, describe, expect, mock, test } from "bun:test"; | ||
|
|
||
| const eventHandlers = new Map<string, (event: { payload: unknown }) => void>(); | ||
| const windowListen = mock(async (event: string, handler: (event: { payload: unknown }) => void) => { | ||
| eventHandlers.set(event, handler); | ||
| return () => { | ||
| eventHandlers.delete(event); | ||
| }; | ||
| }); | ||
| const globalListen = mock(async () => () => {}); | ||
| const appendOutput = mock(() => undefined); | ||
| const finishProcess = mock(() => undefined); | ||
| const releaseMavenSessionWorkspace = mock(() => undefined); | ||
|
|
||
| mock.module("@tauri-apps/api/webviewWindow", () => ({ | ||
| getCurrentWebviewWindow: () => ({ | ||
| label: "workspace-1", | ||
| listen: windowListen, | ||
| }), | ||
| })); | ||
| mock.module("@tauri-apps/api/event", () => ({ listen: globalListen })); | ||
| mock.module("../stores/maven.store", () => ({ | ||
| mavenStoreForSession: () => ({ | ||
| getState: () => ({ | ||
| actions: { appendOutput, finishProcess }, | ||
| }), | ||
| }), | ||
| releaseMavenSessionWorkspace, | ||
| })); | ||
|
|
||
| const { ensureMavenProcessListeners } = await import("./use-maven-process-events"); | ||
|
|
||
| describe("maven process event listeners", () => { | ||
| beforeEach(() => { | ||
| appendOutput.mockClear(); | ||
| finishProcess.mockClear(); | ||
| releaseMavenSessionWorkspace.mockClear(); | ||
| }); | ||
|
|
||
| test("registers run-output and run-exit on the current webview window", async () => { | ||
| await ensureMavenProcessListeners(); | ||
|
|
||
| expect(windowListen).toHaveBeenCalledTimes(2); | ||
| expect(windowListen).toHaveBeenCalledWith("run-output", expect.any(Function)); | ||
| expect(windowListen).toHaveBeenCalledWith("run-exit", expect.any(Function)); | ||
| expect(globalListen).not.toHaveBeenCalled(); | ||
| expect(eventHandlers.has("run-output")).toBe(true); | ||
| expect(eventHandlers.has("run-exit")).toBe(true); | ||
| }); | ||
|
|
||
| test("ignores non-maven session output", () => { | ||
| const outputHandler = eventHandlers.get("run-output"); | ||
| expect(outputHandler).toBeDefined(); | ||
|
|
||
| outputHandler?.({ | ||
| payload: { sessionId: "primary", chunk: "leaked\n" }, | ||
| }); | ||
|
|
||
| expect(appendOutput).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test("routes maven session output through the maven store", () => { | ||
| const outputHandler = eventHandlers.get("run-output"); | ||
| expect(outputHandler).toBeDefined(); | ||
|
|
||
| outputHandler?.({ | ||
| payload: { sessionId: "maven:task-1", chunk: "BUILD SUCCESS\n" }, | ||
| }); | ||
|
|
||
| expect(appendOutput).toHaveBeenCalledWith("maven:task-1", "BUILD SUCCESS\n"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { beforeEach, describe, expect, mock, test } from "bun:test"; | ||
|
|
||
| const invoke = mock(async () => undefined); | ||
| const getCurrentWebviewWindow = mock(() => ({ label: "project-window" })); | ||
|
|
||
| mock.module("@/platform/tauri-core", () => ({ invoke })); | ||
| mock.module("@tauri-apps/api/webviewWindow", () => ({ getCurrentWebviewWindow })); | ||
|
|
||
| const { startRunProcess, stopRunProcess, writeRunStdin } = await import("../api/run-host-api"); | ||
|
|
||
| describe("run host API window scoping", () => { | ||
| beforeEach(() => { | ||
| invoke.mockClear(); | ||
| getCurrentWebviewWindow.mockClear(); | ||
| }); | ||
|
|
||
| test("startRunProcess includes the current window label", async () => { | ||
| await startRunProcess({ | ||
| sessionId: "primary", | ||
| executable: "go.exe", | ||
| arguments: ["run", "."], | ||
| workingDirectory: "D:\\demo", | ||
| environment: {}, | ||
| }); | ||
|
|
||
| expect(invoke).toHaveBeenCalledWith("run_start_process", { | ||
| args: { | ||
| sessionId: "primary", | ||
| executable: "go.exe", | ||
| arguments: ["run", "."], | ||
| workingDirectory: "D:\\demo", | ||
| environment: {}, | ||
| windowLabel: "project-window", | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] 这个测试能确认 invoke 带上了 现在 CI 全绿也说明不了串窗已修好(尤其是 listener 仍是全局 建议补一条更贴场景的断言,例如 mock/验证 run、maven 的 listener 使用了当前 window label(或 |
||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| test("stop and stdin commands include the current window label", async () => { | ||
| await stopRunProcess("primary"); | ||
| await writeRunStdin("primary", "input\n"); | ||
|
|
||
| expect(invoke).toHaveBeenNthCalledWith(1, "run_stop_process", { | ||
| windowLabel: "project-window", | ||
| sessionId: "primary", | ||
| }); | ||
| expect(invoke).toHaveBeenNthCalledWith(2, "run_write_stdin", { | ||
| windowLabel: "project-window", | ||
| sessionId: "primary", | ||
| input: "input\n", | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3]
windows/tauri/src-tauri/src/run.rs:43RunSessionKey { window_label, session_id }这个建模很干净,直接对准了多窗共用"primary"会互踩进程的根因,后续 debug 会话也可以按同一复合键扩展。这部分建议原样保留。