From bc089f34104803ca8b2c2f72dee9789d54a71391 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Wed, 11 Jun 2025 13:27:38 +0500 Subject: [PATCH 1/3] chore: add windows icon (cherry-pick #18312) (#18322) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ケイラ --- site/src/theme/icons.json | 1 + site/static/icon/windows.svg | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 site/static/icon/windows.svg diff --git a/site/src/theme/icons.json b/site/src/theme/icons.json index 8e92dd9a48198..b60dce9fbcab3 100644 --- a/site/src/theme/icons.json +++ b/site/src/theme/icons.json @@ -105,6 +105,7 @@ "vsphere.svg", "webstorm.svg", "widgets.svg", + "windows.svg", "windsurf.svg", "zed.svg" ] diff --git a/site/static/icon/windows.svg b/site/static/icon/windows.svg new file mode 100644 index 0000000000000..8b774a501cdc1 --- /dev/null +++ b/site/static/icon/windows.svg @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 8e8dd58506c1f10a51d5c9d82055a1783761f787 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Mon, 16 Jun 2025 13:36:44 +1000 Subject: [PATCH 2/3] fix(site): remove trailing comment from cursor.svg (cherry-pick #18072) (#18378) Cherry-picked fix(site): remove trailing comment from cursor.svg (#18072) The trailing comment was preventing the SVG from rendering on Coder Desktop macOS, with the SVG loader we use. I've moved it to a place where it's apparently OK? Couldn't tell you why. https://validator.w3.org/ had no complaints. I tested this by hardcoding the icon to that served by a build of coder with this new svg. ![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/jI7h94jB23BidWsYTSCk/4c94ae5f-d0e2-496e-90eb-4968cf40d639.png) The first icon is without the trailing comment, the second is with. Co-authored-by: Ethan <39577870+ethanndickson@users.noreply.github.com> --- site/static/icon/cursor.svg | 41 +++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/site/static/icon/cursor.svg b/site/static/icon/cursor.svg index 5c37cb9c053b0..f224c88d6c985 100644 --- a/site/static/icon/cursor.svg +++ b/site/static/icon/cursor.svg @@ -1,4 +1,25 @@ + Cursor @@ -24,24 +45,4 @@ - From 75e7a9359827c2ef2e55237fe6860c47fc106761 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Mon, 30 Jun 2025 23:09:47 +0400 Subject: [PATCH 3/3] fix: stop tearing down non-TTY processes on SSH session end (cherry-pick #18673) (#18677) Cherry-picked fix: stop tearing down non-TTY processes on SSH session end (#18673) (possibly temporary) fix for #18519 Matches OpenSSH for non-tty sessions, where we don't actively terminate the process. Adds explicit tracking to the SSH server for these processes so that if we are shutting down we terminate them: this ensures that we can shut down quickly to allow shutdown scripts to run. It also ensures our tests don't leak system resources. Co-authored-by: Spike Curtis --- agent/agentssh/agentssh.go | 42 +++++++++++++++++++++++++++++++++- agent/agentssh/exec_other.go | 10 ++++---- agent/agentssh/exec_windows.go | 20 ++++++++-------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index 293dd4db169ac..b802ede9c93e7 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -124,6 +124,7 @@ type Server struct { listeners map[net.Listener]struct{} conns map[net.Conn]struct{} sessions map[ssh.Session]struct{} + processes map[*os.Process]struct{} closing chan struct{} // Wait for goroutines to exit, waited without // a lock on mu but protected by closing. @@ -182,6 +183,7 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom fs: fs, conns: make(map[net.Conn]struct{}), sessions: make(map[ssh.Session]struct{}), + processes: make(map[*os.Process]struct{}), logger: logger, config: config, @@ -586,7 +588,10 @@ func (s *Server) startNonPTYSession(logger slog.Logger, session ssh.Session, mag // otherwise context cancellation will not propagate properly // and SSH server close may be delayed. cmd.SysProcAttr = cmdSysProcAttr() - cmd.Cancel = cmdCancel(session.Context(), logger, cmd) + + // to match OpenSSH, we don't actually tear a non-TTY command down, even if the session ends. + // c.f. https://github.com/coder/coder/issues/18519#issuecomment-3019118271 + cmd.Cancel = nil cmd.Stdout = session cmd.Stderr = session.Stderr() @@ -609,6 +614,16 @@ func (s *Server) startNonPTYSession(logger slog.Logger, session ssh.Session, mag s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, "no", "start_command").Add(1) return xerrors.Errorf("start: %w", err) } + + // Since we don't cancel the process when the session stops, we still need to tear it down if we are closing. So + // track it here. + if !s.trackProcess(cmd.Process, true) { + // must be closing + err = cmdCancel(logger, cmd.Process) + return xerrors.Errorf("failed to track process: %w", err) + } + defer s.trackProcess(cmd.Process, false) + sigs := make(chan ssh.Signal, 1) session.Signals(sigs) defer func() { @@ -1052,6 +1067,27 @@ func (s *Server) trackSession(ss ssh.Session, add bool) (ok bool) { return true } +// trackCommand registers the process with the server. If the server is +// closing, the process is not registered and should be closed. +// +//nolint:revive +func (s *Server) trackProcess(p *os.Process, add bool) (ok bool) { + s.mu.Lock() + defer s.mu.Unlock() + if add { + if s.closing != nil { + // Server closed. + return false + } + s.wg.Add(1) + s.processes[p] = struct{}{} + return true + } + s.wg.Done() + delete(s.processes, p) + return true +} + // Close the server and all active connections. Server can be re-used // after Close is done. func (s *Server) Close() error { @@ -1091,6 +1127,10 @@ func (s *Server) Close() error { _ = c.Close() } + for p := range s.processes { + _ = cmdCancel(s.logger, p) + } + s.logger.Debug(ctx, "closing SSH server") err := s.srv.Close() diff --git a/agent/agentssh/exec_other.go b/agent/agentssh/exec_other.go index 54dfd50899412..aef496a1ef775 100644 --- a/agent/agentssh/exec_other.go +++ b/agent/agentssh/exec_other.go @@ -4,7 +4,7 @@ package agentssh import ( "context" - "os/exec" + "os" "syscall" "cdr.dev/slog" @@ -16,9 +16,7 @@ func cmdSysProcAttr() *syscall.SysProcAttr { } } -func cmdCancel(ctx context.Context, logger slog.Logger, cmd *exec.Cmd) func() error { - return func() error { - logger.Debug(ctx, "cmdCancel: sending SIGHUP to process and children", slog.F("pid", cmd.Process.Pid)) - return syscall.Kill(-cmd.Process.Pid, syscall.SIGHUP) - } +func cmdCancel(logger slog.Logger, p *os.Process) error { + logger.Debug(context.Background(), "cmdCancel: sending SIGHUP to process and children", slog.F("pid", p.Pid)) + return syscall.Kill(-p.Pid, syscall.SIGHUP) } diff --git a/agent/agentssh/exec_windows.go b/agent/agentssh/exec_windows.go index 39f0f97198479..0dafa67958a67 100644 --- a/agent/agentssh/exec_windows.go +++ b/agent/agentssh/exec_windows.go @@ -2,7 +2,7 @@ package agentssh import ( "context" - "os/exec" + "os" "syscall" "cdr.dev/slog" @@ -12,14 +12,12 @@ func cmdSysProcAttr() *syscall.SysProcAttr { return &syscall.SysProcAttr{} } -func cmdCancel(ctx context.Context, logger slog.Logger, cmd *exec.Cmd) func() error { - return func() error { - logger.Debug(ctx, "cmdCancel: killing process", slog.F("pid", cmd.Process.Pid)) - // Windows doesn't support sending signals to process groups, so we - // have to kill the process directly. In the future, we may want to - // implement a more sophisticated solution for process groups on - // Windows, but for now, this is a simple way to ensure that the - // process is terminated when the context is cancelled. - return cmd.Process.Kill() - } +func cmdCancel(logger slog.Logger, p *os.Process) error { + logger.Debug(context.Background(), "cmdCancel: killing process", slog.F("pid", p.Pid)) + // Windows doesn't support sending signals to process groups, so we + // have to kill the process directly. In the future, we may want to + // implement a more sophisticated solution for process groups on + // Windows, but for now, this is a simple way to ensure that the + // process is terminated when the context is cancelled. + return p.Kill() }