From 83dbbf6889255c46af52307973854b0fda40d063 Mon Sep 17 00:00:00 2001 From: Sylvia Crowe Date: Wed, 1 Jan 2025 13:02:54 -0800 Subject: [PATCH 1/5] fix: remove context from wsl connection struct This fixes a bug where disconnecting the controller does not allow the controller to restart until the app reboots. --- pkg/shellexec/shellexec.go | 8 +++++--- pkg/wsl/wsl.go | 17 +++++++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go index d2cf64d498..6ee730cde6 100644 --- a/pkg/shellexec/shellexec.go +++ b/pkg/shellexec/shellexec.go @@ -148,10 +148,12 @@ func (pp *PipePty) WriteString(s string) (n int, err error) { } func StartWslShellProc(ctx context.Context, termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOptsType, conn *wsl.WslConn) (*ShellProc, error) { + shellProcCtx, cancelFn := context.WithCancel(ctx) + defer cancelFn() client := conn.GetClient() shellPath := cmdOpts.ShellPath if shellPath == "" { - remoteShellPath, err := wsl.DetectShell(conn.Context, client) + remoteShellPath, err := wsl.DetectShell(shellProcCtx, client) if err != nil { return nil, err } @@ -160,13 +162,13 @@ func StartWslShellProc(ctx context.Context, termSize waveobj.TermSize, cmdStr st var shellOpts []string log.Printf("detected shell: %s", shellPath) - err := wsl.InstallClientRcFiles(conn.Context, client) + err := wsl.InstallClientRcFiles(shellProcCtx, client) if err != nil { log.Printf("error installing rc files: %v", err) return nil, err } - homeDir := wsl.GetHomeDir(conn.Context, client) + homeDir := wsl.GetHomeDir(shellProcCtx, client) shellOpts = append(shellOpts, "~", "-d", client.Name()) var subShellOpts []string diff --git a/pkg/wsl/wsl.go b/pkg/wsl/wsl.go index 673bc1a73f..7f34586bfb 100644 --- a/pkg/wsl/wsl.go +++ b/pkg/wsl/wsl.go @@ -51,7 +51,6 @@ type WslConn struct { HasWaiter *atomic.Bool LastConnectTime int64 ActiveConnNum int - Context context.Context cancelFn func() } @@ -188,6 +187,13 @@ func (conn *WslConn) OpenDomainSocketListener() error { } func (conn *WslConn) StartConnServer() error { + ctx, cancelFn := context.WithCancel(context.Background()) + conn.WithLock(func() { + if conn.cancelFn != nil { + conn.cancelFn() + } + conn.cancelFn = cancelFn + }) var allowed bool conn.WithLock(func() { if conn.Status != Status_Connecting { @@ -200,7 +206,7 @@ func (conn *WslConn) StartConnServer() error { return fmt.Errorf("cannot start conn server for %q when status is %q", conn.GetName(), conn.GetStatus()) } client := conn.GetClient() - wshPath := GetWshPath(conn.Context, client) + wshPath := GetWshPath(ctx, client) rpcCtx := wshrpc.RpcContext{ ClientType: wshrpc.ClientType_ConnServer, Conn: conn.GetName(), @@ -210,7 +216,7 @@ func (conn *WslConn) StartConnServer() error { if err != nil { return fmt.Errorf("unable to create jwt token for conn controller: %w", err) } - shellPath, err := DetectShell(conn.Context, client) + shellPath, err := DetectShell(ctx, client) if err != nil { return err } @@ -221,7 +227,7 @@ func (conn *WslConn) StartConnServer() error { cmdStr = fmt.Sprintf("%s=\"%s\" %s connserver --router", wshutil.WaveJwtTokenVarName, jwtToken, wshPath) } log.Printf("starting conn controller: %s\n", cmdStr) - cmd := client.WslCommand(conn.Context, cmdStr) + cmd := client.WslCommand(ctx, cmdStr) pipeRead, pipeWrite := io.Pipe() inputPipeRead, inputPipeWrite := io.Pipe() cmd.SetStdout(pipeWrite) @@ -473,8 +479,7 @@ func getConnInternal(name string) *WslConn { connName := WslName{Distro: name} rtn := clientControllerMap[name] if rtn == nil { - ctx, cancelFn := context.WithCancel(context.Background()) - rtn = &WslConn{Lock: &sync.Mutex{}, Status: Status_Init, Name: connName, HasWaiter: &atomic.Bool{}, Context: ctx, cancelFn: cancelFn} + rtn = &WslConn{Lock: &sync.Mutex{}, Status: Status_Init, Name: connName, HasWaiter: &atomic.Bool{}, cancelFn: nil} clientControllerMap[name] = rtn } return rtn From 246640afe83887d04ce11cd3fa2cfcbd47eb9357 Mon Sep 17 00:00:00 2001 From: Sylvia Crowe Date: Thu, 2 Jan 2025 10:17:00 -0800 Subject: [PATCH 2/5] fix: use regular context for shellproc --- pkg/shellexec/shellexec.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go index 6ee730cde6..6279f93658 100644 --- a/pkg/shellexec/shellexec.go +++ b/pkg/shellexec/shellexec.go @@ -148,12 +148,10 @@ func (pp *PipePty) WriteString(s string) (n int, err error) { } func StartWslShellProc(ctx context.Context, termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOptsType, conn *wsl.WslConn) (*ShellProc, error) { - shellProcCtx, cancelFn := context.WithCancel(ctx) - defer cancelFn() client := conn.GetClient() shellPath := cmdOpts.ShellPath if shellPath == "" { - remoteShellPath, err := wsl.DetectShell(shellProcCtx, client) + remoteShellPath, err := wsl.DetectShell(ctx, client) if err != nil { return nil, err } @@ -162,13 +160,13 @@ func StartWslShellProc(ctx context.Context, termSize waveobj.TermSize, cmdStr st var shellOpts []string log.Printf("detected shell: %s", shellPath) - err := wsl.InstallClientRcFiles(shellProcCtx, client) + err := wsl.InstallClientRcFiles(ctx, client) if err != nil { log.Printf("error installing rc files: %v", err) return nil, err } - homeDir := wsl.GetHomeDir(shellProcCtx, client) + homeDir := wsl.GetHomeDir(ctx, client) shellOpts = append(shellOpts, "~", "-d", client.Name()) var subShellOpts []string From c8d1157e187b55b85aa88351f4fe6b14b41024a8 Mon Sep 17 00:00:00 2001 From: Sylvia Crowe Date: Thu, 2 Jan 2025 12:28:56 -0800 Subject: [PATCH 3/5] fix: use separate contexts for connserver launch --- pkg/wsl/wsl.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/wsl/wsl.go b/pkg/wsl/wsl.go index 7f34586bfb..42a492cfd1 100644 --- a/pkg/wsl/wsl.go +++ b/pkg/wsl/wsl.go @@ -187,13 +187,8 @@ func (conn *WslConn) OpenDomainSocketListener() error { } func (conn *WslConn) StartConnServer() error { - ctx, cancelFn := context.WithCancel(context.Background()) - conn.WithLock(func() { - if conn.cancelFn != nil { - conn.cancelFn() - } - conn.cancelFn = cancelFn - }) + utilCtx, cancelFn := context.WithTimeout(context.Background(), 2*time.Second) + defer cancelFn() var allowed bool conn.WithLock(func() { if conn.Status != Status_Connecting { @@ -206,7 +201,7 @@ func (conn *WslConn) StartConnServer() error { return fmt.Errorf("cannot start conn server for %q when status is %q", conn.GetName(), conn.GetStatus()) } client := conn.GetClient() - wshPath := GetWshPath(ctx, client) + wshPath := GetWshPath(utilCtx, client) rpcCtx := wshrpc.RpcContext{ ClientType: wshrpc.ClientType_ConnServer, Conn: conn.GetName(), @@ -216,7 +211,7 @@ func (conn *WslConn) StartConnServer() error { if err != nil { return fmt.Errorf("unable to create jwt token for conn controller: %w", err) } - shellPath, err := DetectShell(ctx, client) + shellPath, err := DetectShell(utilCtx, client) if err != nil { return err } @@ -227,7 +222,14 @@ func (conn *WslConn) StartConnServer() error { cmdStr = fmt.Sprintf("%s=\"%s\" %s connserver --router", wshutil.WaveJwtTokenVarName, jwtToken, wshPath) } log.Printf("starting conn controller: %s\n", cmdStr) - cmd := client.WslCommand(ctx, cmdStr) + connServerCtx, cancelFn := context.WithCancel(context.Background()) + conn.WithLock(func() { + if conn.cancelFn != nil { + conn.cancelFn() + } + conn.cancelFn = cancelFn + }) + cmd := client.WslCommand(connServerCtx, cmdStr) pipeRead, pipeWrite := io.Pipe() inputPipeRead, inputPipeWrite := io.Pipe() cmd.SetStdout(pipeWrite) From 01bcd396bc4ed5d3cedcdacc0296a5b2567845bb Mon Sep 17 00:00:00 2001 From: Sylvia Crowe Date: Thu, 2 Jan 2025 12:34:24 -0800 Subject: [PATCH 4/5] fix: use timeouts for wsl shellexec --- pkg/shellexec/shellexec.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/shellexec/shellexec.go b/pkg/shellexec/shellexec.go index 6279f93658..6f2145827c 100644 --- a/pkg/shellexec/shellexec.go +++ b/pkg/shellexec/shellexec.go @@ -148,10 +148,12 @@ func (pp *PipePty) WriteString(s string) (n int, err error) { } func StartWslShellProc(ctx context.Context, termSize waveobj.TermSize, cmdStr string, cmdOpts CommandOptsType, conn *wsl.WslConn) (*ShellProc, error) { + utilCtx, cancelFn := context.WithTimeout(ctx, 2*time.Second) + defer cancelFn() client := conn.GetClient() shellPath := cmdOpts.ShellPath if shellPath == "" { - remoteShellPath, err := wsl.DetectShell(ctx, client) + remoteShellPath, err := wsl.DetectShell(utilCtx, client) if err != nil { return nil, err } @@ -160,13 +162,13 @@ func StartWslShellProc(ctx context.Context, termSize waveobj.TermSize, cmdStr st var shellOpts []string log.Printf("detected shell: %s", shellPath) - err := wsl.InstallClientRcFiles(ctx, client) + err := wsl.InstallClientRcFiles(utilCtx, client) if err != nil { log.Printf("error installing rc files: %v", err) return nil, err } - homeDir := wsl.GetHomeDir(ctx, client) + homeDir := wsl.GetHomeDir(utilCtx, client) shellOpts = append(shellOpts, "~", "-d", client.Name()) var subShellOpts []string From cc866709dfd6cddfdfd8852e674f94d34a607401 Mon Sep 17 00:00:00 2001 From: Sylvia Crowe Date: Thu, 2 Jan 2025 12:42:26 -0800 Subject: [PATCH 5/5] fix: use coderabbit suggestion of 5 sec timeout --- pkg/wsl/wsl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/wsl/wsl.go b/pkg/wsl/wsl.go index 42a492cfd1..4cad32bde9 100644 --- a/pkg/wsl/wsl.go +++ b/pkg/wsl/wsl.go @@ -187,7 +187,7 @@ func (conn *WslConn) OpenDomainSocketListener() error { } func (conn *WslConn) StartConnServer() error { - utilCtx, cancelFn := context.WithTimeout(context.Background(), 2*time.Second) + utilCtx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second) defer cancelFn() var allowed bool conn.WithLock(func() {