Skip to content

Commit 72911ce

Browse files
bugerbuger
authored
fix: handle SIGUSR2 reloads in runner mode (#561)
1 parent 323657c commit 72911ce

4 files changed

Lines changed: 225 additions & 38 deletions

File tree

‎src/cli-main.ts‎

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,39 +1818,21 @@ export async function main(): Promise<void> {
18181818
const names = requestedRunners.join(', ');
18191819
console.log(`✅ Runner(s) started: ${names}. Press Ctrl+C to exit.`);
18201820

1821-
// Config watcher (shared, broadcasts to all runners)
1822-
let configWatcher: { stop(): void } | undefined;
1823-
let configWatchStore: { shutdown(): Promise<void> } | undefined;
1824-
if (options.watch) {
1825-
if (!options.configPath) {
1826-
console.error('❌ --watch requires --config <path>');
1827-
process.exit(1);
1828-
}
1829-
try {
1830-
const { ConfigSnapshotStore } = await import('./config/config-snapshot-store');
1831-
const { ConfigReloader } = await import('./config/config-reloader');
1832-
const { ConfigWatcher } = await import('./config/config-watcher');
1833-
const watchStore = new ConfigSnapshotStore();
1834-
await watchStore.initialize();
1835-
const reloader = new ConfigReloader({
1836-
configPath: options.configPath,
1837-
configManager,
1838-
snapshotStore: watchStore,
1839-
onSwap: newConfig => {
1840-
config = newConfig;
1841-
host.broadcastConfigUpdate(newConfig);
1842-
logger.info('[Watch] Config updated');
1843-
},
1844-
});
1845-
const watcher = new ConfigWatcher(options.configPath, reloader);
1846-
watcher.start();
1847-
configWatcher = watcher;
1848-
configWatchStore = watchStore;
1849-
logger.info('Config watching enabled');
1850-
} catch (watchErr: unknown) {
1851-
logger.warn(`Config watch setup failed (runners continue without it): ${watchErr}`);
1852-
}
1821+
if (options.watch && !options.configPath) {
1822+
console.error('❌ --watch requires --config <path>');
1823+
process.exit(1);
18531824
}
1825+
const { setupRunnerConfigReloadRuntime } = await import('./runners/config-reload-runtime');
1826+
const configReloadRuntime = await setupRunnerConfigReloadRuntime({
1827+
configPath: options.configPath,
1828+
watch: Boolean(options.watch),
1829+
configManager,
1830+
onSwap: newConfig => {
1831+
config = newConfig;
1832+
host.broadcastConfigUpdate(newConfig);
1833+
logger.info('[Watch] Config updated');
1834+
},
1835+
});
18541836

18551837
// Unified graceful shutdown
18561838
let shuttingDown = false;
@@ -1867,8 +1849,7 @@ export async function main(): Promise<void> {
18671849
}, 5000);
18681850
forceTimer.unref();
18691851
try {
1870-
if (configWatcher) configWatcher.stop();
1871-
if (configWatchStore) configWatchStore.shutdown().catch(() => {});
1852+
await configReloadRuntime.cleanup();
18721853
await host.stopAll();
18731854
if (sharedTaskStore) {
18741855
try {
@@ -1893,8 +1874,7 @@ export async function main(): Promise<void> {
18931874
const restartManager = new GracefulRestartManager(host, config.graceful_restart);
18941875
// Register cleanup callbacks for resources outside RunnerHost
18951876
restartManager.onCleanup(async () => {
1896-
if (configWatcher) configWatcher.stop();
1897-
if (configWatchStore) await configWatchStore.shutdown().catch(() => {});
1877+
await configReloadRuntime.cleanup();
18981878
if (sharedTaskStore) await sharedTaskStore.shutdown().catch(() => {});
18991879
});
19001880
process.on('SIGUSR1', () => {

‎src/config/config-watcher.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ export class ConfigWatcher {
113113
this.debounceMs = debounceMs ?? DEFAULT_DEBOUNCE_MS;
114114
}
115115

116-
start(): void {
116+
start(options?: { listenForSignals?: boolean }): void {
117+
const listenForSignals = options?.listenForSignals !== false;
117118
// Remove any previous listeners in case start() is called after stop()
118119
this.stop();
119120

@@ -125,7 +126,7 @@ export class ConfigWatcher {
125126
}
126127

127128
// Listen for SIGUSR2 (non-Windows)
128-
if (process.platform !== 'win32') {
129+
if (listenForSignals && process.platform !== 'win32') {
129130
this.signalHandler = () => {
130131
logger.info('[ConfigWatcher] Received SIGUSR2, triggering config reload');
131132
this.safeReload();

‎src/runners/config-reload-runtime.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { logger } from '../logger';
2+
import { ConfigManager } from '../config';
3+
import type { VisorConfig } from '../types/config';
4+
import { ConfigSnapshotStore } from '../config/config-snapshot-store';
5+
import { ConfigReloader } from '../config/config-reloader';
6+
import { ConfigWatcher } from '../config/config-watcher';
7+
8+
export interface RunnerConfigReloadRuntimeOptions {
9+
configPath?: string;
10+
watch: boolean;
11+
configManager: ConfigManager;
12+
onSwap: (newConfig: VisorConfig) => void;
13+
}
14+
15+
export interface RunnerConfigReloadRuntime {
16+
cleanup(): Promise<void>;
17+
}
18+
19+
export async function setupRunnerConfigReloadRuntime(
20+
options: RunnerConfigReloadRuntimeOptions
21+
): Promise<RunnerConfigReloadRuntime> {
22+
let snapshotStore: ConfigSnapshotStore | undefined;
23+
let watcher: ConfigWatcher | undefined;
24+
let signalHandler: (() => void) | undefined;
25+
26+
if (process.platform !== 'win32') {
27+
if (options.configPath) {
28+
try {
29+
snapshotStore = new ConfigSnapshotStore();
30+
await snapshotStore.initialize();
31+
32+
const reloader = new ConfigReloader({
33+
configPath: options.configPath,
34+
configManager: options.configManager,
35+
snapshotStore,
36+
onSwap: options.onSwap,
37+
});
38+
39+
signalHandler = () => {
40+
logger.info('[ConfigReload] Received SIGUSR2, triggering config reload');
41+
void reloader.reload();
42+
};
43+
process.on('SIGUSR2', signalHandler);
44+
logger.info('[ConfigReload] Send SIGUSR2 to hot-reload configuration');
45+
46+
if (options.watch) {
47+
watcher = new ConfigWatcher(options.configPath, reloader);
48+
watcher.start({ listenForSignals: false });
49+
logger.info('Config watching enabled');
50+
}
51+
} catch (err: unknown) {
52+
logger.warn(`Config reload setup failed (runners continue without it): ${err}`);
53+
}
54+
} else {
55+
signalHandler = () => {
56+
logger.warn('[ConfigReload] Ignoring SIGUSR2: no --config path configured');
57+
};
58+
process.on('SIGUSR2', signalHandler);
59+
logger.info(
60+
'[ConfigReload] Send SIGUSR2 to hot-reload configuration (will be ignored without --config)'
61+
);
62+
}
63+
}
64+
65+
return {
66+
async cleanup(): Promise<void> {
67+
if (watcher) watcher.stop();
68+
if (signalHandler && process.platform !== 'win32') {
69+
process.removeListener('SIGUSR2', signalHandler);
70+
}
71+
if (snapshotStore) {
72+
await snapshotStore.shutdown().catch(() => {});
73+
}
74+
},
75+
};
76+
}

‎tests/unit/runners/config-reload-runtime.test.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { setupRunnerConfigReloadRuntime } from '../../../src/runners/config-reload-runtime';
2+
3+
const reloadMock = jest.fn().mockResolvedValue(true);
4+
const initializeMock = jest.fn().mockResolvedValue(undefined);
5+
const shutdownMock = jest.fn().mockResolvedValue(undefined);
6+
const watcherStartMock = jest.fn();
7+
const watcherStopMock = jest.fn();
8+
9+
jest.mock('../../../src/logger', () => ({
10+
logger: {
11+
info: jest.fn(),
12+
warn: jest.fn(),
13+
debug: jest.fn(),
14+
error: jest.fn(),
15+
},
16+
}));
17+
18+
jest.mock('../../../src/config/config-snapshot-store', () => ({
19+
ConfigSnapshotStore: jest.fn().mockImplementation(() => ({
20+
initialize: initializeMock,
21+
shutdown: shutdownMock,
22+
})),
23+
}));
24+
25+
jest.mock('../../../src/config/config-reloader', () => ({
26+
ConfigReloader: jest.fn().mockImplementation(() => ({
27+
reload: reloadMock,
28+
})),
29+
}));
30+
31+
jest.mock('../../../src/config/config-watcher', () => ({
32+
ConfigWatcher: jest.fn().mockImplementation(() => ({
33+
start: watcherStartMock,
34+
stop: watcherStopMock,
35+
})),
36+
}));
37+
38+
describe('setupRunnerConfigReloadRuntime', () => {
39+
const mockLogger = jest.requireMock('../../../src/logger').logger as {
40+
info: jest.Mock;
41+
warn: jest.Mock;
42+
debug: jest.Mock;
43+
error: jest.Mock;
44+
};
45+
let processOnSpy: jest.SpyInstance;
46+
let processRemoveListenerSpy: jest.SpyInstance;
47+
let registeredUsr2Handler: (() => void) | undefined;
48+
49+
beforeEach(() => {
50+
jest.clearAllMocks();
51+
registeredUsr2Handler = undefined;
52+
processOnSpy = jest.spyOn(process, 'on').mockImplementation(((
53+
event: string,
54+
handler: () => void
55+
) => {
56+
if (event === 'SIGUSR2') registeredUsr2Handler = handler;
57+
return process;
58+
}) as any);
59+
processRemoveListenerSpy = jest.spyOn(process, 'removeListener').mockImplementation(((
60+
event: string,
61+
handler: () => void
62+
) => {
63+
if (event === 'SIGUSR2' && registeredUsr2Handler === handler) {
64+
registeredUsr2Handler = undefined;
65+
}
66+
return process;
67+
}) as any);
68+
});
69+
70+
afterEach(() => {
71+
processOnSpy.mockRestore();
72+
processRemoveListenerSpy.mockRestore();
73+
});
74+
75+
it('registers SIGUSR2 reload handling without file watch mode', async () => {
76+
const runtime = await setupRunnerConfigReloadRuntime({
77+
configPath: '/tmp/test.visor.yaml',
78+
watch: false,
79+
configManager: {} as any,
80+
onSwap: jest.fn(),
81+
});
82+
83+
expect(initializeMock).toHaveBeenCalledTimes(1);
84+
expect(watcherStartMock).not.toHaveBeenCalled();
85+
expect(registeredUsr2Handler).toEqual(expect.any(Function));
86+
87+
registeredUsr2Handler?.();
88+
await Promise.resolve();
89+
90+
expect(reloadMock).toHaveBeenCalledTimes(1);
91+
92+
await runtime.cleanup();
93+
expect(shutdownMock).toHaveBeenCalledTimes(1);
94+
expect(registeredUsr2Handler).toBeUndefined();
95+
});
96+
97+
it('starts file watching without registering a duplicate SIGUSR2 handler in watch mode', async () => {
98+
const runtime = await setupRunnerConfigReloadRuntime({
99+
configPath: '/tmp/test.visor.yaml',
100+
watch: true,
101+
configManager: {} as any,
102+
onSwap: jest.fn(),
103+
});
104+
105+
expect(watcherStartMock).toHaveBeenCalledWith({ listenForSignals: false });
106+
expect(registeredUsr2Handler).toEqual(expect.any(Function));
107+
108+
await runtime.cleanup();
109+
expect(watcherStopMock).toHaveBeenCalledTimes(1);
110+
});
111+
112+
it('installs a non-fatal SIGUSR2 handler even without a config path', async () => {
113+
const runtime = await setupRunnerConfigReloadRuntime({
114+
configPath: undefined,
115+
watch: false,
116+
configManager: {} as any,
117+
onSwap: jest.fn(),
118+
});
119+
120+
expect(initializeMock).not.toHaveBeenCalled();
121+
expect(registeredUsr2Handler).toEqual(expect.any(Function));
122+
123+
expect(() => registeredUsr2Handler?.()).not.toThrow();
124+
expect(mockLogger.warn).toHaveBeenCalledWith(
125+
'[ConfigReload] Ignoring SIGUSR2: no --config path configured'
126+
);
127+
128+
await runtime.cleanup();
129+
});
130+
});

0 commit comments

Comments
 (0)