Fix potential memory leak in module runner

[jamesopstad]

Follow up to #12953

Clean up module runner callbacks on errors to fix potential memory leak.

---

• Tests
  • Tests included/updated
  • Automated tests not possible - manual testing has been completed as follows:
  • Additional testing not necessary because: covered by existing tests
• Public documentation
  • Cloudflare docs PR(s):
  • Documentation not necessary because: bug fix

A picture of a cute animal (not mandatory, but encouraged)

---

[changeset-bot]

⚠️ No Changeset found

Latest commit: 2f10f6e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[workers-devprod]

Codeowners approval required for this PR:

• ✅ @cloudflare/wrangler

Show detailed file reviewers

[github-actions]

⚠️ Issues found

File: .changeset/loose-humans-join.md

Issue: Missing body/description

The changeset contains only a title with no body paragraph. Per the guidelines in .changeset/README.md:

BODY: One or more paragraphs explaining the reason for the change and anything notable about the approach. Aim for more than one sentence but less than three paragraphs to keep it succinct and useful.

A patch-level bug fix description like "Fix potential memory leak in module runner" should explain:

• What caused the memory leak (e.g., which resource was not being released)
• Under what conditions it occurred
• How it was fixed

Example of what the changeset should look like:

---
"@cloudflare/vite-plugin": patch
---

Fix potential memory leak in module runner

Previously, [describe what was not cleaned up, e.g., "event listeners / references were not released when the module runner was disposed"]. This could cause memory to grow unboundedly in long-running dev sessions. The fix ensures [brief description of the fix].

The version type (patch) is appropriate for a bug fix.

[ask-bonk]

Now I have a thorough understanding of the changes. Let me analyze the PR:

Changes summary:

1. In runInRunnerObject: Wraps the stub call and result retrieval in a try/finally block so that both pendingCallbacks and callbackResults are cleaned up regardless of whether the call succeeds or throws.

2. In executeCallback: Removes the pendingCallbacks.delete(id) call, since cleanup is now centralized in the finally block of runInRunnerObject.

Analysis:

• Previously, pendingCallbacks.delete(id) was called in executeCallback (the DO method), and callbackResults.delete(id) was called after the await in runInRunnerObject. If stub.executeCallback(id) threw an error, neither pendingCallbacks nor callbackResults would be cleaned up (the callback entry would remain in the map forever since executeCallback only deletes on the happy path before the await callback() call, and if executeCallback threw, the code after await stub.executeCallback(id) in runInRunnerObject would never run).

• The fix moves all cleanup to a finally block in runInRunnerObject, which is the correct place since that function is the one that inserts into both maps.

• The removal of pendingCallbacks.delete(id) from executeCallback is correct — the callback is still looked up via pendingCallbacks.get(id) (line 156), and the finally block in runInRunnerObject will handle deletion after executeCallback completes (or throws).

• There's a subtlety: if executeCallback succeeds (the callback runs and stores a result), but then the return path in runInRunnerObject is in the try block, the finally will delete the result from callbackResults after it's been retrieved by callbackResults.get(id) on line 53. This is fine because the value is already captured by the return statement before finally runs.

This is a clean, correct fix. No logic bugs, no edge cases missed.

LGTM

github run

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

[pkg-pr-new]

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13087

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13087

miniflare

npm i https://pkg.pr.new/miniflare@13087

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13087

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13087

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13087

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13087

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13087

wrangler

npm i https://pkg.pr.new/wrangler@13087

commit: 2f10f6e