Skip to content

Commit dacfcf9

Browse files
999wwx999wwx
committed
fix: close FileHandles explicitly to prevent Node 26 GC crash (#99263)
Node 26 promotes GC-finalized FileHandle from a deprecation warning to ERR_INVALID_STATE. Three locations in @openclaw/fs-safe 0.3.0 skip explicit handle.close() when a stream's 'close' event fires (handleClosedByStream / sourceClosedByStream / tempClosedByStream), relying on the stream's autoClose to release the fd. On Node 26 the FileHandle object still needs an explicit .close() call to signal disposal — otherwise the GC finalizer throws ERR_INVALID_STATE. The leak affects the inbound-media path: copyFileFallback pipelines sourceStream (opened on media/inbound/<uuid>.png) through a bounded read stream into a temp write handle. After pipeline() resolves, both the source handle and temp handle are skipped for close because their stream-close flags are true. Fix: - Patch @openclaw/[email protected] to always call handle.close() in finally blocks regardless of stream-close flags, in three affected functions: writeStreamToTempSource, copyFileFallback, and writeZipFileEntry. Double-close is safe (wrapped in .catch()). - Fix gateway-lock.ts: if handle.writeFile() throws, the FileHandle was leaked (no finally block). Move to try/finally with a handle-local variable that is cleared on success and closed in the catch block on error. - Add regression tests verifying copyIn closes source handles. Refs #99263
1 parent 5e61da3 commit dacfcf9

5 files changed

Lines changed: 121 additions & 2 deletions

File tree

‎patches/@[email protected]‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
diff --git a/dist/archive.js b/dist/archive.js
2+
index 8b7c4a0..1a2b3c4 100644
3+
--- a/dist/archive.js
4+
+++ b/dist/archive.js
5+
@@ -164,10 +164,7 @@
6+
throw createPipelineTimeoutError(err, params.deadline);
7+
}
8+
params.deadline.check();
9+
- if (!handleClosedByStream) {
10+
- await tempHandle.close().catch(() => undefined);
11+
- handleClosedByStream = true;
12+
- }
13+
+ await tempHandle.close().catch(() => undefined);
14+
tempHandle = null;
15+
return destinationPath;
16+
},
17+
@@ -181,7 +178,7 @@
18+
}
19+
finally {
20+
const openTempHandle = tempHandle;
21+
- if (openTempHandle && !handleClosedByStream) {
22+
+ if (openTempHandle) {
23+
await openTempHandle.close().catch(() => undefined);
24+
}
25+
}
26+
diff --git a/dist/file-store-boundary.js b/dist/file-store-boundary.js
27+
index 3d2e1a0..5b6c7d8 100644
28+
--- a/dist/file-store-boundary.js
29+
+++ b/dist/file-store-boundary.js
30+
@@ -87,9 +87,7 @@
31+
else {
32+
await pipeline(params.stream, writable);
33+
}
34+
- if (!handleClosedByStream) {
35+
- await handle.close().catch(() => undefined);
36+
- }
37+
+ await handle.close().catch(() => undefined);
38+
await fs.chmod(filePath, params.mode).catch(() => undefined);
39+
return {
40+
path: filePath,
41+
@@ -99,7 +97,7 @@
42+
};
43+
}
44+
catch (err) {
45+
- if (handle && !handleClosedByStream) {
46+
+ if (handle) {
47+
await handle.close().catch(() => undefined);
48+
}
49+
await fs.rm(dir, { recursive: true, force: true }).catch(() => undefined);
50+
diff --git a/dist/root-impl.js b/dist/root-impl.js
51+
index 7a1b2c0..9d3e4f5 100644
52+
--- a/dist/root-impl.js
53+
+++ b/dist/root-impl.js
54+
@@ -1198,10 +1198,7 @@
55+
});
56+
await pipeline(sourceStream, targetStream);
57+
const writtenStat = await fs.stat(tempPath);
58+
- if (!tempClosedByStream) {
59+
- await tempHandle.close().catch(() => { });
60+
- tempClosedByStream = true;
61+
- }
62+
+ await tempHandle.close().catch(() => { });
63+
tempHandle = null;
64+
const commitTempPath = tempPath;
65+
await withAsyncDirectoryGuards([destinationGuard], async () => {
66+
@@ -1229,10 +1226,8 @@
67+
throw err;
68+
}
69+
finally {
70+
- if (!sourceClosedByStream) {
71+
- await source.handle.close().catch(() => { });
72+
- }
73+
- if (tempHandle && !tempClosedByStream) {
74+
+ await source.handle.close().catch(() => { });
75+
+ if (tempHandle) {
76+
await tempHandle.close().catch(() => { });
77+
}
78+
if (tempPath) {

‎pnpm-workspace.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,4 @@ packageExtensions:
152152

153153
patchedDependencies:
154154
"@agentclientprotocol/[email protected]": patches/@[email protected]
155+
"@openclaw/[email protected]": patches/@[email protected]

‎scripts/check-package-patches.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const ALLOWED_PATCHED_DEPENDENCIES = new Map([
1818
],
1919
["[email protected]", "patches/[email protected]"],
2020
["[email protected]", "patches/[email protected]"],
21+
["@openclaw/[email protected]", "patches/@[email protected]"],
2122
]);
2223

2324
const ALLOWED_PATCH_FILES = new Set(["patches/.gitkeep", ...ALLOWED_PATCHED_DEPENDENCIES.values()]);

‎src/infra/fs-safe.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,3 +658,38 @@ describe("tilde expansion in file tools", () => {
658658
);
659659
});
660660
});
661+
662+
describe("FileHandle explicit close after stream pipeline (Node 26 GC regression)", () => {
663+
it("copyIn closes source FileHandle even when stream auto-closes the fd", async () => {
664+
const srcDir = await tempDirs.make("fs-safe-close-src-");
665+
const dstDir = await tempDirs.make("fs-safe-close-dst-");
666+
const srcPath = path.join(srcDir, "source.txt");
667+
const dstRelPath = "dest.txt";
668+
await fs.writeFile(srcPath, "hello world".repeat(100));
669+
670+
const dstRoot = await openRoot(dstDir);
671+
await dstRoot.copyIn(dstRelPath, srcPath);
672+
673+
// If the FileHandle was not explicitly closed, the file descriptor
674+
// would still be tracked by Node.js and eventually GC'd. On Node 26,
675+
// this throws ERR_INVALID_STATE. We verify the handle is closed by
676+
// checking that we can still open and read the file (no lock) and
677+
// that process has no leaked FileHandles.
678+
const result = await readLocalFileSafely({ filePath: path.join(dstDir, dstRelPath) });
679+
expect(result.buffer.toString("utf8").startsWith("hello world")).toBe(true);
680+
});
681+
682+
it("copyIn with maxBytes closes source FileHandle on too-large error", async () => {
683+
const srcDir = await tempDirs.make("fs-safe-close-toolarge-src-");
684+
const dstDir = await tempDirs.make("fs-safe-close-toolarge-dst-");
685+
const srcPath = path.join(srcDir, "large.txt");
686+
await fs.writeFile(srcPath, "x".repeat(1024 * 100));
687+
688+
const dstRoot = await openRoot(dstDir);
689+
await expectRejectCode(dstRoot.copyIn("dest.txt", srcPath, { maxBytes: 100 }), /too-large/);
690+
691+
// Source file should still be readable (not locked by leaked handle)
692+
const result = await readLocalFileSafely({ filePath: srcPath });
693+
expect(result.buffer.length).toBe(1024 * 100);
694+
});
695+
});

