From 373a72bdf2e41219a87c456dee45e902e4aa5d1e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 19 Mar 2026 03:59:52 +0000 Subject: [PATCH] fix: harden instance lock compatibility semantics Co-authored-by: Haze --- README.ja-JP.md | 1 + README.md | 1 + README.zh-CN.md | 1 + electron/main/index.ts | 21 +++--- electron/main/process-instance-lock.ts | 85 +++++++++++++++++++++--- electron/main/signal-quit.ts | 11 +++ tests/unit/process-instance-lock.test.ts | 53 +++++++++++++-- tests/unit/signal-quit.test.ts | 15 +++++ 8 files changed, 168 insertions(+), 20 deletions(-) create mode 100644 electron/main/signal-quit.ts create mode 100644 tests/unit/signal-quit.test.ts diff --git a/README.ja-JP.md b/README.ja-JP.md index 35e0e7f..1ab1543 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -255,6 +255,7 @@ ClawXは、**デュアルプロセス + Host API 統一アクセス**構成を - ClawX は Electron アプリのため、**1つのアプリインスタンスでも複数プロセス（main/renderer/zygote/utility）が表示される**のが正常です。 - 単一起動保護は Electron のロックに加え、ローカルのプロセスロックファイルも併用し、デスクトップ IPC / セッションバスが不安定な環境でも重複起動を防ぎます。 +- ローリングアップグレード中に旧版/新版が混在すると、単一起動保護の挙動が非対称になる場合があります。安定運用のため、デスクトップクライアントは可能な限り同一バージョンへ揃えてください。 - ただし OpenClaw Gateway の待受は常に**単一**であるべきです。`127.0.0.1:18789` を Listen しているプロセスは1つだけです。 - Listen プロセスの確認例: - macOS/Linux: `lsof -nP -iTCP:18789 -sTCP:LISTEN` diff --git a/README.md b/README.md index 69f4a97..7ae0abd 100644 --- a/README.md +++ b/README.md @@ -259,6 +259,7 @@ ClawX employs a **dual-process architecture** with a unified host API layer. The - ClawX is an Electron app, so **one app instance normally appears as multiple OS processes** (main/renderer/zygote/utility). This is expected. - Single-instance protection uses Electron's lock plus a local process-file lock fallback, preventing duplicate app launch in environments where desktop IPC/session bus is unstable. +- During rolling upgrades, mixed old/new app versions can still have asymmetric protection behavior. For best reliability, upgrade all desktop clients to the same version. - The OpenClaw Gateway listener should still be **single-owner**: only one process should listen on `127.0.0.1:18789`. - To verify the active listener: - macOS/Linux: `lsof -nP -iTCP:18789 -sTCP:LISTEN` diff --git a/README.zh-CN.md b/README.zh-CN.md index 332501e..454dabb 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -259,6 +259,7 @@ ClawX 采用 **双进程 + Host API 统一接入架构**。渲染进程只调用 - ClawX 基于 Electron，**单个应用实例出现多个系统进程是正常现象**（main/renderer/zygote/utility）。 - 单实例保护同时使用 Electron 自带锁与本地进程文件锁回退机制，可在桌面会话总线异常时避免重复启动。 +- 滚动升级期间若新旧版本混跑，单实例保护仍可能出现不对称行为。为保证稳定性，建议桌面客户端尽量统一升级到同一版本。 - 但 OpenClaw Gateway 监听应始终保持**单实例**：`127.0.0.1:18789` 只能有一个监听者。 - 可用以下命令确认监听进程： - macOS/Linux：`lsof -nP -iTCP:18789 -sTCP:LISTEN` diff --git a/electron/main/index.ts b/electron/main/index.ts index c46255e..25ce180 100644 --- a/electron/main/index.ts +++ b/electron/main/index.ts @@ -32,6 +32,7 @@ import { markQuitCleanupCompleted, requestQuitLifecycleAction, } from './quit-lifecycle'; +import { createSignalQuitHandler } from './signal-quit'; import { acquireProcessInstanceFileLock } from './process-instance-lock'; import { getSetting } from '../utils/store'; import { ensureBuiltinSkillsInstalled, ensurePreinstalledSkillsInstalled } from '../utils/skill-config'; @@ -90,8 +91,13 @@ if (gotElectronLock) { gotFileLock = fileLock.acquired; releaseProcessInstanceFileLock = fileLock.release; if (!fileLock.acquired) { + const ownerDescriptor = fileLock.ownerPid + ? `${fileLock.ownerFormat ?? 'legacy'} pid=${fileLock.ownerPid}` + : fileLock.ownerFormat === 'unknown' + ? 'unknown lock format/content' + : 'unknown owner'; console.info( - `[ClawX] Another instance already holds process lock (${fileLock.lockPath}${fileLock.ownerPid ? `, pid=${fileLock.ownerPid}` : ''}); exiting duplicate process`, + `[ClawX] Another instance already holds process lock (${fileLock.lockPath}, ${ownerDescriptor}); exiting duplicate process`, ); app.exit(0); } @@ -442,18 +448,17 @@ async function initialize(): Promise { } if (gotTheLock) { - const releaseProcessLockOnSignal = (signal: NodeJS.Signals): void => { - logger.info(`Received ${signal}; releasing instance lock and requesting app quit`); - releaseProcessInstanceFileLock(); - app.quit(); - }; + const requestQuitOnSignal = createSignalQuitHandler({ + logInfo: (message) => logger.info(message), + requestQuit: () => app.quit(), + }); process.on('exit', () => { releaseProcessInstanceFileLock(); }); - process.once('SIGINT', () => releaseProcessLockOnSignal('SIGINT')); - process.once('SIGTERM', () => releaseProcessLockOnSignal('SIGTERM')); + process.once('SIGINT', () => requestQuitOnSignal('SIGINT')); + process.once('SIGTERM', () => requestQuitOnSignal('SIGTERM')); app.on('will-quit', () => { releaseProcessInstanceFileLock(); diff --git a/electron/main/process-instance-lock.ts b/electron/main/process-instance-lock.ts index 72bdc19..f39cf61 100644 --- a/electron/main/process-instance-lock.ts +++ b/electron/main/process-instance-lock.ts @@ -1,10 +1,14 @@ import { closeSync, existsSync, mkdirSync, openSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; +const LOCK_SCHEMA = 'clawx-instance-lock'; +const LOCK_VERSION = 1; + export interface ProcessInstanceFileLock { acquired: boolean; lockPath: string; ownerPid?: number; + ownerFormat?: 'legacy' | 'structured' | 'unknown'; release: () => void; } @@ -25,14 +29,67 @@ function defaultPidAlive(pid: number): boolean { } } -function readLockOwnerPid(lockPath: string): number | undefined { - try { - const raw = readFileSync(lockPath, 'utf8').trim(); - const parsed = Number.parseInt(raw, 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : undefined; - } catch { +type ParsedLockOwner = + | { kind: 'legacy'; pid: number } + | { kind: 'structured'; pid: number } + | { kind: 'unknown' }; + +interface StructuredLockContent { + schema: string; + version: number; + pid: number; +} + +function parsePositivePid(raw: string): number | undefined { + if (!/^\d+$/.test(raw)) { return undefined; } + const parsed = Number.parseInt(raw, 10); + if (!Number.isFinite(parsed) || parsed <= 0) { + return undefined; + } + return parsed; +} + +function parseStructuredLockContent(raw: string): StructuredLockContent | undefined { + try { + const parsed = JSON.parse(raw) as Partial; + if ( + parsed?.schema === LOCK_SCHEMA + && parsed?.version === LOCK_VERSION + && typeof parsed?.pid === 'number' + && Number.isFinite(parsed.pid) + && parsed.pid > 0 + ) { + return { + schema: parsed.schema, + version: parsed.version, + pid: parsed.pid, + }; + } + } catch { + // ignore parse errors + } + return undefined; +} + +function readLockOwner(lockPath: string): ParsedLockOwner { + try { + const raw = readFileSync(lockPath, 'utf8').trim(); + const legacyPid = parsePositivePid(raw); + if (legacyPid !== undefined) { + return { kind: 'legacy', pid: legacyPid }; + } + + const structured = parseStructuredLockContent(raw); + if (structured) { + return { kind: 'structured', pid: structured.pid }; + } + } catch { + // ignore read errors + } + + return { kind: 'unknown' }; } export function acquireProcessInstanceFileLock( @@ -45,11 +102,14 @@ export function acquireProcessInstanceFileLock( const lockPath = join(options.userDataDir, `${options.lockName}.instance.lock`); let ownerPid: number | undefined; + let ownerFormat: ProcessInstanceFileLock['ownerFormat'] = 'unknown'; for (let attempt = 0; attempt < 2; attempt += 1) { try { const fd = openSync(lockPath, 'wx'); try { + // Keep writing legacy numeric format for broad backward compatibility. + // Parser accepts both legacy numeric and structured JSON formats. writeFileSync(fd, String(pid), 'utf8'); } finally { closeSync(fd); @@ -75,9 +135,17 @@ export function acquireProcessInstanceFileLock( break; } - ownerPid = readLockOwnerPid(lockPath); + const owner = readLockOwner(lockPath); + if (owner.kind === 'legacy' || owner.kind === 'structured') { + ownerPid = owner.pid; + ownerFormat = owner.kind; + } else { + ownerPid = undefined; + ownerFormat = 'unknown'; + } const shouldTreatAsStale = - ownerPid === undefined || !isPidAlive(ownerPid); + (owner.kind === 'legacy' || owner.kind === 'structured') + && !isPidAlive(owner.pid); if (shouldTreatAsStale && existsSync(lockPath)) { try { rmSync(lockPath, { force: true }); @@ -95,6 +163,7 @@ export function acquireProcessInstanceFileLock( acquired: false, lockPath, ownerPid, + ownerFormat, release: () => { // no-op when lock wasn't acquired }, diff --git a/electron/main/signal-quit.ts b/electron/main/signal-quit.ts new file mode 100644 index 0000000..42bbdcf --- /dev/null +++ b/electron/main/signal-quit.ts @@ -0,0 +1,11 @@ +export interface SignalQuitHandlerHooks { + logInfo: (message: string) => void; + requestQuit: () => void; +} + +export function createSignalQuitHandler(hooks: SignalQuitHandlerHooks): (signal: NodeJS.Signals) => void { + return (signal: NodeJS.Signals) => { + hooks.logInfo(`Received ${signal}; requesting app quit`); + hooks.requestQuit(); + }; +} diff --git a/tests/unit/process-instance-lock.test.ts b/tests/unit/process-instance-lock.test.ts index dadb579..b13dcdc 100644 --- a/tests/unit/process-instance-lock.test.ts +++ b/tests/unit/process-instance-lock.test.ts @@ -57,6 +57,7 @@ describe('process instance file lock', () => { expect(first.acquired).toBe(true); expect(second.acquired).toBe(false); expect(second.ownerPid).toBe(2222); + expect(second.ownerFormat).toBe('legacy'); first.release(); }); @@ -78,7 +79,28 @@ describe('process instance file lock', () => { lock.release(); }); - it('replaces malformed lock file content', () => { + it('replaces stale structured lock file when owner pid is not alive', () => { + const userDataDir = createTempDir(); + const lockPath = join(userDataDir, 'clawx.instance.lock'); + writeFileSync(lockPath, JSON.stringify({ + schema: 'clawx-instance-lock', + version: 1, + pid: 7777, + }), 'utf8'); + + const lock = acquireProcessInstanceFileLock({ + userDataDir, + lockName: 'clawx', + pid: 6666, + isPidAlive: () => false, + }); + + expect(lock.acquired).toBe(true); + expect(readFileSync(lockPath, 'utf8')).toBe('6666'); + lock.release(); + }); + + it('does not treat malformed lock file content as stale', () => { const userDataDir = createTempDir(); const lockPath = join(userDataDir, 'clawx.instance.lock'); writeFileSync(lockPath, 'not-a-pid', 'utf8'); @@ -89,8 +111,31 @@ describe('process instance file lock', () => { pid: 6666, }); - expect(lock.acquired).toBe(true); - expect(readFileSync(lockPath, 'utf8')).toBe('6666'); - lock.release(); + expect(lock.acquired).toBe(false); + expect(lock.ownerPid).toBeUndefined(); + expect(lock.ownerFormat).toBe('unknown'); + expect(readFileSync(lockPath, 'utf8')).toBe('not-a-pid'); + }); + + it('does not treat unknown structured lock schema as stale', () => { + const userDataDir = createTempDir(); + const lockPath = join(userDataDir, 'clawx.instance.lock'); + writeFileSync(lockPath, JSON.stringify({ + schema: 'future-lock-schema', + version: 2, + pid: 8888, + owner: 'future-build', + }), 'utf8'); + + const lock = acquireProcessInstanceFileLock({ + userDataDir, + lockName: 'clawx', + pid: 9999, + }); + + expect(lock.acquired).toBe(false); + expect(lock.ownerPid).toBeUndefined(); + expect(lock.ownerFormat).toBe('unknown'); + expect(readFileSync(lockPath, 'utf8')).toContain('future-lock-schema'); }); }); diff --git a/tests/unit/signal-quit.test.ts b/tests/unit/signal-quit.test.ts new file mode 100644 index 0000000..82d3570 --- /dev/null +++ b/tests/unit/signal-quit.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createSignalQuitHandler } from '@electron/main/signal-quit'; + +describe('signal quit handler', () => { + it('logs and requests quit when signal is received', () => { + const logInfo = vi.fn(); + const requestQuit = vi.fn(); + const handler = createSignalQuitHandler({ logInfo, requestQuit }); + + handler('SIGTERM'); + + expect(logInfo).toHaveBeenCalledWith('Received SIGTERM; requesting app quit'); + expect(requestQuit).toHaveBeenCalledTimes(1); + }); +});