Skip to content

[vite-plugin] Don't advertise a dev session's runtime until it is final - #15024

Open
petebacondarwin wants to merge 1 commit into
mainfrom
fix/defer-dev-registry-registration
Open

[vite-plugin] Don't advertise a dev session's runtime until it is final#15024
petebacondarwin wants to merge 1 commit into
mainfrom
fix/defer-dev-registry-registration

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

A vite dev session publishes a workerd debug port to the dev registry and then immediately replaces the runtime behind it, so another dev session can be left holding an address that no longer exists.

What happens

The Vite plugin brings workerd up twice while starting: once to discover each Worker's exports by running it, then again with a config assembled from what it found (dev.ts, the hasChanged branch). The first runtime is advertised in the dev registry and then torn down. A peer that resolves the Worker during that window gets a debug port that is about to disappear — and on Windows, a peer with a tail_consumers edge to it can abort its own runtime with *** std::terminate() called with no exception (reported upstream as cloudflare/workerd#6913).

Miniflare already unregisters before restarting, but peers only learn by watching the registry directory (polling on Windows) and then pushing the update into their own proxy Worker. That propagation loses the race against a ~150ms restart, so this closes the window rather than trying to shrink it.

The change

  • miniflare gains unsafeDeferDevRegistryRegistration, which holds back advertising this instance's Workers, and unsafeRegisterInDevRegistry() to release the hold once the runtime is final. The hold is re-armed by each setOptions() that asks to defer, so a dev server restart gets the same protection. Reading the registry is untouched, so a starting session still resolves Workers from sessions already running.
  • @cloudflare/vite-plugin defers registration and releases it at the end of configureServer, unconditionally — so it applies whether or not the export types turned out to differ.

The fixture test asserts the invariant directly: exactly one debug port is ever advertised for a Worker during startup. Without the fix it observes two.

Scope — this does not fix the Windows flake

fixtures/dev-registry is currently skipped on Windows (#15018). I temporarily re-enabled it and ran the vite dev <-> vite dev suite four times on a Windows runner to check whether this change is enough to lift that skip. It is not. One of four rounds of supports exported handler fetch over service binding still timed out at 50s, and three std::terminate aborts still occurred.

So this is a real bug with a targeted fix and a regression test, but the skip in #15018 should stay until the remaining trigger is found. I'm continuing to investigate that separately; the leading suspect is that #registerWorkers advertises every named Worker, and the plugin names its internals with global constants (__asset-worker__, __router-worker__, __vite_proxy_worker__), so concurrent vite sessions overwrite and delete each other's entries in the shared registry.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: the new Miniflare options are unsafe* internal APIs for consumers that stage their runtime startup, and the user-visible effect is a bug fix.

Four Miniflare tests plus one fixture test, all confirmed to fail without the change.

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

a very serious owl

Note

This is a contribution from an AI agent: OpenCode, claude-opus-5.

@changeset-bot

changeset-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c1da2d7

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

This PR includes changesets to release 8 packages
Name Type
@cloudflare/vite-plugin Patch
miniflare Minor
@cloudflare/deploy-helpers Patch
@cloudflare/pages-shared Patch
@cloudflare/remote-bindings Patch
@cloudflare/runtime-types Patch
@cloudflare/vitest-pool-workers Patch
wrangler Patch

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

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Aug 4, 2026
@workers-devprod
workers-devprod requested review from a team and edmundhung and removed request for a team August 4, 2026 20:59
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/tidy-donkeys-listen.md: [@cloudflare/wrangler]
  • fixtures/dev-registry/tests/dev-registry.test.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/index.ts: [@cloudflare/wrangler]
  • packages/miniflare/src/plugins/core/index.ts: [@cloudflare/wrangler]
  • packages/miniflare/test/dev-registry.spec.ts: [@cloudflare/wrangler]
  • packages/vite-plugin-cloudflare/src/miniflare-options.ts: [@cloudflare/wrangler]
  • packages/vite-plugin-cloudflare/src/plugins/dev.ts: [@cloudflare/wrangler]

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@ask-bonk

ask-bonk Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The config includes an auxiliaryWorkers with internal-durable-object. This means the vite session runs two Miniflare-managed workers. Note the fixture test asserts only ONE debug port for worker-entrypoint. Each Miniflare instance has its own debug port, and both the main and auxiliary workers share the same Miniflare instance (auxiliaryWorkers run in the same Miniflare). So the single worker-entrypoint file's debugPortAddress reflects that one instance's debug port. Good.

One thing worth verifying: worker-entrypoint has tail_consumers pointing at exported-handler-with-assets, which is not running in this test — but that doesn't matter for the "one debug port" assertion.

The implementation and tests are correct and well-reasoned. Let me do a final check on whether there could be an issue with unsafeRegisterInDevRegistry being called when the option is NOT set (idempotent no-op claim). It sets #devRegistryRegistrationReleased = true and calls #registerWorkers(). If option not set, workers were already registered eagerly during init, so calling again just re-registers same data — harmless. The doc comment says "a no-op unless unsafeDeferDevRegistryRegistration is set" — slightly inaccurate (it re-registers), but harmless. Not actionable.

Let me verify the Mutex has hasWaiting / drained and that runWith returning inside another runWith isn't a deadlock risk. unsafeRegisterInDevRegistry awaits this.ready (drains mutex) THEN acquires mutex via runWith — no nesting, no deadlock.

The change is well-designed, correct, tested, and documented. Let me confirm there are no lint concerns like floating promises in the new code (the fixture sampler IIFE is awaited via await sampler).

The code is clean. No actionable logic bugs, security issues, backward-compat violations, or incorrect API behavior. The implementation correctly:

  • Defers only self-advertisement while preserving registry reads
  • Re-arms the hold on each setOptions that requests deferral
  • Handles disabled-registry and no-debug-port cases gracefully
  • Serializes via the runtime mutex without deadlock
  • Is covered by targeted Miniflare unit tests and a fixture regression test

LGTM

github run

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

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

Open in Devin Review

@petebacondarwin
petebacondarwin force-pushed the fix/defer-dev-registry-registration branch from d699b3f to cb72c40 Compare August 4, 2026 21:08
@pkg-pr-new

pkg-pr-new Bot commented Aug 4, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/@cloudflare/autoconfig@15024

@cloudflare/build-output-utils

npm i https://pkg.pr.new/@cloudflare/build-output-utils@15024

@cloudflare/config

npm i https://pkg.pr.new/@cloudflare/config@15024

create-cloudflare

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

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@15024

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-functions

npm i https://pkg.pr.new/@cloudflare/pages-functions@15024

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-auth

npm i https://pkg.pr.new/@cloudflare/workers-auth@15024

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

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

wrangler

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

commit: c1da2d7

The plugin starts workerd twice: once to discover each Worker's exports by
running it, then again with a config built from what it found. The first
runtime was published to the dev registry and then torn down, leaving peers
holding a debug port that no longer existed. On Windows a peer with a
tail_consumers edge to it aborts its own workerd with std::terminate.

Miniflare gains unsafeDeferDevRegistryRegistration to hold back
self-advertisement, and unsafeRegisterInDevRegistry() to release it once the
runtime is final. Reading the registry is unaffected, so a starting session
still resolves Workers from sessions already running.
@petebacondarwin
petebacondarwin force-pushed the fix/defer-dev-registry-registration branch from cb72c40 to c1da2d7 Compare August 4, 2026 21:40
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.

2 participants