Skip to content

fix: cleanup resources aggresively on vite-plugin#12583

Open
anonrig wants to merge 2 commits intomainfrom
yagiz/cleanup-aggresively
Open

fix: cleanup resources aggresively on vite-plugin#12583
anonrig wants to merge 2 commits intomainfrom
yagiz/cleanup-aggresively

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Feb 17, 2026

An attempt to cleanup resources more aggresively on vite-plugin.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: impossible to test in an isolated way
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: no user facing change.

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


Open with Devin

@anonrig anonrig requested a review from a team as a code owner February 17, 2026 16:10
@changeset-bot
Copy link

changeset-bot bot commented Feb 17, 2026

🦋 Changeset detected

Latest commit: 334aa84

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

@claude
Copy link
Contributor

claude bot commented Feb 17, 2026

Claude finished @anonrig's task —— View job


Changeset Review

Reviewing changeset files...

Tasks:

  • Read changeset guidelines
  • Validate .changeset/quiet-taxis-rest.md
  • Check for cute animal images

All changesets look good

The changeset .changeset/quiet-taxis-rest.md passes all validation criteria:

  • ✅ Version type: patch is appropriate for cleanup/memory improvements
  • ✅ Changelog quality: Clear, meaningful description explaining the purpose (cleanup listeners and reduce memory growth)
  • ✅ No markdown header violations (no h1/h2/h3 headers used)
  • ✅ Not analytics-related
  • ✅ Not an experimental feature requiring opt-in notes

devin-ai-integration[bot]

This comment was marked as resolved.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 17, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@12583

wrangler

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

commit: 334aa84

@anonrig anonrig force-pushed the yagiz/cleanup-aggresively branch from 081c544 to d74ff9d Compare February 17, 2026 16:16
@anonrig anonrig force-pushed the yagiz/cleanup-aggresively branch from d74ff9d to 23fbf99 Compare February 17, 2026 16:20
devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 14 additional findings in Devin Review.

Open in Devin Review

Comment on lines +174 to +184
const cleanupWebSocket = handleWebSocket(
viteDevServer.httpServer,
ctx.miniflare,
entryWorkerName
);
viteDevServer.httpServer.once("close", () => {
cleanupWebSocket();
hotListenerCleanup?.();
hotListenerCleanup = undefined;
void disposeRemoteProxySessions();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 WebSocket upgrade handler accumulates on each server restart without cleanup

On every Vite dev server restart, configureServer is called again, and handleWebSocket registers a new upgrade handler on the same httpServer (line 174). However, the returned cleanupWebSocket function is only invoked inside httpServer.once("close", ...) (line 179), which only fires when the HTTP server is fully shut down — not on restart. Vite reuses the same HTTP server across restarts.

Root Cause and Impact

The PR correctly introduces cleanup for hotListenerCleanup at the top of the configureServer block (lines 109-110) using a persisted closure variable:

hotListenerCleanup?.();
hotListenerCleanup = undefined;

But no analogous pattern exists for the WebSocket handler. The cleanupWebSocket reference is a local variable inside configureServer and is lost on each subsequent call. After N restarts, there are N active upgrade handlers on the same httpServer, each calling miniflare.dispatchFetch for every incoming WebSocket upgrade request.

Impact: Each WebSocket upgrade request from a client will be processed by all accumulated handlers, potentially causing duplicate connections, wasted resources, and unexpected behavior. This directly undermines the PR's goal of aggressive resource cleanup.

Prompt for agents
In packages/vite-plugin-cloudflare/src/plugins/dev.ts, add a persisted cleanup variable for the WebSocket handler, similar to how hotListenerCleanup is managed. Specifically: 1. Add a `let webSocketCleanup: (() => void) | undefined;` declaration next to `hotListenerCleanup` around line 42. 2. At the beginning of the `if (viteDevServer.httpServer)` block (around line 173), call `webSocketCleanup?.()` to clean up the previous handler before registering a new one. 3. After calling `handleWebSocket(...)` at line 174, store the returned cleanup function: `webSocketCleanup = cleanupWebSocket;`. This mirrors the pattern used for hotListenerCleanup at lines 109-110.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

1 participant

Comments