From e2c23e0849759b4392c4024c2e10b5fc351bb4f4 Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Tue, 18 Jun 2024 15:43:51 -0300 Subject: [PATCH 1/3] Sets some best-effort timeouts Those functions used the background context. When wslservice.exe hangs/crashes, wsl.exe commands might hang for very long. We're setting some generous timeouts to couple with a variety of hardware. In the happy paths, 1/100th to 1/10th of those timeouts should be enough. --- internal/backend/windows/wslexe_windows.go | 25 ++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/internal/backend/windows/wslexe_windows.go b/internal/backend/windows/wslexe_windows.go index a4adb31..b307904 100644 --- a/internal/backend/windows/wslexe_windows.go +++ b/internal/backend/windows/wslexe_windows.go @@ -6,21 +6,29 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "os" "os/exec" "strings" + "time" "github.com/ubuntu/gowsl/internal/state" ) +// ErrWslTimeout is the error returned when wsl.exe commands don't respond in time. +var ErrWslTimeout = errors.New("wsl.exe did not respond: consider restarting wslservice.exe") + // Shutdown shuts down all distros // // It is analogous to // // `wsl.exe --Shutdown func (Backend) Shutdown() error { - _, err := wslExe(context.Background(), "--shutdown") + ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, ErrWslTimeout) + defer cancel() + + _, err := wslExe(ctx, "--shutdown") if err != nil { return fmt.Errorf("could not shut WSL down: %w", err) } @@ -33,7 +41,10 @@ func (Backend) Shutdown() error { // // `wsl.exe --Terminate ` func (Backend) Terminate(distroName string) error { - _, err := wslExe(context.Background(), "--terminate", distroName) + ctx, cancel := context.WithTimeoutCause(context.Background(), 3*time.Second, ErrWslTimeout) + defer cancel() + + _, err := wslExe(ctx, "--terminate", distroName) if err != nil { return fmt.Errorf("could not terminate distro %q: %w", distroName, err) } @@ -46,7 +57,10 @@ func (Backend) Terminate(distroName string) error { // // `wsl.exe --set-default ` func (Backend) SetAsDefault(distroName string) error { - _, err := wslExe(context.Background(), "--set-default", distroName) + ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, ErrWslTimeout) + defer cancel() + + _, err := wslExe(ctx, "--set-default", distroName) if err != nil { return fmt.Errorf("could not set %q as default: %w", distroName, err) } @@ -55,7 +69,10 @@ func (Backend) SetAsDefault(distroName string) error { // State returns the state of a particular distro as seen in `wsl.exe -l -v`. func (Backend) State(distributionName string) (s state.State, err error) { - out, err := wslExe(context.Background(), "--list", "--all", "--verbose") + ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, ErrWslTimeout) + defer cancel() + + out, err := wslExe(ctx, "--list", "--all", "--verbose") if err != nil { return s, fmt.Errorf("could not get states of distros: %w", err) } From 62215fbb7a6ff72221cfd58015da26247e025e08 Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Wed, 19 Jun 2024 09:16:44 -0300 Subject: [PATCH 2/3] More relaxed timeouts Minimum of 5s, with 10s for Shutdown which seems to do more work. --- internal/backend/windows/wslexe_windows.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/backend/windows/wslexe_windows.go b/internal/backend/windows/wslexe_windows.go index b307904..e36c78c 100644 --- a/internal/backend/windows/wslexe_windows.go +++ b/internal/backend/windows/wslexe_windows.go @@ -41,7 +41,7 @@ func (Backend) Shutdown() error { // // `wsl.exe --Terminate ` func (Backend) Terminate(distroName string) error { - ctx, cancel := context.WithTimeoutCause(context.Background(), 3*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrWslTimeout) defer cancel() _, err := wslExe(ctx, "--terminate", distroName) @@ -57,7 +57,7 @@ func (Backend) Terminate(distroName string) error { // // `wsl.exe --set-default ` func (Backend) SetAsDefault(distroName string) error { - ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrWslTimeout) defer cancel() _, err := wslExe(ctx, "--set-default", distroName) @@ -69,7 +69,7 @@ func (Backend) SetAsDefault(distroName string) error { // State returns the state of a particular distro as seen in `wsl.exe -l -v`. func (Backend) State(distributionName string) (s state.State, err error) { - ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrWslTimeout) defer cancel() out, err := wslExe(ctx, "--list", "--all", "--verbose") From 0edba5b614849256480fd59f4e66161a28f6163a Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Wed, 19 Jun 2024 09:26:10 -0300 Subject: [PATCH 3/3] Make the error value private Until we have a definite reason to expose it. --- internal/backend/windows/wslexe_windows.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/backend/windows/wslexe_windows.go b/internal/backend/windows/wslexe_windows.go index e36c78c..d16ec16 100644 --- a/internal/backend/windows/wslexe_windows.go +++ b/internal/backend/windows/wslexe_windows.go @@ -16,8 +16,8 @@ import ( "github.com/ubuntu/gowsl/internal/state" ) -// ErrWslTimeout is the error returned when wsl.exe commands don't respond in time. -var ErrWslTimeout = errors.New("wsl.exe did not respond: consider restarting wslservice.exe") +// errWslTimeout is the error returned when wsl.exe commands don't respond in time. +var errWslTimeout = errors.New("wsl.exe did not respond: consider restarting wslservice.exe") // Shutdown shuts down all distros // @@ -25,7 +25,7 @@ var ErrWslTimeout = errors.New("wsl.exe did not respond: consider restarting wsl // // `wsl.exe --Shutdown func (Backend) Shutdown() error { - ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, errWslTimeout) defer cancel() _, err := wslExe(ctx, "--shutdown") @@ -41,7 +41,7 @@ func (Backend) Shutdown() error { // // `wsl.exe --Terminate ` func (Backend) Terminate(distroName string) error { - ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, errWslTimeout) defer cancel() _, err := wslExe(ctx, "--terminate", distroName) @@ -57,7 +57,7 @@ func (Backend) Terminate(distroName string) error { // // `wsl.exe --set-default ` func (Backend) SetAsDefault(distroName string) error { - ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, errWslTimeout) defer cancel() _, err := wslExe(ctx, "--set-default", distroName) @@ -69,7 +69,7 @@ func (Backend) SetAsDefault(distroName string) error { // State returns the state of a particular distro as seen in `wsl.exe -l -v`. func (Backend) State(distributionName string) (s state.State, err error) { - ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, ErrWslTimeout) + ctx, cancel := context.WithTimeoutCause(context.Background(), 5*time.Second, errWslTimeout) defer cancel() out, err := wslExe(ctx, "--list", "--all", "--verbose")