providerCommandTimeout is documented and reported as a 5 second bound, but it is a floor rather than a bound. I hit this while reproducing the Windows smoke flakes fixed in #810, and it is a separate, user-facing problem.
Observed
Running the provider-command tests under CPU contention, LoadProviderCommand returned after 19.7s, and on another run after 106s, against a 5s timeout:
returned after 19.7147647s, want before the background child finishes naturally after 10s
returned after 1m46.2165326s, want before the background child finishes naturally after 10s
The magnitudes came from synthetic load and are not representative of CI, but the code ordering below is wrong regardless of load.
Three causes, all in internal/config/command.go
1. Process creation is outside the timer. startCommandProcess runs at command.go:54, but the timer is not armed until command.go:65. Everything before that is unbounded. On Windows that phase includes CREATE_SUSPENDED, the job-object setup, and the system-wide thread snapshot in resumeMainThread (internal/config/process_windows.go), none of which is fast on a cold or AV-heavy box.
2. The drain after Terminate() is unbounded. On the timer path, command.go:85 does a bare <-done after proc.Terminate(). If the tree is slow to die, the call blocks for as long as that takes, well past the deadline that just fired.
3. The timeout path discards all diagnostics. command.go:19-21 returns a hardcoded string. commandOutput(stderr) is only reached on the non-timeout branch, and the underlying error is dropped. A user whose provider command is slow gets no stderr and no clue which of the two timeout paths they hit.
Why it matters
A provider command runs during config resolution, so this is on the startup path. A user on a slow Windows box can have Zero appear to hang for far longer than the error text promises, then get a message with no diagnostics attached.
Notes
I have deliberately not fixed this in #810. That PR stops the tests from racing the deadline and is otherwise behaviour-preserving; changing what the timeout actually bounds is a real behaviour change and deserves its own review. #810 does make one narrow change here, wrapping instead of replacing on the WaitDelay path so the two timeout paths can finally be told apart, which is a prerequisite for fixing 3 properly.
providerCommandTimeoutis documented and reported as a 5 second bound, but it is a floor rather than a bound. I hit this while reproducing the Windows smoke flakes fixed in #810, and it is a separate, user-facing problem.Observed
Running the provider-command tests under CPU contention,
LoadProviderCommandreturned after 19.7s, and on another run after 106s, against a 5s timeout:The magnitudes came from synthetic load and are not representative of CI, but the code ordering below is wrong regardless of load.
Three causes, all in
internal/config/command.go1. Process creation is outside the timer.
startCommandProcessruns atcommand.go:54, but the timer is not armed untilcommand.go:65. Everything before that is unbounded. On Windows that phase includesCREATE_SUSPENDED, the job-object setup, and the system-wide thread snapshot inresumeMainThread(internal/config/process_windows.go), none of which is fast on a cold or AV-heavy box.2. The drain after
Terminate()is unbounded. On the timer path,command.go:85does a bare<-doneafterproc.Terminate(). If the tree is slow to die, the call blocks for as long as that takes, well past the deadline that just fired.3. The timeout path discards all diagnostics.
command.go:19-21returns a hardcoded string.commandOutput(stderr)is only reached on the non-timeout branch, and the underlying error is dropped. A user whose provider command is slow gets no stderr and no clue which of the two timeout paths they hit.Why it matters
A provider command runs during config resolution, so this is on the startup path. A user on a slow Windows box can have Zero appear to hang for far longer than the error text promises, then get a message with no diagnostics attached.
Notes
I have deliberately not fixed this in #810. That PR stops the tests from racing the deadline and is otherwise behaviour-preserving; changing what the timeout actually bounds is a real behaviour change and deserves its own review. #810 does make one narrow change here, wrapping instead of replacing on the
WaitDelaypath so the two timeout paths can finally be told apart, which is a prerequisite for fixing 3 properly.