diff --git a/README.md b/README.md index 68e1d6e..6357f79 100644 --- a/README.md +++ b/README.md @@ -136,9 +136,12 @@ Installed Tentacles run as the current user and require no administrator setup o particular init system. Each session has a private Linux subreaper helper. The helper watches a reader-owned lifetime pipe and terminates and reaps its child and detached descendants when the session ends, including abrupt reader exit. +Exited orphaned descendants are also reaped while the Tentacle remains active. Installation and enabling report success only after registration, keyboard shortcut setup and activation succeed. A failed start leaves the installed package disabled, with an error in Preferences; enabling it again retries. +Enabling an already enabled Tentacle restarts it if its session has failed. +Cancelling startup during shortcut setup restores the other Tentacles' shortcuts. Only install Tentacles from publishers you trust. They inherit your account's file and network access. Protocol sizes, request deadlines and host admission diff --git a/internal/app/tentacle_management.go b/internal/app/tentacle_management.go index deabd5a..774506f 100644 --- a/internal/app/tentacle_management.go +++ b/internal/app/tentacle_management.go @@ -573,8 +573,17 @@ func (runtime *tentacleRuntime) run_managed(ctx context.Context) error { } next, err := change_tentacle_entries(entries, call.request) if err == nil && call.request.Action != "retry" && reflect.DeepEqual(next, entries) { - call.result <- nil - continue + // Saved enablement is intent, not proof of a live session. An + // explicit enable/update must recover a failed or starting entry. + index := slices.IndexFunc(next, func(entry tentacle.Entry) bool { return entry.ID == call.request.ID }) + state := tentacle.Stopped + if session := sessions[call.request.ID]; session != nil { + state, _ = session.Status() + } + if !next[index].Enabled || state == tentacle.Active { + call.result <- nil + continue + } } if err == nil && call.request.Action != "retry" { err = runtime.registry.Save(next) @@ -633,7 +642,16 @@ func (runtime *tentacleRuntime) run_managed(ctx context.Context) error { failStartup(session.TentacleID(), "Tentacle could not acquire its keyboard shortcuts; enable it again after resolving the conflict.") continue } - if err := session.Activate(); err != nil { + // Native binding setup can wait for held keys. Cancellation during + // that wait must roll back the new grabs before reporting failure. + activationError := ctx.Err() + if startup, exists := startups[session.TentacleID()]; exists && activationError == nil { + activationError = startup.call.ctx.Err() + } + if activationError == nil { + activationError = session.Activate() + } + if activationError != nil { session.Disable() if err := runtime.bind(ctx, active); err != nil && ctx.Err() == nil { select { diff --git a/internal/app/tentacle_management_test.go b/internal/app/tentacle_management_test.go index 176ca00..f223ea7 100644 --- a/internal/app/tentacle_management_test.go +++ b/internal/app/tentacle_management_test.go @@ -11,6 +11,7 @@ import ( "time" "git.stormux.org/storm/cthulhu/internal/config" + "git.stormux.org/storm/cthulhu/internal/input" "git.stormux.org/storm/cthulhu/internal/preferences" "git.stormux.org/storm/cthulhu/internal/tentacle" ) @@ -337,3 +338,69 @@ func TestManagedTentacleCancelledStartupAndConcurrentDisable(t *testing.T) { }) } } + +func TestManagedTentacleEnableRestartsFailedSession(t *testing.T) { + owner, bindings := managed_tentacle_fixture(t, filepath.Join(t.TempDir(), "tentacles.toml"), false) + if err := owner.Start(context.Background()); err != nil { + t.Fatal(err) + } + entry := probe_entry(t, "probe", "crash") + entry.Enabled = true + manage_tentacle(t, owner, "add", entry.ID, &entry) + command := bindings.TentacleBindings()[0].Command + owner.Invoke(context.Background(), command) + wait_managed_state(t, owner, entry.ID, "failed") + manage_tentacle(t, owner, "enable", entry.ID, nil) + status := owner.Snapshot().Entries[0] + if status.State != "active" || !status.Entry.Enabled || status.Message != "" { + t.Fatalf("enable reported success without restarting: %+v", status) + } + if current := bindings.TentacleBindings(); len(current) != 1 || current[0].Command == command { + t.Fatalf("enable did not publish a new session: %+v", current) + } +} + +type cancellingTentacleBindings struct { + *tentacleBindingRecorder + cancel context.CancelFunc +} + +func (bindings *cancellingTentacleBindings) SetTentacleBindings(ctx context.Context, next []input.BindingOverride) error { + err := bindings.tentacleBindingRecorder.SetTentacleBindings(ctx, next) + if len(next) > 1 { + // Cancellation arrives after native grabs succeed, before activation. + bindings.cancel() + } + return err +} + +func TestManagedTentacleCancellationDuringBindingsRollsBack(t *testing.T) { + path := filepath.Join(t.TempDir(), "tentacles.toml") + owner, bindings := managed_tentacle_fixture(t, path, false) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + owner.input = &cancellingTentacleBindings{tentacleBindingRecorder: bindings, cancel: cancel} + if err := owner.Start(context.Background()); err != nil { + t.Fatal(err) + } + first := probe_entry(t, "first", "called") + first.Enabled = true + manage_tentacle(t, owner, "add", first.ID, &first) + firstCommand := bindings.TentacleBindings()[0].Command + second := probe_entry(t, "second", "called") + manage_tentacle(t, owner, "add", second.ID, &second) + if err := owner.Manage(ctx, tentacle.ManagementRequest{Action: "enable", ID: second.ID}); err == nil { + t.Fatal("startup cancelled during binding reported success") + } + status := owner.Snapshot().Entries + if len(status) != 2 || status[0].State != "active" || status[1].Entry.Enabled { + t.Fatalf("incorrect rollback state: %+v", status) + } + if current := bindings.TentacleBindings(); len(current) != 1 || current[0].Command != firstCommand { + t.Fatalf("rollback did not preserve the first session's bindings: %+v", current) + } + disk, err := (&tentacle.Registry{Path: path}).Load() + if err != nil || len(disk) != 2 || !disk[0].Enabled || disk[1].Enabled { + t.Fatalf("incorrect persisted rollback: %+v %v", disk, err) + } +} diff --git a/internal/tentacle/supervisor.go b/internal/tentacle/supervisor.go index 59cdcec..ec2cf78 100644 --- a/internal/tentacle/supervisor.go +++ b/internal/tentacle/supervisor.go @@ -10,6 +10,7 @@ import ( "io" "os" "os/exec" + "os/signal" "path/filepath" "runtime" "strconv" @@ -79,6 +80,9 @@ func RunSupervisor(ctx context.Context) error { command.Dir = spec.Directory command.Stdin, command.Stdout, command.Stderr = os.Stdin, os.Stdout, os.Stderr command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Pdeathsig: syscall.SIGKILL} + childSignals := make(chan os.Signal, 1) + signal.Notify(childSignals, syscall.SIGCHLD) + defer signal.Stop(childSignals) if err := command.Start(); err != nil { // No process was started. Protocol EOF reports the startup failure; // successful cleanup keeps the manager available for a later retry. @@ -86,13 +90,74 @@ func RunSupervisor(ctx context.Context) error { } exited := make(chan struct{}) go func() { _ = command.Wait(); close(exited) }() - select { - case <-ctx.Done(): - _ = command.Process.Kill() - <-exited - case <-exited: + var supervisionError error +running: + for { + select { + case <-ctx.Done(): + break running + case <-exited: + break running + case <-childSignals: + // The subreaper owns orphaned descendants for the entire session, + // not just teardown. Only command.Wait may reap the main child. + supervisionError = reap_exited_supervised_children(command.Process.Pid) + if supervisionError != nil { + break running + } + } } - return reap_supervised_children(DefaultResourceLimits().CleanupTimeout) + _ = command.Process.Kill() + <-exited + return errors.Join(supervisionError, reap_supervised_children(DefaultResourceLimits().CleanupTimeout)) +} + +func supervised_children() ([]int, error) { + paths, err := filepath.Glob("/proc/self/task/*/children") + if err != nil || len(paths) == 0 { + return nil, ErrSupervision + } + var children []int + for _, path := range paths { + data, err := os.ReadFile(path) + if os.IsNotExist(err) || errors.Is(err, syscall.ESRCH) { // A runtime thread exited during the scan. + continue + } + if err != nil { + return nil, ErrSupervision + } + for _, value := range strings.Fields(string(data)) { + pid, err := strconv.Atoi(value) + if err != nil || pid <= 1 { + return nil, ErrSupervision + } + children = append(children, pid) + } + } + return children, nil +} + +func reap_exited_supervised_children(mainPID int) error { + children, err := supervised_children() + if err != nil { + return err + } + for _, childPID := range children { + if childPID == mainPID { + continue + } + for { + _, err := syscall.Wait4(childPID, nil, syscall.WNOHANG, nil) + if err == syscall.EINTR { + continue + } + if err != nil && err != syscall.ECHILD { + return fmt.Errorf("%w: reap exited descendant: %w", ErrSupervision, err) + } + break + } + } + return nil } // Only this helper's children are killed. A child PID cannot be reused until @@ -104,26 +169,13 @@ func reap_supervised_children(timeout time.Duration) error { tick := time.NewTicker(5 * time.Millisecond) defer tick.Stop() for { - paths, err := filepath.Glob("/proc/self/task/*/children") - if err != nil || len(paths) == 0 { - return ErrSupervision + children, err := supervised_children() + if err != nil { + return err } - for _, path := range paths { - data, err := os.ReadFile(path) - if os.IsNotExist(err) || errors.Is(err, syscall.ESRCH) { // A runtime thread exited during the scan. - continue - } - if err != nil { - return ErrSupervision - } - for _, value := range strings.Fields(string(data)) { - pid, err := strconv.Atoi(value) - if err != nil || pid <= 1 { - return ErrSupervision - } - if err := syscall.Kill(pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH { - return fmt.Errorf("%w: terminate descendant: %w", ErrSupervision, err) - } + for _, pid := range children { + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH { + return fmt.Errorf("%w: terminate descendant: %w", ErrSupervision, err) } } for { diff --git a/internal/tentacle/supervisor_test.go b/internal/tentacle/supervisor_test.go index dc2641a..ab0f5d9 100644 --- a/internal/tentacle/supervisor_test.go +++ b/internal/tentacle/supervisor_test.go @@ -148,6 +148,22 @@ func TestSupervisionCleansDescendantsAfterReaderDeath(t *testing.T) { wait_supervised_reap(t, pid) } +func TestSupervisionReapsExitedDescendantWhileSessionActive(t *testing.T) { + t.Setenv("CTHULHU_TENTACLE_CGROUP_PARENT", "") + manager := supervised_manager(t) + session := start_active(t, manager, "sample", "spawn-chain") + pid := supervised_descendant(t, session) + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { + t.Fatal(err) + } + wait_supervised_reap(t, pid) + if state, err := session.Status(); state != Active || err != nil { + t.Fatalf("descendant exit retired the live session: %s %v", state, err) + } + // A status flag alone does not prove the main child's protocol still works. + _ = supervised_descendant(t, session) +} + func TestSupervisionMissingProgramReleasesCapacity(t *testing.T) { t.Setenv("CTHULHU_TENTACLE_CGROUP_PARENT", "") manager := supervised_manager(t)