Fix/issue 98958 gateway lock fd leak by chenyangjun-xy · Pull Request #99291 · openclaw/openclaw

chenyangjun-xy · 2026-07-03T01:18:30Z

What Problem This Solves

Fixes #98958: In src/infra/gateway-lock.ts, when fs.open(lockPath, "wx") succeeds but the subsequent handle.writeFile() fails (e.g. disk full / ENOSPC), the code throws GatewayLockError without closing the file handle or removing the partially-written lock file — leaking a file descriptor and leaving a stale lock artifact on disk.

Root Cause

acquireGatewayLock() opens a lock file with fs.open(lockPath, "wx"), then writes the payload with handle.writeFile(). The outer try-catch only handled the EEXIST case (lock already held). Any other writeFile failure fell through to throw new GatewayLockError(...) without cleanup — the fd from fs.open was never closed and the partially-written lock file was never removed.

Why This Change Was Made

Wrap the payload-write block in a nested try-catch inside the existing try block. When writeFile fails:

Close the file handle (await handle.close()) to release the fd
Remove the lock file (await fs.rm(lockPath, { force: true })) to clean up the stale artifact
Re-throw so the outer catch wraps it in GatewayLockError as before

The success path is unchanged: the handle stays open and is closed later in the release() callback.

Changes

src/infra/gateway-lock.ts: +6 lines — nested try-catch around writeFile; catch closes handle + removes lock file, then re-throws
src/infra/gateway-lock.test.ts: +31 lines — new test "closes handle and removes lock file when writeFile fails after open succeeds"

Evidence

Verification environment

Item	Value
OS	Linux 4.19.112 x86_64
Node.js	v22.19.0
Vitest	v4.1.8

Regression tests (17/17, 1 new)

 ✓ blocks concurrent acquisition until release
 ✓ treats recycled linux pid as stale when start time mismatches
 ✓ keeps lock on linux when proc access fails unless stale
 ✓ keeps lock when fs.stat fails until payload is stale
 ✓ treats lock as stale when owner pid is alive but configured port is free
 ✓ keeps lock when configured port is busy and owner pid is alive
 ✓ bounds oversized lock polling intervals by the acquire timeout
 ✓ returns null when multi-gateway override is enabled
 ✓ returns null in test env unless allowInTests is set
 ✓ falls back instead of throwing when lock payload clock is outside Date range
 ✓ wraps unexpected fs errors as GatewayLockError
 ✓ closes handle and removes lock file when writeFile fails after open succeeds  ← NEW
 ✓ clears stale lock on win32 when process cmdline is not a gateway
 ✓ keeps lock on win32 when process cmdline is a gateway
 ✓ falls back to unknown on win32 when cmdline reader returns null
 ✓ clears stale lock on darwin when process cmdline is not a gateway
 ✓ keeps lock on darwin when process cmdline is a gateway
 Test Files  1 passed (1) | Tests  17 passed (17)

The new test:

Mocks fs.open to return a handle with a failing writeFile (ENOSPC)
Asserts handle.close() is called exactly once after the failure
Asserts fs.rm(lockPath, { force: true }) is called to remove the partial lock file

Real-process fault injection: open succeeds, writeFile fails

Standalone script (scripts/verify-gateway-lock-fd-leak-fault.mjs) imports the production acquireGatewayLock module via tsx. It wraps fs.open to return a handle with a patched writeFile that throws ENOSPC — simulating what the kernel does when the disk is genuinely full. The patched close records whether the production cleanup path executed.

This is NOT a Vitest mock. The only mutation is handle.writeFile (to inject the fault) and handle.close (to record the call). All other logic — the inner try-catch, the fs.rm, the GatewayLockError wrapping — is the actual production code running in a real Node.js process.

