memory: lock heartbeat refreshes a lock file the process no longer owns

Đã đóng Phù hợp với người mới
  • Có gì không ổn?
#5,418 1 bình luận 0 reaction 1 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@cliffhall đang làm issue này rồi.

Từ ngày 5/11/2026.

  • #5462 của @cliffhall — đã merge

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
68/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
nodejs, typescript
Lĩnh vực
backend

Hướng nghiên cứu

Bắt đầu từ withFileLock() trong src/memory/index.ts, quanh setInterval của heartbeat (dòng 304-322) và việc đọc lại token trong đường giải phóng finally, là kiểm tra quyền sở hữu cần làm theo. Hiện tại heartbeat gọi fs.utimes trên lock mà không kiểm tra ai là chủ sở hữu. Hoàn thành khi heartbeat chỉ làm mới lock trong lúc nội dung của nó vẫn bằng token của chính nó, và dừng khi hai giá trị khác nhau. Xác nhận bằng một lần chạy với hai bên giữ lock, trong đó một lock thay thế được lấy giữa chừng thao tác, như issue mô tả.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

bug server-memory v2
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's fs.utimes hit 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 staleMs deadline 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 own timeoutMs and 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.)

Ngôn ngữ chính
TypeScript
Star
91.7k
Fork
11.9k
Merge trung bình
3 giờ 30 phút
Pull request đã merge (30 ngày)
204

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của modelcontextprotocol/servers

Tất cả issue của modelcontextprotocol/servers

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.