fix(envd): avoid Start deadlock after request cancellation - #3256
Conversation
Start sent its bootstrap event directly to the unbuffered channel returned by MultiplexedChannel.Fork. If the stream sender observed request cancellation first, no receiver remained and the handler blocked on the send, preventing its process-event subscriptions from being released. Replace the unnecessary single-subscriber multiplexer with a one-slot channel so bootstrap publication cannot block after cancellation. The regression test exercises this ordering through the Connect handler and verifies that server shutdown is no longer held open by the stuck request.
PR SummaryMedium Risk Overview The bootstrap path no longer uses a single-subscriber multiplexer on an unbuffered fork; it uses a one-slot buffered channel so emitting the start event cannot block after the sender goroutine has already exited on cancellation. A regression test cancels the request at handler entry and asserts httptest server close completes within a timeout. Version bumps to 0.6.10. Reviewed by Cursor Bugbot for commit 843de1c. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Code Review
This pull request replaces a multiplexed channel with a buffered channel of size 1 in handleStart to prevent blocking when a receiver goroutine exits early due to a cancelled context. It also updates the test helper newTestService to support middleware and adds a new test TestStart_CancelBeforeStartEvent to verify this behavior. Feedback was provided regarding the middleware wrapping order in the test helper, which currently executes middlewares in reverse (right-to-left) order.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
❌ 1 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Small, targeted deadlock fix with a solid regression test.
Extended reasoning...
Overview
Touches only packages/envd/internal/services/process/start.go (~10 lines), a new regression test in start_test.go, and the required envd version bump (0.6.9 → 0.6.10). The fix replaces handler.MultiplexedChannel — which was being used with a single subscriber — with a plain chan rpc.ProcessEvent_Start of buffer 1. This ensures the bootstrap send after proc.Start() can never block if the sender goroutine has already exited via ctx.Done().
Security risks
None. The change is confined to intra-handler goroutine synchronization inside envd; no auth, crypto, or network-visible surface is affected.
Level of scrutiny
Low-to-moderate. Envd runs inside guest microVMs and the handler is on a well-trafficked code path, but the semantic change is narrow: buffered channel semantics for a single-shot bootstrap event. Correctness verified by inspection — if the sender goroutine wins ctx.Done(), the buffered event is silently dropped (correct, since the client has disconnected); if it wins the start path, behavior is identical to before. The <-exitChan join at the end still guarantees the outer handler waits for the sender.
Other factors
The regression test cancels the request context at HTTP handler entry via middleware, which reliably exercises the racing ordering, and bounds the server close in a goroutine with a 20s watchdog — a stuck handler would fail loudly. The PR description accurately describes both the bug and the fix. No outstanding reviewer feedback.
🤖 I have created a release *beep* *boop* --- ## 0.0.1 (2026-07-29) ### Features * **envd:** add --no-cgroups flag to disable cgroup management ([#2811](#2811)) ([e10814c](e10814c)) * **envd:** add optional EntryInfo to watch FilesystemEvent ([#2930](#2930)) ([bbbc7c8](bbbc7c8)) * **envd:** allow opting into watching network mounts ([#2982](#2982)) ([9799dd0](9799dd0)) * **envd:** give envd realtime IO priority, reset for user processes ([#2681](#2681)) ([f4bd1b2](f4bd1b2)) * **envd:** split collapse stats into real migrations vs already-huge ([#3021](#3021)) ([0d77614](0d77614)) * **envd:** support user-defined file metadata via xattrs ([#2732](#2732)) ([da8fbe4](da8fbe4)) * freeze user cgroup across pause/resume to keep envd /init responsive ([#2688](#2688)) ([eceb741](eceb741)) * **orch:** collapse envd's heap into 2 MiB hugepages before pause to cut cold-resume faults ([#2997](#2997)) ([6677f73](6677f73)) * **orch:** distro-aware template base-image provisioning ([#3411](#3411)) ([f8c7b5b](f8c7b5b)) ### Bug Fixes * added envd to artifact repository ([#3432](#3432)) ([6c4f0e2](6c4f0e2)) * correct 3 CVES ([#3218](#3218)) ([076823b](076823b)) * **envd:** avoid Start deadlock after request cancellation ([#3256](#3256)) ([04317f8](04317f8)) * **envd:** bound the in-memory logs queue ([#2676](#2676)) ([05c9939](05c9939)) * **envd:** discard output when no subscriber is connected ([#2639](#2639)) ([8cf1795](8cf1795)) * **envd:** fall back to lazy unmount when forced NFS umount fails ([#2683](#2683)) ([5346a0d](5346a0d)) * **envd:** ignore closed pty read errors ([#2769](#2769)) ([6118672](6118672)) * **envd:** include suppressed count in exporter error logs ([#2680](#2680)) ([35c1141](35c1141)) * **envd:** make /init lock ctx-aware to prevent retry pile-up ([#2702](#2702)) ([173afd4](173afd4)) * **envd:** make CA install lock ctx-aware ([#2690](#2690)) ([83ee89f](83ee89f)) * **envd:** replace env vars in /init instead of merging ([#2706](#2706)) ([1b52e9a](1b52e9a)) * **envd:** replace time.Sleep with ticker in ScanAndBroadcast for prompt shutdown ([#3374](#3374)) ([002fd9f](002fd9f)) * **envd:** self-heal MMDS routing on /init lookup failure ([#2701](#2701)) ([90944d5](90944d5)) * **envd:** stop freezing socat cgroup across pause/resume ([#2923](#2923)) ([8b6f2b9](8b6f2b9)) * **envd:** stop misleading CA install cancel errors on rapid /init ([#3206](#3206)) ([91d09e4](91d09e4)) * **envd:** suppress repeat MMDS poll failures ([#2678](#2678)) ([73d691a](73d691a)) * **envd:** tolerate busy tmpfs cleanup in tests ([#2938](#2938)) ([a485834](a485834)) * **envd:** use constant-time comparison for signature validation ([#3145](#3145)) ([fcf92fa](fcf92fa)) * **envd:** use WithoutCancel for CA cleanup goroutine ctx ([#3207](#3207)) ([ee7bf84](ee7bf84)) ### Performance Improvements * **envd:** stop logging streamed payload content ([#2755](#2755)) ([db3868c](db3868c)) * **sandbox:** keep envd logging out of journald ([#2675](#2675)) ([f6943ca](f6943ca)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: e2b-release-please[bot] <298072688+e2b-release-please[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## 0.0.1 (2026-07-29) ### Features * **envd:** add --no-cgroups flag to disable cgroup management ([#2811](#2811)) ([e10814c](e10814c)) * **envd:** add optional EntryInfo to watch FilesystemEvent ([#2930](#2930)) ([bbbc7c8](bbbc7c8)) * **envd:** allow opting into watching network mounts ([#2982](#2982)) ([9799dd0](9799dd0)) * **envd:** give envd realtime IO priority, reset for user processes ([#2681](#2681)) ([f4bd1b2](f4bd1b2)) * **envd:** split collapse stats into real migrations vs already-huge ([#3021](#3021)) ([0d77614](0d77614)) * **envd:** support user-defined file metadata via xattrs ([#2732](#2732)) ([da8fbe4](da8fbe4)) * freeze user cgroup across pause/resume to keep envd /init responsive ([#2688](#2688)) ([eceb741](eceb741)) * **orch:** collapse envd's heap into 2 MiB hugepages before pause to cut cold-resume faults ([#2997](#2997)) ([6677f73](6677f73)) * **orch:** distro-aware template base-image provisioning ([#3411](#3411)) ([1abece1](1abece1)) ### Bug Fixes * added envd to artifact repository ([#3432](#3432)) ([b7024ba](b7024ba)) * correct 3 CVES ([#3218](#3218)) ([076823b](076823b)) * **envd:** avoid Start deadlock after request cancellation ([#3256](#3256)) ([04317f8](04317f8)) * **envd:** bound the in-memory logs queue ([#2676](#2676)) ([05c9939](05c9939)) * **envd:** discard output when no subscriber is connected ([#2639](#2639)) ([8cf1795](8cf1795)) * **envd:** fall back to lazy unmount when forced NFS umount fails ([#2683](#2683)) ([5346a0d](5346a0d)) * **envd:** ignore closed pty read errors ([#2769](#2769)) ([6118672](6118672)) * **envd:** include suppressed count in exporter error logs ([#2680](#2680)) ([35c1141](35c1141)) * **envd:** make /init lock ctx-aware to prevent retry pile-up ([#2702](#2702)) ([173afd4](173afd4)) * **envd:** make CA install lock ctx-aware ([#2690](#2690)) ([83ee89f](83ee89f)) * **envd:** replace env vars in /init instead of merging ([#2706](#2706)) ([1b52e9a](1b52e9a)) * **envd:** replace time.Sleep with ticker in ScanAndBroadcast for prompt shutdown ([#3374](#3374)) ([002fd9f](002fd9f)) * **envd:** self-heal MMDS routing on /init lookup failure ([#2701](#2701)) ([90944d5](90944d5)) * **envd:** stop freezing socat cgroup across pause/resume ([#2923](#2923)) ([8b6f2b9](8b6f2b9)) * **envd:** stop misleading CA install cancel errors on rapid /init ([#3206](#3206)) ([91d09e4](91d09e4)) * **envd:** suppress repeat MMDS poll failures ([#2678](#2678)) ([73d691a](73d691a)) * **envd:** tolerate busy tmpfs cleanup in tests ([#2938](#2938)) ([a485834](a485834)) * **envd:** use constant-time comparison for signature validation ([#3145](#3145)) ([fcf92fa](fcf92fa)) * **envd:** use WithoutCancel for CA cleanup goroutine ctx ([#3207](#3207)) ([ee7bf84](ee7bf84)) ### Performance Improvements * **envd:** stop logging streamed payload content ([#2755](#2755)) ([db3868c](db3868c)) * **sandbox:** keep envd logging out of journald ([#2675](#2675)) ([f6943ca](f6943ca)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: e2b-release-please[bot] <298072688+e2b-release-please[bot]@users.noreply.github.com>
Start sent its bootstrap event directly to the unbuffered channel returned by MultiplexedChannel.Fork. If the stream sender observed request cancellation first, no receiver remained and the handler blocked on the send, preventing its process-event subscriptions from being released.
Replace the unnecessary single-subscriber multiplexer with a one-slot channel so bootstrap publication cannot block after cancellation. The regression test exercises this ordering through the Connect handler and verifies that server shutdown is no longer held open by the stuck request.