From 4c63f7e3b0d726fc0979d177703790eaff81c6bc Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 13 Jul 2026 00:09:11 -0700 Subject: [PATCH] fix(sync): decode multibyte output correctly and settle on exit drain - Use StringDecoder to avoid corrupting multibyte chars (e.g. Japanese titles) split across stdout/stderr chunks - Settle a run within EXIT_DRAIN_MS (2s) after child exit when no terminal NDJSON arrives, instead of waiting on `close` that retained pipes can withhold indefinitely --- changes/sync-ui-window-close-quit.md | 3 +- src/main/runtime/sync-launcher-client.test.ts | 47 +++++++++++++++++++ src/main/runtime/sync-launcher-client.ts | 35 ++++++++++++-- 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/changes/sync-ui-window-close-quit.md b/changes/sync-ui-window-close-quit.md index 26726f5c..12e5ade6 100644 --- a/changes/sync-ui-window-close-quit.md +++ b/changes/sync-ui-window-close-quit.md @@ -2,4 +2,5 @@ type: fixed area: sync - Fixed the app staying alive in the background after closing a standalone Sync window (`subminer sync --ui`) on macOS. The quit re-issued after async will-quit cleanup was dropped by Electron when the cleanup settled within the same tick as the will-quit dispatch; the re-quit now runs on a fresh macrotask, and the standalone Sync window close path uses the same forced-exit fallback as SIGTERM. Windows opened from the tray menu of a running app are unaffected: closing them still leaves the app running. -- Fixed cancelling or timing out a sync run hanging when the sync child had already exited without emitting a terminal result: the run waited on `close`, which a descendant holding the inherited stdio pipes can delay indefinitely, stalling the quit-time sync shutdown. Cancellation now settles the run immediately once the child has exited. +- Fixed a sync run hanging when the sync child exited without emitting a terminal result: the run waited on `close`, which a descendant holding the inherited stdio pipes can delay indefinitely, stalling the quit-time sync shutdown. Cancelling now settles the run as soon as the child has exited, and an exit with no result settles after a bounded stdout drain window. +- Fixed sync progress and error text being corrupted when a multibyte character (e.g. a Japanese media title) straddled a chunk boundary in the sync child's output. diff --git a/src/main/runtime/sync-launcher-client.test.ts b/src/main/runtime/sync-launcher-client.test.ts index d326f71e..b100026d 100644 --- a/src/main/runtime/sync-launcher-client.test.ts +++ b/src/main/runtime/sync-launcher-client.test.ts @@ -55,6 +55,52 @@ test('runSyncLauncher parses NDJSON events across chunk boundaries', async () => assert.deepEqual(result, { ok: true, error: null }); }); +test('runSyncLauncher preserves multibyte characters split across chunks', async () => { + const { spawn, children } = makeSpawn(); + const events: SyncProgressEvent[] = []; + const handle = runSyncLauncher({ + command: ['subminer'], + args: ['sync', 'media-box', '--json'], + onEvent: (event) => events.push(event), + spawn, + }); + + const child = children[0]!; + const line = Buffer.from('{"type":"result","ok":false,"error":"進捗の同期に失敗"}\n'); + // Split mid-character: the boundary lands inside a multibyte code point. + const boundary = line.indexOf(Buffer.from('進')) + 1; + child.stdout.emit('data', line.subarray(0, boundary)); + child.stdout.emit('data', line.subarray(boundary)); + child.emit('close', 1, null); + + const result = await handle.done; + assert.deepEqual(events.at(-1), { + type: 'result', + ok: false, + error: '進捗の同期に失敗', + }); + assert.equal(result.error, '進捗の同期に失敗'); +}); + +test('runSyncLauncher settles after exit when close never arrives', async () => { + const { spawn, children } = makeSpawn(); + const handle = runSyncLauncher({ + command: ['subminer'], + args: ['sync', 'media-box', '--json'], + onEvent: () => {}, + spawn, + }); + const child = children[0]!; + child.stderr.emit('data', Buffer.from('[ERROR] remote refused\n')); + // The child is gone, but a descendant still holds the inherited stdio pipes, + // so `close` never fires. + child.emit('exit', 1, null); + + const result = await handle.done; + assert.equal(result.ok, false); + assert.match(result.error ?? '', /remote refused/); +}); + test('runSyncLauncher reports failures with the result event error or stderr tail', async () => { const { spawn, children } = makeSpawn(); const handle = runSyncLauncher({ @@ -213,6 +259,7 @@ test('runSyncLauncher times out a child that already exited without a result', a new Promise((resolve) => setTimeout(() => resolve(null), 25)), ]); assert.deepEqual(result, { ok: false, error: 'Sync operation timed out.' }); + assert.equal(child.killed, false); }); test('runSyncLauncher times out a child that never completes', async () => { diff --git a/src/main/runtime/sync-launcher-client.ts b/src/main/runtime/sync-launcher-client.ts index 9f462ed0..d7a1f771 100644 --- a/src/main/runtime/sync-launcher-client.ts +++ b/src/main/runtime/sync-launcher-client.ts @@ -1,10 +1,17 @@ import { spawn as nodeSpawn } from 'node:child_process'; +import { StringDecoder } from 'node:string_decoder'; import { parseSyncProgressLine, type SyncProgressEvent } from '../../shared/sync/sync-events'; import { SYNC_CLI_FLAG } from '../../core/services/stats-sync/cli-args'; /** How long a cancelled sync child gets to exit on SIGTERM before SIGKILL. */ const CANCEL_GRACE_MS = 5000; +/** + * How long a child that exited without terminal NDJSON gets to flush its + * remaining stdout before the exit is treated as authoritative. + */ +const EXIT_DRAIN_MS = 2000; + export interface SyncLauncherChildLike { stdout: { on(event: 'data', listener: (chunk: Buffer | string) => void): unknown } | null; stderr: { on(event: 'data', listener: (chunk: Buffer | string) => void): unknown } | null; @@ -77,8 +84,16 @@ export function runSyncLauncher(options: { let settleAfterTerminalEvent: (() => void) | null = null; let settleAfterTermination: (() => boolean) | null = null; + // Decode incrementally: a multibyte character can straddle a chunk boundary, + // and a per-chunk toString() would corrupt it (sync payloads carry Japanese + // media titles and error detail). + const stdoutDecoder = new StringDecoder('utf8'); + const stderrDecoder = new StringDecoder('utf8'); + const decodeChunk = (decoder: StringDecoder, chunk: Buffer | string): string => + typeof chunk === 'string' ? chunk : decoder.write(chunk); + child.stdout?.on('data', (chunk) => { - stdoutBuffer += chunk.toString(); + stdoutBuffer += decodeChunk(stdoutDecoder, chunk); let newlineIndex = stdoutBuffer.indexOf('\n'); while (newlineIndex !== -1) { const line = stdoutBuffer.slice(0, newlineIndex); @@ -95,18 +110,21 @@ export function runSyncLauncher(options: { } }); child.stderr?.on('data', (chunk) => { - const text = chunk.toString(); + const text = decodeChunk(stderrDecoder, chunk); stderrTail = `${stderrTail}${text}`.slice(-4000); options.onStderr?.(text); }); let killTimer: ReturnType | null = null; let operationTimer: ReturnType | null = null; + let drainTimer: ReturnType | null = null; const clearTimers = (): void => { if (killTimer !== null) clearTimeout(killTimer); if (operationTimer !== null) clearTimeout(operationTimer); + if (drainTimer !== null) clearTimeout(drainTimer); killTimer = null; operationTimer = null; + drainTimer = null; }; const done = new Promise((resolve) => { @@ -149,7 +167,18 @@ export function runSyncLauncher(options: { child.on('exit', (code) => { exitObserved = true; exitCode = code; - if (resultEvent || terminationError) settleFromExit(code); + if (resultEvent || terminationError) { + settleFromExit(code); + return; + } + // No terminal event yet: stdout may still be flushing, so give it a + // bounded window rather than waiting on a `close` that retained pipes + // can withhold forever. + drainTimer = setTimeout(() => { + drainTimer = null; + settleFromExit(code); + }, EXIT_DRAIN_MS); + drainTimer.unref?.(); }); child.on('close', settleFromExit); });