memory: lock heartbeat refreshes a lock file the process no longer owns
维护者通常 1 天内回复
评估
- 难度
- 2/5
- 预计耗时
- 1-3 小时
- 新手友好度
- 68/100
- Issue 类型
- 缺陷
- 描述清晰度
- 描述清楚
- 活跃度
- 活跃
- 技术栈
- nodejs, typescript
- 领域
- backend
调研方向
从 withFileLock()(位于 src/memory/index.ts)开始,关注心跳的 setInterval(第 304-322 行)以及 finally 释放路径中重新读取 token 的部分,这是需要照搬的所有权检查。心跳目前会对锁调用 fs.utimes,但没有检查锁属于谁。完成标准是:只有锁的内容仍与其自身 token 相同时,心跳才刷新该锁;一旦两者不同,就停止刷新。按 issue 所述,在操作中途有替换锁被获取的双持有者运行场景下进行验证。
由索引模型根据 Issue 内容生成。
描述
| Field | Value |
|---|---|
| Labels (suggested) | bug, memory, concurrency |
| Reproduced on | commit a5587f2835dd4a2ba7f8fbcb6d8bdca6d5f4b8ba (default branch at the time of analysis) |
| Introduced by | commit 4ddd6ce6d89ad3a9b5a4294fadc8bd2d993fc5e3 — "fix(memory): serialize stale-lock breaking behind a guard" |
Summary
withFileLock() in src/memory/index.ts keeps its lock alive with a heartbeat interval that calls
fs.utimes(lockPath, ...) unconditionally. The release path a few lines below already knows that the
lock file may have changed hands — it re-reads the file and only unlinks it when the contents are still
its own token — but the heartbeat has no such check. If the lock was broken as stale and re-taken by
another waiter (or released and re-created), the original holder keeps refreshing the replacement
owner's lock file for as long as its own operation runs.
Affected code
src/memory/index.ts, lines 304–322 at a5587f2835dd:
// Keep the lock fresh while the operation runs, so a mutation that outlasts
// staleMs (a large graph, a slow disk) is not mistaken for an abandoned one.
const heartbeat = setInterval(() => {
const now = new Date();
fs.utimes(lockPath, now, now).catch(ignoreError); // <- no ownership check
}, staleMs / 2);
heartbeat.unref();
try {
return await operation();
} finally {
clearInterval(heartbeat);
// Release only our own lock: if it was broken as stale while we held it,
// the file may now belong to another process.
const current = await fs.readFile(lockPath, "utf-8").catch(() => null);
if (current === token) { // <- release DOES check
await fs.unlink(lockPath).catch(ignoreError);
}
}
The comment on the release path states the exact invariant the heartbeat violates: after a stale break
"the file may now belong to another process".
Expected behavior
After the lock has been broken and re-taken, A's heartbeat must leave B's lock file alone. A must not
touch a lock file whose contents are not its token (the release path already enforces this).
Actual behavior
Holder A is inside withFileLock with staleMs = 4000 (heartbeat every 2000 ms) when the lock file's mtime
is backdated by 60 s. Holder B (staleMs = 59000; its 29.5 s heartbeat never fires inside the ~2.6 s
observation) judges the lock stale, breaks it and re-takes it with a new token — and A's own heartbeat still
fires inside that window:
{"mode":"bug","beatsOnReplacement":1,"beatsOnOther":0,"replacementTokenDiffers":true,
"lockSurvivedARelease":true,"lockAfterBRelease":"gone"}
HEARTBEAT_REFRESHED_REPLACEMENT_LOCK
beatsOnReplacement: 1— A'sfs.utimeshit the lock file whose contents were B's token.beatsOnOther: 0— the observation is clean; the single hit is attributable to A.replacementTokenDiffers: true— B's token is demonstrably different from A's.lockSurvivedARelease: true— A's release correctly declined to delete B's lock.
Control: adding the ownership re-read to the heartbeat (the guard the release path already uses) makes the same
run report {"mode":"control","beatsOnReplacement":0,...} — CONTROL_NO_REPLACEMENT_TOUCH.
Why it matters
The heartbeat exists so a legitimately slow mutation is not mistaken for an abandoned one. Once the
lock has changed hands, the same heartbeat instead keeps a lock file fresh on behalf of a process that
may already be dead:
- A's remaining work can extend B's
staleMsdeadline indefinitely. If B crashes while A is still
running, a third process waiting on the lock will see a continuously refreshed mtime and will not
break the abandoned lock; it waits until its owntimeoutMsand then fails with
Timed out after … ms waiting for the memory file lock. - The effect is the opposite of the intent: instead of making progress possible, the heartbeat delays
recovery from a crashed peer by the length of the previous holder's operation.
This is a robustness/availability bug rather than a data-corruption bug: the token-guarded release
still prevents deleting someone else's lock.
Suggested fix
Revalidate ownership before refreshing, exactly as the release path does — and keep the check cheap:
const heartbeat = setInterval(() => {
const now = new Date();
void fs.readFile(lockPath, "utf-8")
.then((current) => {
if (current !== token) { // lock changed hands: stop refreshing it
clearInterval(heartbeat);
return;
}
return fs.utimes(lockPath, now, now);
})
.catch(ignoreError);
}, staleMs / 2);
(Terminating the interval on token mismatch also stops the pointless reads. Holding the created
FileHandle open and using handle.utimes() is an alternative, but it changes unlink semantics on
Windows, where an open handle keeps the file from being removed — the token re-read keeps the current
behaviour.)
- 主要语言
- TypeScript
- 星标
- 91.7k
- 派生
- 11.9k
- 平均合并
- 3 小时 30 分钟
- 30 天内合并 PR
- 204
环境准备
- 没有 Dockerfile 或 Docker Compose 文件
- 有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
modelcontextprotocol/servers 的其他 Issue
-
bug v2
难度 1/5 1 小时以内 新手友好度 88/100
modelcontextprotocol/servers#5483 ·
维护者通常 1 天内回复
-
bug v2
难度 2/5 1-3 小时 新手友好度 65/100
modelcontextprotocol/servers#5482 ·
维护者通常 1 天内回复
-
chore dependencies server-filesystem v2
难度 2/5 1-3 小时 新手友好度 78/100
modelcontextprotocol/servers#5481 · 1 条评论 ·
维护者通常 1 天内回复
-
chore v2
难度 4/5 3-5 天 新手友好度 45/100
modelcontextprotocol/servers#5463 ·
维护者通常 1 天内回复
-
bug server-filesystem v2
难度 5/5 一周以上 新手友好度 25/100
modelcontextprotocol/servers#5260 · 3 条评论 ·
维护者通常 1 天内回复
查看 modelcontextprotocol/servers 的全部 Issue
相似的 Issue
-
community first-timers-only good first issue hacktoberfest help wanted low hanging fruit up-for-grabs
难度 1/5 1 小时以内 新手友好度 83/100
lingdojo/kana-dojo#33701 · 1 条评论 · 5 个 reaction ·
维护者通常 1 天内回复
-
tech-debt
难度 2/5 1-3 小时 新手友好度 68/100
rookslog/zlibrary-mcp#408 ·
维护者通常 1 天内回复
-
mp4box.js — 20-byte `sgpd` box with large `entry_count` causes unbounded loop and heap OOM abort未关闭
难度 2/5 1-3 小时 新手友好度 77/100
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 78/100
维护者通常 1 天内回复
-
status: needs triage
难度 2/5 1-3 小时 新手友好度 64/100
mastra-ai/mastra#29641 · 1 条评论 ·
维护者通常 1 天内回复