mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-07-27 04:49:49 -07:00
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
This commit is contained in:
@@ -2,4 +2,5 @@ type: fixed
|
|||||||
area: sync
|
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 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.
|
||||||
|
|||||||
@@ -55,6 +55,52 @@ test('runSyncLauncher parses NDJSON events across chunk boundaries', async () =>
|
|||||||
assert.deepEqual(result, { ok: true, error: null });
|
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 () => {
|
test('runSyncLauncher reports failures with the result event error or stderr tail', async () => {
|
||||||
const { spawn, children } = makeSpawn();
|
const { spawn, children } = makeSpawn();
|
||||||
const handle = runSyncLauncher({
|
const handle = runSyncLauncher({
|
||||||
@@ -213,6 +259,7 @@ test('runSyncLauncher times out a child that already exited without a result', a
|
|||||||
new Promise<null>((resolve) => setTimeout(() => resolve(null), 25)),
|
new Promise<null>((resolve) => setTimeout(() => resolve(null), 25)),
|
||||||
]);
|
]);
|
||||||
assert.deepEqual(result, { ok: false, error: 'Sync operation timed out.' });
|
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 () => {
|
test('runSyncLauncher times out a child that never completes', async () => {
|
||||||
|
|||||||
@@ -1,10 +1,17 @@
|
|||||||
import { spawn as nodeSpawn } from 'node:child_process';
|
import { spawn as nodeSpawn } from 'node:child_process';
|
||||||
|
import { StringDecoder } from 'node:string_decoder';
|
||||||
import { parseSyncProgressLine, type SyncProgressEvent } from '../../shared/sync/sync-events';
|
import { parseSyncProgressLine, type SyncProgressEvent } from '../../shared/sync/sync-events';
|
||||||
import { SYNC_CLI_FLAG } from '../../core/services/stats-sync/cli-args';
|
import { SYNC_CLI_FLAG } from '../../core/services/stats-sync/cli-args';
|
||||||
|
|
||||||
/** How long a cancelled sync child gets to exit on SIGTERM before SIGKILL. */
|
/** How long a cancelled sync child gets to exit on SIGTERM before SIGKILL. */
|
||||||
const CANCEL_GRACE_MS = 5000;
|
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 {
|
export interface SyncLauncherChildLike {
|
||||||
stdout: { on(event: 'data', listener: (chunk: Buffer | string) => void): unknown } | null;
|
stdout: { on(event: 'data', listener: (chunk: Buffer | string) => void): unknown } | null;
|
||||||
stderr: { 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 settleAfterTerminalEvent: (() => void) | null = null;
|
||||||
let settleAfterTermination: (() => boolean) | 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) => {
|
child.stdout?.on('data', (chunk) => {
|
||||||
stdoutBuffer += chunk.toString();
|
stdoutBuffer += decodeChunk(stdoutDecoder, chunk);
|
||||||
let newlineIndex = stdoutBuffer.indexOf('\n');
|
let newlineIndex = stdoutBuffer.indexOf('\n');
|
||||||
while (newlineIndex !== -1) {
|
while (newlineIndex !== -1) {
|
||||||
const line = stdoutBuffer.slice(0, newlineIndex);
|
const line = stdoutBuffer.slice(0, newlineIndex);
|
||||||
@@ -95,18 +110,21 @@ export function runSyncLauncher(options: {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
child.stderr?.on('data', (chunk) => {
|
child.stderr?.on('data', (chunk) => {
|
||||||
const text = chunk.toString();
|
const text = decodeChunk(stderrDecoder, chunk);
|
||||||
stderrTail = `${stderrTail}${text}`.slice(-4000);
|
stderrTail = `${stderrTail}${text}`.slice(-4000);
|
||||||
options.onStderr?.(text);
|
options.onStderr?.(text);
|
||||||
});
|
});
|
||||||
|
|
||||||
let killTimer: ReturnType<typeof setTimeout> | null = null;
|
let killTimer: ReturnType<typeof setTimeout> | null = null;
|
||||||
let operationTimer: ReturnType<typeof setTimeout> | null = null;
|
let operationTimer: ReturnType<typeof setTimeout> | null = null;
|
||||||
|
let drainTimer: ReturnType<typeof setTimeout> | null = null;
|
||||||
const clearTimers = (): void => {
|
const clearTimers = (): void => {
|
||||||
if (killTimer !== null) clearTimeout(killTimer);
|
if (killTimer !== null) clearTimeout(killTimer);
|
||||||
if (operationTimer !== null) clearTimeout(operationTimer);
|
if (operationTimer !== null) clearTimeout(operationTimer);
|
||||||
|
if (drainTimer !== null) clearTimeout(drainTimer);
|
||||||
killTimer = null;
|
killTimer = null;
|
||||||
operationTimer = null;
|
operationTimer = null;
|
||||||
|
drainTimer = null;
|
||||||
};
|
};
|
||||||
|
|
||||||
const done = new Promise<SyncLauncherRunResult>((resolve) => {
|
const done = new Promise<SyncLauncherRunResult>((resolve) => {
|
||||||
@@ -149,7 +167,18 @@ export function runSyncLauncher(options: {
|
|||||||
child.on('exit', (code) => {
|
child.on('exit', (code) => {
|
||||||
exitObserved = true;
|
exitObserved = true;
|
||||||
exitCode = code;
|
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);
|
child.on('close', settleFromExit);
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user