fix(cli): prevent sqlDB leaks in ConnectToPostgres

mafredri · mafredri · commit c1dd875e1dd7 · 2023-10-05T09:18:15.000Z
diff --git a/cli/server.go b/cli/server.go
@@ -1875,19 +1875,26 @@ func BuildLogger(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (slog.
 	}, nil
 }
 
-func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string) (*sql.DB, error) {
+func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, dbURL string) (sqlDB *sql.DB, err error) {
 	logger.Debug(ctx, "connecting to postgresql")
 
 	// Try to connect for 30 seconds.
 	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 	defer cancel()
 
-	var (
-		sqlDB *sql.DB
-		err   error
-		ok    = false
-		tries int
-	)
+	var tries int
+
+	defer func() {
+		if err == nil {
+			return
+		}
+		if sqlDB != nil {
+			_ = sqlDB.Close()
+			sqlDB = nil
+		}
+		logger.Error(ctx, "connect to postgres; tries %d", slog.Error(err))
+	}()
+
 	for r := retry.New(time.Second, 3*time.Second); r.Wait(ctx); {
 		tries++
 
@@ -1900,18 +1907,16 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
 		err = pingPostgres(ctx, sqlDB)
 		if err != nil {
 			logger.Warn(ctx, "ping postgres; retrying", slog.Error(err), slog.F("try", tries))
+			_ = sqlDB.Close()
+			sqlDB = nil
 			continue
 		}
 
 		break
 	}
-	// Make sure we close the DB in case it opened but the ping failed for some
-	// reason.
-	defer func() {
-		if !ok && sqlDB != nil {
-			_ = sqlDB.Close()
-		}
-	}()
+	if err == nil {
+		err = ctx.Err()
+	}
 	if err != nil {
 		return nil, xerrors.Errorf("connect to postgres; tries %d; last error: %w", tries, err)
 	}
@@ -1958,7 +1963,6 @@ func ConnectToPostgres(ctx context.Context, logger slog.Logger, driver string, d
 	// of connection churn.
 	sqlDB.SetMaxIdleConns(3)
 
-	ok = true
 	return sqlDB, nil
 }
 