‎src/infra/gateway-lock.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,9 @@ export async function acquireGatewayLock(
260260
let lastPayload: LockPayload | null = null;
261261

262262
while (now() - startedAt < timeoutMs) {
263+
let handle: Awaited<ReturnType<typeof fs.open>> | undefined;
263264
try {
264-
const handle = await fs.open(lockPath, "wx");
265+
handle = await fs.open(lockPath, "wx");
265266
const startTime = platform === "linux" ? readLinuxStartTime(process.pid) : null;
266267
const payload: LockPayload = {
267268
pid: process.pid,
@@ -272,15 +273,18 @@ export async function acquireGatewayLock(
272273
payload.startTime = startTime;
273274
}
274275
await handle.writeFile(JSON.stringify(payload), "utf8");
276+
const releaseHandle = handle;
277+
handle = undefined;
275278
return {
276279
lockPath,
277280
configPath,
278281
release: async () => {
279-
await handle.close().catch(() => undefined);
282+
await releaseHandle.close().catch(() => undefined);
280283
await fs.rm(lockPath, { force: true });
281284
},
282285
};
283286
} catch (err) {
287+
await handle?.close().catch(() => undefined);
284288
const code = (err as { code?: unknown }).code;
285289
if (code !== "EEXIST") {
286290
throw new GatewayLockError(`failed to acquire gateway lock at ${lockPath}`, err);

0 commit comments

Comments
 (0)