Skip to content

Commit b30bb42

Browse files
committed
fix(lib/screentracker,cmd/server): add f.Sync, remove duplicate SIGINT, verify PID ownership
Add f.Sync() before close in SaveState() so data is flushed to disk before the atomic rename, preventing corrupt state files on power failure. Remove duplicate syscall.SIGINT from signal.Notify in signals_unix.go since os.Interrupt is identical to syscall.SIGINT on Unix. Make cleanupPIDFile read the PID file and verify it contains the current process PID before deleting, preventing a new instance's PID file from being removed by an old instance's deferred cleanup.
1 parent 6dd7bbc commit b30bb42

4 files changed

Lines changed: 23 additions & 4 deletions

File tree

cmd/server/server.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,21 @@ func writePIDFile(pidFile string, logger *slog.Logger) error {
291291
return nil
292292
}
293293

294-
// cleanupPIDFile removes the PID file if it exists
294+
// cleanupPIDFile removes the PID file if it was written by this process.
295295
func cleanupPIDFile(pidFile string, logger *slog.Logger) {
296+
data, err := os.ReadFile(pidFile)
297+
if err != nil {
298+
if !os.IsNotExist(err) {
299+
logger.Error("Failed to read PID file for cleanup", "pidFile", pidFile, "error", err)
300+
}
301+
return
302+
}
303+
pidStr := strings.TrimSpace(string(data))
304+
filePID, err := strconv.Atoi(pidStr)
305+
if err != nil || filePID != os.Getpid() {
306+
logger.Info("PID file belongs to another process, skipping cleanup", "pidFile", pidFile, "filePID", pidStr)
307+
return
308+
}
296309
if err := os.Remove(pidFile); err != nil && !os.IsNotExist(err) {
297310
logger.Error("Failed to remove PID file", "pidFile", pidFile, "error", err)
298311
} else if err == nil {

cmd/server/server_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -675,8 +675,8 @@ func TestPIDFileOperations(t *testing.T) {
675675
tmpDir := t.TempDir()
676676
pidFile := tmpDir + "/test.pid"
677677

678-
// Create PID file
679-
err := os.WriteFile(pidFile, []byte("12345\n"), 0o644)
678+
// Create PID file with current process PID so ownership check passes
679+
err := os.WriteFile(pidFile, []byte(fmt.Sprintf("%d\n", os.Getpid())), 0o644)
680680
require.NoError(t, err)
681681

682682
// Cleanup

cmd/server/signals_unix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
func handleSignals(ctx context.Context, cancel context.CancelFunc, logger *slog.Logger, srv *httpapi.Server) {
1919
// Handle shutdown signals (SIGTERM, SIGINT, SIGHUP)
2020
shutdownCh := make(chan os.Signal, 1)
21-
signal.Notify(shutdownCh, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGINT)
21+
signal.Notify(shutdownCh, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP)
2222
go func() {
2323
defer signal.Stop(shutdownCh)
2424
sig := <-shutdownCh

lib/screentracker/pty_conversation.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,12 @@ func (c *PTYConversation) SaveState() error {
610610
return xerrors.Errorf("failed to encode state: %w", err)
611611
}
612612

613+
// Flush to disk before rename for crash safety
614+
if err := f.Sync(); err != nil {
615+
_ = f.Close()
616+
return xerrors.Errorf("failed to sync state file: %w", err)
617+
}
618+
613619
// Close file before rename
614620
if err := f.Close(); err != nil {
615621
return xerrors.Errorf("failed to close temp state file: %w", err)

0 commit comments

Comments
 (0)