Skip to content

Commit 0ace968

Browse files
committed
fix: harden push lock acquisition
1 parent b5b944b commit 0ace968

3 files changed

Lines changed: 90 additions & 9 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ openclaw plugins list # should show openclaw-b2-backup
316316
pnpm test
317317
```
318318

319-
128 tests across 11 test files covering encryption round-trips, manifest diffing, snapshot filtering, file gathering, B2 client signing, debounce timing, CLI behavior, push coordination, and plugin registration.
319+
130 tests across 11 test files covering encryption round-trips, manifest diffing, snapshot filtering, file gathering, B2 client signing, debounce timing, CLI behavior, push coordination, and plugin registration.
320320

321321
## License
322322

src/push.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,75 @@ describe("push", () => {
9898
await fs.promises.rm(stateDir, { recursive: true, force: true });
9999
});
100100

101+
it("returns a lock error when another process wins the stale-lock retry", async () => {
102+
const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "b2-state-"));
103+
const lockDir = path.join(stateDir, ".b2-backup-push.lock");
104+
await fs.promises.writeFile(path.join(stateDir, "openclaw.json"), "{}");
105+
await fs.promises.mkdir(lockDir);
106+
await fs.promises.writeFile(path.join(lockDir, "owner.json"), JSON.stringify({ pid: 12345 }));
107+
const config: ResolvedB2BackupConfig = {
108+
keyId: "test-key",
109+
applicationKey: "test-secret",
110+
bucket: "test-bucket",
111+
region: "test-region",
112+
encrypt: false,
113+
};
114+
const logger = { info: vi.fn(), warn: vi.fn(), debug: vi.fn() };
115+
const rm = fs.promises.rm.bind(fs.promises);
116+
const rmSpy = vi.spyOn(fs.promises, "rm").mockImplementation(async (target, options) => {
117+
await rm(target, options);
118+
if (path.resolve(String(target)) === path.resolve(lockDir)) {
119+
await fs.promises.mkdir(lockDir, { recursive: true });
120+
await fs.promises.writeFile(
121+
path.join(lockDir, "owner.json"),
122+
JSON.stringify({ pid: process.pid }),
123+
);
124+
}
125+
});
126+
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => {
127+
const err = new Error("no such process") as Error & { code: string };
128+
err.code = "ESRCH";
129+
throw err;
130+
});
131+
132+
await expect(push(config, stateDir, mockB2(), logger)).rejects.toThrow(PushLockError);
133+
await expect(push(config, stateDir, mockB2(), logger)).rejects.toThrow(
134+
"another push is already running",
135+
);
136+
137+
rmSpy.mockRestore();
138+
killSpy.mockRestore();
139+
await fs.promises.rm(stateDir, { recursive: true, force: true });
140+
});
141+
142+
it("removes the lock directory if writing owner metadata fails", async () => {
143+
const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "b2-state-"));
144+
const lockDir = path.join(stateDir, ".b2-backup-push.lock");
145+
const ownerPath = path.join(lockDir, "owner.json");
146+
await fs.promises.writeFile(path.join(stateDir, "openclaw.json"), "{}");
147+
const config: ResolvedB2BackupConfig = {
148+
keyId: "test-key",
149+
applicationKey: "test-secret",
150+
bucket: "test-bucket",
151+
region: "test-region",
152+
encrypt: false,
153+
};
154+
const logger = { info: vi.fn(), warn: vi.fn(), debug: vi.fn() };
155+
const writeFile = fs.promises.writeFile.bind(fs.promises);
156+
const writeSpy = vi.spyOn(fs.promises, "writeFile").mockImplementation(async (file, data, options) => {
157+
if (path.resolve(String(file)) === path.resolve(ownerPath)) {
158+
throw new Error("disk full");
159+
}
160+
return writeFile(file, data, options);
161+
});
162+
163+
await expect(push(config, stateDir, mockB2(), logger)).rejects.toThrow("disk full");
164+
await expect(fs.promises.access(lockDir)).rejects.toThrow();
165+
166+
writeSpy.mockRestore();
167+
await fs.promises.rm(stateDir, { recursive: true, force: true });
168+
});
169+
101170
it("adds a random suffix to snapshot IDs", () => {
102171
const snapshotId = createSnapshotId("2026-02-19T12:00:00.000Z");
103172

src/push.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,25 +176,37 @@ async function acquirePushLock(stateDir: string): Promise<{ release: () => Promi
176176
const lockDir = path.join(stateDir, PUSH_LOCK_DIRNAME);
177177
await fs.promises.mkdir(stateDir, { recursive: true });
178178

179-
try {
180-
await fs.promises.mkdir(lockDir);
181-
} catch (err) {
182-
if (!isNodeError(err, "EEXIST")) throw err;
183-
if (await removeDeadProcessLock(lockDir)) {
184-
await fs.promises.mkdir(lockDir);
185-
} else {
179+
if (!(await tryCreateLockDir(lockDir))) {
180+
const removedStaleLock = await removeDeadProcessLock(lockDir);
181+
if (!removedStaleLock || !(await tryCreateLockDir(lockDir))) {
186182
throw new PushLockError(`another push is already running (lock: ${lockDir})`);
187183
}
188184
}
189185

190-
await writeLockOwner(lockDir);
186+
try {
187+
await writeLockOwner(lockDir);
188+
} catch (err) {
189+
await fs.promises.rm(lockDir, { recursive: true, force: true }).catch(() => undefined);
190+
throw err;
191+
}
192+
191193
return {
192194
async release() {
193195
await fs.promises.rm(lockDir, { recursive: true, force: true }).catch(() => undefined);
194196
},
195197
};
196198
}
197199

200+
async function tryCreateLockDir(lockDir: string): Promise<boolean> {
201+
try {
202+
await fs.promises.mkdir(lockDir);
203+
return true;
204+
} catch (err) {
205+
if (isNodeError(err, "EEXIST")) return false;
206+
throw err;
207+
}
208+
}
209+
198210
async function writeLockOwner(lockDir: string): Promise<void> {
199211
const owner = {
200212
pid: process.pid,

0 commit comments

Comments
 (0)