fix(envd): replace time.Sleep with ticker in ScanAndBroadcast for prompt shutdown - #3374
Merged
arkamar merged 4 commits intoJul 24, 2026
Merged
Conversation
The tests depended on wall-clock timing (small exit budgets around real /proc scans via net.Connections), making them flaky on slow or loaded machines. The sleep-to-ticker change in ScanAndBroadcast is a straightforward select on scanExit that is verifiable by inspection, so the tests are not worth the flake risk.
The added line described the implementation rather than the contract; keep the function comment as it was on main.
arkamar
approved these changes
Jul 24, 2026
arkamar
left a comment
Member
There was a problem hiding this comment.
Thanks for the change, LGTM. I think the tests are not needed for this, as they could easily become flaky, at least as they were written. The timing assertions could false-fail on a loaded runner. The fix itself is a straightforward select on scanExit, verifiable by inspection.
I removed the additional comment from ScanAndBroadcast() as it described the implementation detail rather than the contract, and the original one-liner still covers what the function does.
I pushed these changes directly to your branch, along with the envd version bump to 0.6.11. The remaining diff is just the sleep→ticker change, which is exactly what we want here.
charlie-e2b
pushed a commit
that referenced
this pull request
Jul 29, 2026
🤖 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>
jakubno
pushed a commit
that referenced
this pull request
Aug 3, 2026
🤖 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #3356
Problem
ScanAndBroadcast()usedtime.Sleepinside adefaultselect case:Destroy()closesscanExit, but the goroutine only checksscanExitat the top of each iteration — before the sleep starts. Oncetime.Sleepis entered, a concurrentDestroy()call is not noticed until the sleep completes. WithportScannerInterval = 1 s, every envd shutdown was delayed by up to 1 second waiting for the port scanner goroutine.Fix
Replace
time.Sleepwithtime.NewTickersoscanExitand the tick interval are waited on simultaneously in a singleselect:Destroy()now unblocks the goroutine instantly, regardless of how far into the current interval it is.Using a
Ticker(instead oftime.After) also avoids allocating a new timer object on every loop iteration.Tests
Two new tests in
scan_test.go:TestScanAndBroadcastDestroyExitsPromptly— creates a scanner with a 5 s period, lets it enter the select block, callsDestroy(), and asserts the goroutine exits within 200 ms (not after 5 s).TestScanAndBroadcastDestroyBeforeSleep— callsDestroy()beforeScanAndBroadcaststarts, verifying the goroutine still exits promptly.Both pass in < 100 ms total.
/cc @jakubno @dobrac @ValentaTomas