========================================================================
OpenClaw Gateway Lock — Fault Injection Proof (#98958)
========================================================================
PID:       1424146
Node:      v22.19.0
Platform:  linux x64
Temp dir:  /tmp/gateway-fault-proof-hLM6nt

── Phase 1: Normal acquire + release (baseline) ──
  FDs at start: 28
  Lock acquired: gateway.0f63a1b2.lock
  Payload valid: pid=1424146
  Lock file cleaned up: PASS
  FDs after release: 28 (delta: 0)

── Phase 2: Fault injection (ENOSPC on writeFile) ──
  FDs before: 28
  fs.open succeeded:      PASS
  writeFile reached:      PASS
  handle.close() called:  PASS
  lock file removed:      PASS
  error is GatewayLockErr: PASS
  error message:          failed to acquire gateway lock at /tmp/gateway-fault-proof-hLM6nt/locks/
  FDs after fault:        28 (before: 28, delta: 0)

── Phase 3: System health after fault ──
  Acquired after fault:   PASS
  Released:               PASS

── Event trace ──
  fs.open succeeded — fd obtained, lock file created on disk
  writeFile called (135-byte payload) — injecting ENOSPC
  handle.close() called — fd being released
  GatewayLockError thrown: failed to acquire gateway lock at ...

========================================================================
VERDICT
========================================================================
  Phase 1: normal acquire+release                        PASS
  Phase 2: fs.open succeeded → fd obtained               PASS
  Phase 2: writeFile reached → ENOSPC injected           PASS
  Phase 2: handle.close() called → fd leak fixed         PASS
  Phase 2: fs.rm(lockPath) → no stale lock artifact      PASS
  Phase 2: GatewayLockError thrown correctly             PASS
  Phase 2: no fd leak (delta ≤ 0)                        PASS
  Phase 3: system healthy after fault                    PASS

  OVERALL: PASS

  Fault injection exercised the production inner try-catch:
    1. fs.open  → lock file created on disk, fd obtained
    2. writeFile → ENOSPC injected (simulating kernel ENOSPC)
    3. catch     → handle.close() — fd released, leak prevented
    4. catch     → fs.rm(lockPath) — stale lock artifact removed
    5. catch     → throw → outer catch wraps as GatewayLockError

Measurement methodology:

Phase	What it exercises	Result
1	Baseline acquire+release, no fault	PASS (28→28, delta=0)
2	Fault injection: open succeeds, writeFile throws ENOSPC	PASS: close called, rm called, fd delta=0
3	After fault recovery, system still works	PASS

What each assertion proves:

Assertion	Before fix	After fix
`handle.close()` called after writeFile failure	❌ not called (fd leaked)	✅ called, fd released
Lock file removed after writeFile failure	❌ not removed (stale artifact)	✅ removed by `fs.rm()`
FD count stable after fault	❌ +1 (leaked)	✅ 28→28 (no leak)
Error still propagated as GatewayLockError	✅	✅

User Impact

No more fd leak when lock file write fails (disk full, quota exceeded, etc.)
No stale/partial lock files left behind after write failures
Transparent to users; no config or behavior changes
Normal lock acquisition path unchanged

🤖 Generated with Claude Code

clawsweeper · 2026-07-03T01:30:26Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 5:17 AM ET / 09:17 UTC.

Summary
The PR updates gateway lock acquisition to close the newly opened lock handle and remove the partial lock file if payload writing fails, with a regression test for that failure path.

PR surface: Source +8, Tests +30. Total +38 across 2 files.

Reproducibility: yes. Source-level reproduction is high confidence: make fs.open(lockPath, "wx") succeed and the returned handle's writeFile reject; current main has no close/remove path before throwing GatewayLockError.

Review metrics: 1 noteworthy metric.

Same-root-cause candidates: 2 exact open PRs plus 1 broader open overlap. Maintainers should avoid merging multiple competing gateway-lock cleanup hunks for the same canonical issue.

Stored data model
Persistent data-model change detected: serialized state: src/infra/gateway-lock.test.ts, serialized state: src/infra/gateway-lock.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #98958
Summary: This PR is a candidate fix for the canonical gateway-lock post-open write-failure fd leak; one other open PR targets the same root cause and one broader PR partially overlaps the file-handle cleanup topic.

Members:

canonical: gateway-lock: file descriptor leak when writeFile fails after acquiring lock #98958 - The issue reports the exact fs.open succeeds then writeFile rejects cleanup gap that this PR changes.
same_root_cause: fix(gateway-lock): close fd when writeFile fails after acquiring lock #98848 - The open PR targets the same gateway-lock post-open write-failure cleanup path for the same issue, but has weaker real behavior proof.
partial_overlap: fix: close FileHandles explicitly to prevent Node 26 GC crash #99461 - The open PR is mainly a Node 26 @openclaw/fs-safe FileHandle patch but also includes a partial gateway-lock handle-close hunk without the same stale lock-file cleanup.
adjacent_distinct: fix(gateway-lock): close fd and remove partial lock file on writeFile failure #99356 - The closed PR had matching gateway-lock wording, but live diff inspection shows its actual patch changed usage-bar template caching instead.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Keep one canonical landing path for the gateway-lock cleanup and close or rebase the overlapping PRs after merge.

Risk before merge

[P1] A second exact same-root-cause PR remains open at fix(gateway-lock): close fd when writeFile fails after acquiring lock #98848, and a broader open FileHandle PR at fix: close FileHandles explicitly to prevent Node 26 GC crash #99461 also touches the gateway-lock path; maintainers should land one canonical cleanup and close or rebase the overlaps.

Maintainer options:

Decide the mitigation before merge
Land this PR or an equivalent narrow acquireGatewayLock cleanup as the single fix for gateway-lock: file descriptor leak when writeFile fails after acquiring lock #98958, then close the duplicate candidate path after merge.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair is needed; maintainers should choose the single landing path among the overlapping gateway-lock PRs and close or rebase the rest after merge.

Security
Cleared: The diff only changes local gateway lock cleanup and a colocated regression test; it adds no dependencies, workflows, lockfiles, scripts, secrets handling, or package-resolution surface.

Review details

Best possible solution:

Land this PR or an equivalent narrow acquireGatewayLock cleanup as the single fix for #98958, then close the duplicate candidate path after merge.

Do we have a high-confidence way to reproduce the issue?

Yes. Source-level reproduction is high confidence: make fs.open(lockPath, "wx") succeed and the returned handle's writeFile reject; current main has no close/remove path before throwing GatewayLockError.

Is this the best way to solve the issue?

Yes. Cleanup belongs inside acquireGatewayLock because the caller only receives a release callback after payload writing succeeds, and the sibling file-lock invariant supports closing opened handles on payload-write failure.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 68e06b9ea148.

Label changes

Label justifications:

P2: This is a bounded core gateway-runtime resource cleanup bug with limited blast radius and a focused, proof-backed fix candidate.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output from a real-process fault-injection run showing open succeeded, writeFile failed, cleanup ran, fd count stayed stable, and the gateway lock remained usable afterward.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a real-process fault-injection run showing open succeeded, writeFile failed, cleanup ran, fd count stayed stable, and the gateway lock remained usable afterward.

Evidence reviewed

PR surface:

Source +8, Tests +30. Total +38 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	17	9	+8
Tests	1	30	0	+30
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	47	9	+38

What I checked:

Current main leak path: acquireGatewayLock opens the lock file, writes the payload, and only closes the handle from the returned release callback; a writeFile rejection before return reaches the non-EEXIST GatewayLockError path without close/remove cleanup. (src/infra/gateway-lock.ts:264, 68e06b9ea148)
PR cleanup path: The PR head wraps the post-open payload preparation/write in an inner catch that closes the acquired handle, removes the lock path with force: true, and rethrows to preserve existing GatewayLockError wrapping. (src/infra/gateway-lock.ts:264, cdacea6ab4e2)
PR regression test: The added test creates a partial lock during mocked writeFile, throws ENOSPC, then asserts the handle was closed and the computed lock path was removed. (src/infra/gateway-lock.test.ts:380, cdacea6ab4e2)
Caller boundary: The gateway run loop receives a releasable lock only after acquireGatewayLock returns, so acquisition-time cleanup cannot be delegated to callers. (src/cli/gateway-cli/run-loop.ts:125, 68e06b9ea148)
Sibling cleanup invariant: The plugin SDK file-lock sibling already tests that an opened lock handle is closed when owner-payload writing fails, supporting the same invariant for gateway locks. (src/plugin-sdk/file-lock.test.ts:158, 68e06b9ea148)
Latest release still affected: The latest release tag v2026.6.11 contains the same open-then-write-then-return-release sequence, so the fix has not shipped in that release. (src/infra/gateway-lock.ts:264, e085fa1a3ffd)

Likely related people:

steipete: GitHub commit metadata links this handle to the original gateway singleton lock and multiple later gateway-lock recovery/test changes in the same files. (role: feature owner and recent area contributor; confidence: high; commits: 05a254746eae, e6383a2c13e5, 1670b970ee8c; files: src/infra/gateway-lock.ts, src/infra/gateway-lock.test.ts, src/cli/gateway-cli/run-loop.ts)
TonyDerek-dot: Recent merged PID-recycling and startup-progress hardening touched acquireGatewayLock, its tests, and the gateway run loop. (role: recent gateway-lock contributor; confidence: medium; commits: 5f6e3499f321; files: src/infra/gateway-lock.ts, src/infra/gateway-lock.test.ts, src/cli/gateway-cli/run-loop.ts)
vincentkoc: GitHub commit metadata links this handle to gateway-lock hash-path changes in the same source and test files. (role: adjacent infra contributor; confidence: medium; commits: c20d519e0576; files: src/infra/gateway-lock.ts, src/infra/gateway-lock.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-03T09:05:54Z

Land-ready verification for exact head cdacea6ab4e2ed68a37a8c83b36276828a3c6c29:

Kept cleanup ownership inside acquireGatewayLock: a pre-return payload/write failure closes the acquired handle, removes the partial lock path, and preserves the original cause.
Strengthened the regression to create a real partial lock before injected ENOSPC, then assert close, filesystem cleanup, and cause propagation.
node scripts/run-vitest.mjs src/infra/gateway-lock.test.ts — 17 passed.
Real-process fault injection — closeCalled=true, remainingLocks=0, fdDelta=0, and immediate reacquisition succeeded.
Fresh structured autoreview — clean; full branch review also clean.
Repo-native prepare gate — exact-head hosted CI/Testbox passed.

No screenshot attached: this is a daemon resource-lifecycle fix with no visual surface. Thanks @chenyangjun-xy for the fix.

…law#98958) When fs.open(lockPath, "wx") succeeds but handle.writeFile() fails (e.g. disk full / ENOSPC), close the file handle and remove the partially-written lock file before re-throwing to avoid a file descriptor leak and stale lock artifact. Changes: - src/infra/gateway-lock.ts: nested try-catch around writeFile - src/infra/gateway-lock.test.ts: test for fd close + lock cleanup - scripts/verify-gateway-lock-fd-leak*.mjs: fault-injection proof Co-Authored-By: Claude <[email protected]>

steipete · 2026-07-03T09:23:45Z

Merged via squash.

Prepared head SHA: cdacea6ab4e2ed68a37a8c83b36276828a3c6c29
Landed commit: 36dd9ee3c3c188cc0619f0a3277c64c4b96a9615

* fix(infra): close fd and remove lock file on writeFile failure (openclaw#98958) When fs.open(lockPath, "wx") succeeds but handle.writeFile() fails (e.g. disk full / ENOSPC), close the file handle and remove the partially-written lock file before re-throwing to avoid a file descriptor leak and stale lock artifact. Changes: - src/infra/gateway-lock.ts: nested try-catch around writeFile - src/infra/gateway-lock.test.ts: test for fd close + lock cleanup - scripts/verify-gateway-lock-fd-leak*.mjs: fault-injection proof Co-Authored-By: Claude <[email protected]> * test(infra): strengthen gateway lock cleanup proof --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

* fix(infra): close fd and remove lock file on writeFile failure (openclaw#98958) When fs.open(lockPath, "wx") succeeds but handle.writeFile() fails (e.g. disk full / ENOSPC), close the file handle and remove the partially-written lock file before re-throwing to avoid a file descriptor leak and stale lock artifact. Changes: - src/infra/gateway-lock.ts: nested try-catch around writeFile - src/infra/gateway-lock.test.ts: test for fd close + lock cleanup - scripts/verify-gateway-lock-fd-leak*.mjs: fault-injection proof Co-Authored-By: Claude <[email protected]> * test(infra): strengthen gateway lock cleanup proof --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]> (cherry picked from commit 36dd9ee)

openclaw-barnacle Bot added scripts Repository scripts size: L size: M and removed size: S size: L labels Jul 3, 2026

chenyangjun-xy force-pushed the fix/issue-98958-gateway-lock-fd-leak branch from 4dfc714 to 5f2ce17 Compare July 3, 2026 06:33

openclaw-barnacle Bot added size: L and removed size: M labels Jul 3, 2026

clawsweeper Bot mentioned this pull request Jul 3, 2026

fix(gateway-lock): close fd when writeFile fails after acquiring lock #98848

Closed

chenyangjun-xy force-pushed the fix/issue-98958-gateway-lock-fd-leak branch from 5f2ce17 to 1f05ad1 Compare July 3, 2026 06:47

openclaw-barnacle Bot added size: S and removed scripts Repository scripts size: L labels Jul 3, 2026

steipete self-assigned this Jul 3, 2026

steipete requested a review from a team as a code owner July 3, 2026 08:04

chenyangjun-xy and others added 2 commits July 3, 2026 10:06

test(infra): strengthen gateway lock cleanup proof

cdacea6

steipete force-pushed the fix/issue-98958-gateway-lock-fd-leak branch from 9228eb2 to cdacea6 Compare July 3, 2026 09:06

steipete merged commit 36dd9ee into openclaw:main Jul 3, 2026
149 of 150 checks passed

github-actions Bot mentioned this pull request Jul 3, 2026

📡 Upstream Digest — 2026-07-03 10:27 UTC curtismercier/openclaw-mods#1196

Open

clawsweeper Bot mentioned this pull request Jul 18, 2026

fix: close FileHandles explicitly to prevent Node 26 GC crash #99461

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix/issue 98958 gateway lock fd leak#99291

Fix/issue 98958 gateway lock fd leak#99291
steipete merged 2 commits into
openclaw:mainfrom
chenyangjun-xy:fix/issue-98958-gateway-lock-fd-leak

chenyangjun-xy commented Jul 3, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

chenyangjun-xy commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Root Cause

Why This Change Was Made

Changes

Evidence

Verification environment

Regression tests (17/17, 1 new)

Real-process fault injection: open succeeds, writeFile fails

User Impact

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chenyangjun-xy commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

steipete commented Jul 3, 2026 •

edited

Loading