From 694c8c802a0f9ba55831ce36030460b03803de70 Mon Sep 17 00:00:00 2001 From: Renz Date: Tue, 18 Nov 2025 18:24:49 +0000 Subject: [PATCH] Add comprehensive process cleanup on TUI exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Created internal/cleanup package for orphaned process management - KillOrphanedProcesses(): Finds and kills pg_dump, pg_restore, gzip, pigz - killProcessGroup(): Kills entire process groups (handles pipelines) - Pass parent context through all TUI operations (backup/restore inherit cancellation) - Menu cancel now kills all child processes before exit - Fixed context chain: menu.ctx → backup/restore operations - No more zombie processes when user quits TUI mid-operation Context chain: - signal.NotifyContext in main.go → menu.ctx - menu.ctx → backup_exec.ctx, restore_exec.ctx - Child contexts inherit cancellation via context.WithTimeout(parentCtx) - All exec.CommandContext use proper parent context Prevents: Orphaned pg_dump/pg_restore eating CPU/disk after TUI quit --- internal/cleanup/processes.go | 138 ++++++++++++++++++++++++++++++++ internal/tui/archive_browser.go | 7 +- internal/tui/backup_exec.go | 11 ++- internal/tui/backup_manager.go | 7 +- internal/tui/confirmation.go | 4 +- internal/tui/dbselector.go | 6 +- internal/tui/input.go | 2 +- internal/tui/menu.go | 21 +++-- internal/tui/restore_exec.go | 11 ++- internal/tui/restore_preview.go | 6 +- 10 files changed, 189 insertions(+), 24 deletions(-) create mode 100644 internal/cleanup/processes.go diff --git a/internal/cleanup/processes.go b/internal/cleanup/processes.go new file mode 100644 index 0000000..14366a4 --- /dev/null +++ b/internal/cleanup/processes.go @@ -0,0 +1,138 @@ +package cleanup + +import ( + "fmt" + "os" + "os/exec" + "strconv" + "strings" + "syscall" + + "dbbackup/internal/logger" +) + +// KillOrphanedProcesses finds and kills any orphaned pg_dump, pg_restore, gzip, or pigz processes +func KillOrphanedProcesses(log logger.Logger) error { + processNames := []string{"pg_dump", "pg_restore", "gzip", "pigz", "gunzip"} + + myPID := os.Getpid() + var killed []string + var errors []error + + for _, procName := range processNames { + pids, err := findProcessesByName(procName, myPID) + if err != nil { + log.Warn("Failed to search for processes", "process", procName, "error", err) + continue + } + + for _, pid := range pids { + if err := killProcessGroup(pid); err != nil { + errors = append(errors, fmt.Errorf("failed to kill %s (PID %d): %w", procName, pid, err)) + } else { + killed = append(killed, fmt.Sprintf("%s (PID %d)", procName, pid)) + } + } + } + + if len(killed) > 0 { + log.Info("Cleaned up orphaned processes", "count", len(killed), "processes", strings.Join(killed, ", ")) + } + + if len(errors) > 0 { + return fmt.Errorf("some processes could not be killed: %v", errors) + } + + return nil +} + +// findProcessesByName returns PIDs of processes matching the given name +func findProcessesByName(name string, excludePID int) ([]int, error) { + // Use pgrep for efficient process searching + cmd := exec.Command("pgrep", "-x", name) + output, err := cmd.Output() + if err != nil { + // Exit code 1 means no processes found (not an error) + if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 1 { + return []int{}, nil + } + return nil, err + } + + var pids []int + lines := strings.Split(strings.TrimSpace(string(output)), "\n") + for _, line := range lines { + if line == "" { + continue + } + + pid, err := strconv.Atoi(line) + if err != nil { + continue + } + + // Don't kill our own process + if pid == excludePID { + continue + } + + pids = append(pids, pid) + } + + return pids, nil +} + +// killProcessGroup kills a process and its entire process group +func killProcessGroup(pid int) error { + // First try to get the process group ID + pgid, err := syscall.Getpgid(pid) + if err != nil { + // Process might already be gone + return nil + } + + // Kill the entire process group (negative PID kills the group) + // This catches pipelines like "pg_dump | gzip" + if err := syscall.Kill(-pgid, syscall.SIGTERM); err != nil { + // If SIGTERM fails, try SIGKILL + syscall.Kill(-pgid, syscall.SIGKILL) + } + + // Also kill the specific PID in case it's not in a group + syscall.Kill(pid, syscall.SIGTERM) + + return nil +} + +// SetProcessGroup sets the current process to be a process group leader +// This should be called when starting external commands to ensure clean termination +func SetProcessGroup(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + Pgid: 0, // Create new process group + } +} + +// KillCommandGroup kills a command and its entire process group +func KillCommandGroup(cmd *exec.Cmd) error { + if cmd.Process == nil { + return nil + } + + pid := cmd.Process.Pid + + // Get the process group ID + pgid, err := syscall.Getpgid(pid) + if err != nil { + // Process might already be gone + return nil + } + + // Kill the entire process group + if err := syscall.Kill(-pgid, syscall.SIGTERM); err != nil { + // If SIGTERM fails, use SIGKILL + syscall.Kill(-pgid, syscall.SIGKILL) + } + + return nil +} diff --git a/internal/tui/archive_browser.go b/internal/tui/archive_browser.go index 931cdde..d011a2c 100644 --- a/internal/tui/archive_browser.go +++ b/internal/tui/archive_browser.go @@ -1,6 +1,7 @@ package tui import ( + "context" "fmt" "os" "path/filepath" @@ -55,6 +56,7 @@ type ArchiveBrowserModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context archives []ArchiveInfo cursor int loading bool @@ -65,11 +67,12 @@ type ArchiveBrowserModel struct { } // NewArchiveBrowser creates a new archive browser -func NewArchiveBrowser(cfg *config.Config, log logger.Logger, parent tea.Model, mode string) ArchiveBrowserModel { +func NewArchiveBrowser(cfg *config.Config, log logger.Logger, parent tea.Model, ctx context.Context, mode string) ArchiveBrowserModel { return ArchiveBrowserModel{ config: cfg, logger: log, parent: parent, + ctx: ctx, loading: true, mode: mode, filterType: "all", @@ -206,7 +209,7 @@ func (m ArchiveBrowserModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // Open restore preview - preview := NewRestorePreview(m.config, m.logger, m.parent, selected, m.mode) + preview := NewRestorePreview(m.config, m.logger, m.parent, m.ctx, selected, m.mode) return preview, preview.Init() } diff --git a/internal/tui/backup_exec.go b/internal/tui/backup_exec.go index 568f085..4710860 100644 --- a/internal/tui/backup_exec.go +++ b/internal/tui/backup_exec.go @@ -19,6 +19,7 @@ type BackupExecutionModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context backupType string databaseName string ratio int @@ -32,11 +33,12 @@ type BackupExecutionModel struct { spinnerFrame int } -func NewBackupExecution(cfg *config.Config, log logger.Logger, parent tea.Model, backupType, dbName string, ratio int) BackupExecutionModel { +func NewBackupExecution(cfg *config.Config, log logger.Logger, parent tea.Model, ctx context.Context, backupType, dbName string, ratio int) BackupExecutionModel { return BackupExecutionModel{ config: cfg, logger: log, parent: parent, + ctx: ctx, backupType: backupType, databaseName: dbName, ratio: ratio, @@ -50,7 +52,7 @@ func NewBackupExecution(cfg *config.Config, log logger.Logger, parent tea.Model, func (m BackupExecutionModel) Init() tea.Cmd { // TUI handles all display through View() - no progress callbacks needed return tea.Batch( - executeBackupWithTUIProgress(m.config, m.logger, m.backupType, m.databaseName, m.ratio), + executeBackupWithTUIProgress(m.ctx, m.config, m.logger, m.backupType, m.databaseName, m.ratio), backupTickCmd(), ) } @@ -74,11 +76,12 @@ type backupCompleteMsg struct { err error } -func executeBackupWithTUIProgress(cfg *config.Config, log logger.Logger, backupType, dbName string, ratio int) tea.Cmd { +func executeBackupWithTUIProgress(parentCtx context.Context, cfg *config.Config, log logger.Logger, backupType, dbName string, ratio int) tea.Cmd { return func() tea.Msg { // Use configurable cluster timeout (minutes) from config; default set in config.New() + // Use parent context to inherit cancellation from TUI clusterTimeout := time.Duration(cfg.ClusterTimeoutMinutes) * time.Minute - ctx, cancel := context.WithTimeout(context.Background(), clusterTimeout) + ctx, cancel := context.WithTimeout(parentCtx, clusterTimeout) defer cancel() start := time.Now() diff --git a/internal/tui/backup_manager.go b/internal/tui/backup_manager.go index ef5c5f8..a6634d3 100644 --- a/internal/tui/backup_manager.go +++ b/internal/tui/backup_manager.go @@ -1,6 +1,7 @@ package tui import ( + "context" "fmt" "os" "strings" @@ -17,6 +18,7 @@ type BackupManagerModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context archives []ArchiveInfo cursor int loading bool @@ -27,11 +29,12 @@ type BackupManagerModel struct { } // NewBackupManager creates a new backup manager -func NewBackupManager(cfg *config.Config, log logger.Logger, parent tea.Model) BackupManagerModel { +func NewBackupManager(cfg *config.Config, log logger.Logger, parent tea.Model, ctx context.Context) BackupManagerModel { return BackupManagerModel{ config: cfg, logger: log, parent: parent, + ctx: ctx, loading: true, } } @@ -126,7 +129,7 @@ func (m BackupManagerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if selected.Format.IsClusterBackup() { mode = "restore-cluster" } - preview := NewRestorePreview(m.config, m.logger, m.parent, selected, mode) + preview := NewRestorePreview(m.config, m.logger, m.parent, m.ctx, selected, mode) return preview, preview.Init() } diff --git a/internal/tui/confirmation.go b/internal/tui/confirmation.go index 631a41f..6179108 100644 --- a/internal/tui/confirmation.go +++ b/internal/tui/confirmation.go @@ -1,6 +1,7 @@ package tui import ( + "context" "fmt" "strings" @@ -15,6 +16,7 @@ type ConfirmationModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context title string message string cursor int @@ -75,7 +77,7 @@ func (m ConfirmationModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m.onConfirm() } // Default: execute cluster backup for backward compatibility - executor := NewBackupExecution(m.config, m.logger, m.parent, "cluster", "", 0) + executor := NewBackupExecution(m.config, m.logger, m.parent, m.ctx, "cluster", "", 0) return executor, executor.Init() } return m.parent, nil diff --git a/internal/tui/dbselector.go b/internal/tui/dbselector.go index ea9b5e1..0cada9f 100644 --- a/internal/tui/dbselector.go +++ b/internal/tui/dbselector.go @@ -18,6 +18,7 @@ type DatabaseSelectorModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context databases []string cursor int selected string @@ -28,11 +29,12 @@ type DatabaseSelectorModel struct { backupType string // "single" or "sample" } -func NewDatabaseSelector(cfg *config.Config, log logger.Logger, parent tea.Model, title string, backupType string) DatabaseSelectorModel { +func NewDatabaseSelector(cfg *config.Config, log logger.Logger, parent tea.Model, ctx context.Context, title string, backupType string) DatabaseSelectorModel { return DatabaseSelectorModel{ config: cfg, logger: log, parent: parent, + ctx: ctx, databases: []string{"Loading databases..."}, title: title, loading: true, @@ -115,7 +117,7 @@ func (m DatabaseSelectorModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // For single backup, go directly to execution - executor := NewBackupExecution(m.config, m.logger, m.parent, m.backupType, m.selected, 0) + executor := NewBackupExecution(m.config, m.logger, m.parent, m.ctx, m.backupType, m.selected, 0) return executor, executor.Init() } } diff --git a/internal/tui/input.go b/internal/tui/input.go index ffbea9c..fbc53a4 100644 --- a/internal/tui/input.go +++ b/internal/tui/input.go @@ -65,7 +65,7 @@ func (m InputModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // If this is from database selector, execute backup with ratio if selector, ok := m.parent.(DatabaseSelectorModel); ok { ratio, _ := strconv.Atoi(m.value) - executor := NewBackupExecution(selector.config, selector.logger, selector.parent, + executor := NewBackupExecution(selector.config, selector.logger, selector.parent, selector.ctx, selector.backupType, selector.selected, ratio) return executor, executor.Init() } diff --git a/internal/tui/menu.go b/internal/tui/menu.go index 4d4bbae..00a5fca 100644 --- a/internal/tui/menu.go +++ b/internal/tui/menu.go @@ -8,6 +8,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "dbbackup/internal/cleanup" "dbbackup/internal/config" "dbbackup/internal/logger" ) @@ -119,9 +120,17 @@ func (m MenuModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case tea.KeyMsg: switch msg.String() { case "ctrl+c", "q": + // Cancel all running operations if m.cancel != nil { m.cancel() } + + // Clean up any orphaned processes before exit + m.logger.Info("Cleaning up processes before exit") + if err := cleanup.KillOrphanedProcesses(m.logger); err != nil { + m.logger.Warn("Failed to clean up all processes", "error", err) + } + m.quitting = true return m, tea.Quit @@ -252,13 +261,13 @@ func (m MenuModel) View() string { // handleSingleBackup opens database selector for single backup func (m MenuModel) handleSingleBackup() (tea.Model, tea.Cmd) { - selector := NewDatabaseSelector(m.config, m.logger, m, "🗄️ Single Database Backup", "single") + selector := NewDatabaseSelector(m.config, m.logger, m, m.ctx, "🗄️ Single Database Backup", "single") return selector, selector.Init() } // handleSampleBackup opens database selector for sample backup func (m MenuModel) handleSampleBackup() (tea.Model, tea.Cmd) { - selector := NewDatabaseSelector(m.config, m.logger, m, "📊 Sample Database Backup", "sample") + selector := NewDatabaseSelector(m.config, m.logger, m, m.ctx, "📊 Sample Database Backup", "sample") return selector, selector.Init() } @@ -272,7 +281,7 @@ func (m MenuModel) handleClusterBackup() (tea.Model, tea.Cmd) { "🗄️ Cluster Backup", "This will backup ALL databases in the cluster. Continue?", func() (tea.Model, tea.Cmd) { - executor := NewBackupExecution(m.config, m.logger, m, "cluster", "", 0) + executor := NewBackupExecution(m.config, m.logger, m, m.ctx, "cluster", "", 0) return executor, executor.Init() }) return confirm, nil @@ -305,7 +314,7 @@ func (m MenuModel) handleSettings() (tea.Model, tea.Cmd) { // handleRestoreSingle opens archive browser for single restore func (m MenuModel) handleRestoreSingle() (tea.Model, tea.Cmd) { - browser := NewArchiveBrowser(m.config, m.logger, m, "restore-single") + browser := NewArchiveBrowser(m.config, m.logger, m, m.ctx, "restore-single") return browser, browser.Init() } @@ -315,13 +324,13 @@ func (m MenuModel) handleRestoreCluster() (tea.Model, tea.Cmd) { m.message = errorStyle.Render("❌ Cluster restore is available only for PostgreSQL") return m, nil } - browser := NewArchiveBrowser(m.config, m.logger, m, "restore-cluster") + browser := NewArchiveBrowser(m.config, m.logger, m, m.ctx, "restore-cluster") return browser, browser.Init() } // handleBackupManager opens backup management view func (m MenuModel) handleBackupManager() (tea.Model, tea.Cmd) { - manager := NewBackupManager(m.config, m.logger, m) + manager := NewBackupManager(m.config, m.logger, m, m.ctx) return manager, manager.Init() } diff --git a/internal/tui/restore_exec.go b/internal/tui/restore_exec.go index b38c337..103cc81 100644 --- a/internal/tui/restore_exec.go +++ b/internal/tui/restore_exec.go @@ -23,6 +23,7 @@ type RestoreExecutionModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context archive ArchiveInfo targetDB string cleanFirst bool @@ -48,11 +49,12 @@ type RestoreExecutionModel struct { } // NewRestoreExecution creates a new restore execution model -func NewRestoreExecution(cfg *config.Config, log logger.Logger, parent tea.Model, archive ArchiveInfo, targetDB string, cleanFirst, createIfMissing bool, restoreType string, cleanClusterFirst bool, existingDBs []string) RestoreExecutionModel { +func NewRestoreExecution(cfg *config.Config, log logger.Logger, parent tea.Model, ctx context.Context, archive ArchiveInfo, targetDB string, cleanFirst, createIfMissing bool, restoreType string, cleanClusterFirst bool, existingDBs []string) RestoreExecutionModel { return RestoreExecutionModel{ config: cfg, logger: log, parent: parent, + ctx: ctx, archive: archive, targetDB: targetDB, cleanFirst: cleanFirst, @@ -71,7 +73,7 @@ func NewRestoreExecution(cfg *config.Config, log logger.Logger, parent tea.Model func (m RestoreExecutionModel) Init() tea.Cmd { return tea.Batch( - executeRestoreWithTUIProgress(m.config, m.logger, m.archive, m.targetDB, m.cleanFirst, m.createIfMissing, m.restoreType, m.cleanClusterFirst, m.existingDBs), + executeRestoreWithTUIProgress(m.ctx, m.config, m.logger, m.archive, m.targetDB, m.cleanFirst, m.createIfMissing, m.restoreType, m.cleanClusterFirst, m.existingDBs), restoreTickCmd(), ) } @@ -97,11 +99,12 @@ type restoreCompleteMsg struct { elapsed time.Duration } -func executeRestoreWithTUIProgress(cfg *config.Config, log logger.Logger, archive ArchiveInfo, targetDB string, cleanFirst, createIfMissing bool, restoreType string, cleanClusterFirst bool, existingDBs []string) tea.Cmd { +func executeRestoreWithTUIProgress(parentCtx context.Context, cfg *config.Config, log logger.Logger, archive ArchiveInfo, targetDB string, cleanFirst, createIfMissing bool, restoreType string, cleanClusterFirst bool, existingDBs []string) tea.Cmd { return func() tea.Msg { // Use configurable cluster timeout (minutes) from config; default set in config.New() + // Use parent context to inherit cancellation from TUI restoreTimeout := time.Duration(cfg.ClusterTimeoutMinutes) * time.Minute - ctx, cancel := context.WithTimeout(context.Background(), restoreTimeout) + ctx, cancel := context.WithTimeout(parentCtx, restoreTimeout) defer cancel() start := time.Now() diff --git a/internal/tui/restore_preview.go b/internal/tui/restore_preview.go index 1fa84cf..ba5162a 100644 --- a/internal/tui/restore_preview.go +++ b/internal/tui/restore_preview.go @@ -46,6 +46,7 @@ type RestorePreviewModel struct { config *config.Config logger logger.Logger parent tea.Model + ctx context.Context archive ArchiveInfo mode string targetDB string @@ -61,7 +62,7 @@ type RestorePreviewModel struct { } // NewRestorePreview creates a new restore preview -func NewRestorePreview(cfg *config.Config, log logger.Logger, parent tea.Model, archive ArchiveInfo, mode string) RestorePreviewModel { +func NewRestorePreview(cfg *config.Config, log logger.Logger, parent tea.Model, ctx context.Context, archive ArchiveInfo, mode string) RestorePreviewModel { // Default target database name from archive targetDB := archive.DatabaseName if targetDB == "" { @@ -72,6 +73,7 @@ func NewRestorePreview(cfg *config.Config, log logger.Logger, parent tea.Model, config: cfg, logger: log, parent: parent, + ctx: ctx, archive: archive, mode: mode, targetDB: targetDB, @@ -249,7 +251,7 @@ func (m RestorePreviewModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // Proceed to restore execution - exec := NewRestoreExecution(m.config, m.logger, m.parent, m.archive, m.targetDB, m.cleanFirst, m.createIfMissing, m.mode, m.cleanClusterFirst, m.existingDBs) + exec := NewRestoreExecution(m.config, m.logger, m.parent, m.ctx, m.archive, m.targetDB, m.cleanFirst, m.createIfMissing, m.mode, m.cleanClusterFirst, m.existingDBs) return exec, exec.Init() } }