[v13] Fix leaking connection monitor instances. Expand comment with a…

… warning. (#31042) * Fix leaking connection monitor instances. Expand comment with a warning. * Fix typo.
gravitational · Aug 25, 2023 · 742f1b2 · 742f1b2
1 parent 3446f63
commit 742f1b2
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 2 deletions.
diff --git a/lib/srv/db/proxyserver.go b/lib/srv/db/proxyserver.go
@@ -507,6 +507,9 @@ func isReverseTunnelDownError(err error) bool {
 //
 // Implements common.Service.
 func (s *ProxyServer) Proxy(ctx context.Context, proxyCtx *common.ProxyContext, clientConn, serviceConn net.Conn) error {
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
 	// Wrap a client connection with a monitor that auto-terminates
 	// idle connection and connection with expired cert.
 	var err error

diff --git a/lib/srv/db/server.go b/lib/srv/db/server.go
@@ -1002,7 +1002,7 @@ func (s *Server) handleConnection(ctx context.Context, clientConn net.Conn) erro
 
 	// Wrap a client connection into monitor that auto-terminates
 	// idle connection and connection with expired cert.
-	ctx, clientConn, err = s.cfg.ConnectionMonitor.MonitorConn(ctx, sessionCtx.AuthContext, clientConn)
+	ctx, clientConn, err = s.cfg.ConnectionMonitor.MonitorConn(cancelCtx, sessionCtx.AuthContext, clientConn)
 	if err != nil {
 		return trace.Wrap(err)
 	}

diff --git a/lib/srv/monitor.go b/lib/srv/monitor.go
@@ -213,7 +213,8 @@ type MonitorConfig struct {
 	Tracker ActivityTracker
 	// Conn is a connection to close
 	Conn TrackingConn
-	// Context is an external context to cancel the operation
+	// Context is an external context. To reliably close the monitor and ensure no goroutine leak,
+	// make sure to pass a context which will be canceled on time.
 	Context context.Context
 	// Login is linux box login
 	Login string