feat(session): close ephemeral sessions when their last client exits (#288)
* feat(session): close ephemeral sessions when their last client exits
Add ephemeral.close-on-exit (default on). The client sends Detach{Closed}
on SIGHUP/SIGTERM with a bounded 300ms send; the daemon removes an
ephemeral session once no live, suspended, parked, or parking attachment
remains, and re-checks after park and suspension expiry. Bump protocol
version to 60.
* fix(session): fence ephemeral close-on-exit against rename and races
Check the ephemeral flag under the architecture locks at kill snapshot and
publication, log a lost race at debug, route Detach frames through a
table test, and keep the close-on-exit off path covered by the existing
expiry tests.- commit
- def4c6eaab6717d78e85366a27390c96b40470ec
- author
- bnema · · 2026-09-25 20:07 UTC
- committer
- GitHub · · 2026-09-25 20:07 UTC
- parent
- 6253aa4
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
27 files · +459 -27
M AGENTS.md +1 -1
| @@ -75,7 +75,7 @@ Typed messages and negotiated version live in `internal/protocol`. Remote discov | |||
| 75 | 75 | ||
| 76 | 76 | ## Session flow | |
| 77 | 77 | ||
| 78 | - | - Ephemeral numbered sessions survive detach while the daemon retains them, but are not persisted. | |
| 78 | + | - Ephemeral numbered sessions survive detach while the daemon retains them, but are not persisted. With `ephemeral.close-on-exit` (default on) the daemon removes one when its last attachment ends definitively: a client `Detach{Closed}` (sent on SIGHUP/SIGTERM) or an expired park/suspension. Parked, parking, suspended, or live attachments keep it. | |
| 79 | 79 | - Named sessions survive headless and persist across daemon restarts. | |
| 80 | 80 | - The daemon starts on first use and survives an empty session registry: final session removal is not shutdown, so it can be reused; it ends only on an explicit shutdown request, `kill --all`, or process cancellation. | |
| 81 | 81 | - A bulk purge (`kill --sessions`) takes a transient admission gate and bounds each phase with its own deadline: admission drain, stopped/broken sweep, and a shared live-teardown budget. A repository delete that ignores cancellation is detached behind an exact name/incarnation/created-time fence and reported as a typed failure instead of holding purge admission or control. | |
| @@ -75,7 +75,7 @@ Typed messages and negotiated version live in `internal/protocol`. Remote discov | |||||
| 75 | 75 | ||||
| 76 | ## Session flow | 76 | ## Session flow | ||
| 77 | 77 | ||||
| 78 | - | - Ephemeral numbered sessions survive detach while the daemon retains them, but are not persisted. | 78 | + | - Ephemeral numbered sessions survive detach while the daemon retains them, but are not persisted. With `ephemeral.close-on-exit` (default on) the daemon removes one when its last attachment ends definitively: a client `Detach{Closed}` (sent on SIGHUP/SIGTERM) or an expired park/suspension. Parked, parking, suspended, or live attachments keep it. |
| 79 | - Named sessions survive headless and persist across daemon restarts. | 79 | - Named sessions survive headless and persist across daemon restarts. | ||
| 80 | - The daemon starts on first use and survives an empty session registry: final session removal is not shutdown, so it can be reused; it ends only on an explicit shutdown request, `kill --all`, or process cancellation. | 80 | - The daemon starts on first use and survives an empty session registry: final session removal is not shutdown, so it can be reused; it ends only on an explicit shutdown request, `kill --all`, or process cancellation. | ||
| 81 | - A bulk purge (`kill --sessions`) takes a transient admission gate and bounds each phase with its own deadline: admission drain, stopped/broken sweep, and a shared live-teardown budget. A repository delete that ignores cancellation is detached behind an exact name/incarnation/created-time fence and reported as a typed failure instead of holding purge admission or control. | 81 | - A bulk purge (`kill --sessions`) takes a transient admission gate and bounds each phase with its own deadline: admission drain, stopped/broken sweep, and a shared live-teardown budget. A repository delete that ignores cancellation is detached behind an exact name/incarnation/created-time fence and reported as a typed failure instead of holding purge admission or control. | ||
M README.md +1 -1
| @@ -58,7 +58,7 @@ vev kill --all stop vev entirely | |||
| 58 | 58 | vev --web-daemon start the browser terminal | |
| 59 | 59 | ``` | |
| 60 | 60 | ||
| 61 | - | - **Numbered sessions** survive detach, not a daemon stop. | |
| 61 | + | - **Numbered sessions** survive detach, not a daemon stop. They close when their last client exits ([`ephemeral.close-on-exit`](docs/configuration.md#ephemeral-sessions)). | |
| 62 | 62 | - **Named sessions** also come back after `kill --all` or a daemon restart ([recovery](docs/durable-session-recovery.md)). | |
| 63 | 63 | - The daemon starts on first use. Exiting the last shell of a session returns you to your previous one. | |
| 64 | 64 | ||
| @@ -58,7 +58,7 @@ vev kill --all stop vev entirely | |||||
| 58 | vev --web-daemon start the browser terminal | 58 | vev --web-daemon start the browser terminal | ||
| 59 | ``` | 59 | ``` | ||
| 60 | 60 | ||||
| 61 | - | - **Numbered sessions** survive detach, not a daemon stop. | 61 | + | - **Numbered sessions** survive detach, not a daemon stop. They close when their last client exits ([`ephemeral.close-on-exit`](docs/configuration.md#ephemeral-sessions)). |
| 62 | - **Named sessions** also come back after `kill --all` or a daemon restart ([recovery](docs/durable-session-recovery.md)). | 62 | - **Named sessions** also come back after `kill --all` or a daemon restart ([recovery](docs/durable-session-recovery.md)). | ||
| 63 | - The daemon starts on first use. Exiting the last shell of a session returns you to your previous one. | 63 | - The daemon starts on first use. Exiting the last shell of a session returns you to your previous one. | ||
| 64 | 64 | ||||
M docs/configuration.md +14 -0
| @@ -37,6 +37,9 @@ nav.overflow-sessions = off | |||
| 37 | 37 | # Show the focused pane's terminal title in tab labels; off keeps the process name only. | |
| 38 | 38 | tabs.terminal-title = on | |
| 39 | 39 | ||
| 40 | + | # Remove a numbered (ephemeral) session when its last client exits. | |
| 41 | + | ephemeral.close-on-exit = on | |
| 42 | + | ||
| 40 | 43 | # Rebindable actions. Leave a line out to keep its built-in binding. | |
| 41 | 44 | open-palette = alt+space | |
| 42 | 45 | toggle-floating-pane = alt+f | |
| @@ -118,6 +121,17 @@ code.yank-last-notification = YLN | |||
| 118 | 121 | - A reload applies to existing panes right away. Lowering a limit drops the oldest rows. | |
| 119 | 122 | - Idle panes compress old history in the background. This saves memory but does not raise the limit. | |
| 120 | 123 | ||
| 124 | + | ## Ephemeral sessions | |
| 125 | + | ||
| 126 | + | With `ephemeral.close-on-exit = on` (default), a numbered session is removed once its last client is gone for good: | |
| 127 | + | ||
| 128 | + | - The client process ends (terminal closed, SIGHUP, or SIGTERM). The client tells the daemon on a best-effort basis; if that message is lost, the rule below applies. | |
| 129 | + | - A client that lost its connection does not reconnect before the resume window (15 minutes) expires. | |
| 130 | + | ||
| 131 | + | The session stays while any other client is attached. A detach (keybinding, palette, or picker switch) keeps it, and so does a network drop within the resume window. Named sessions are never affected. Set `off` to keep numbered sessions until you kill them. | |
| 132 | + | ||
| 133 | + | Closing the terminal of a client running inside SSH also sends SIGHUP, so a dropped SSH connection removes the session. Use a named session for work you want to resume. | |
| 134 | + | ||
| 121 | 135 | ## Development environments | |
| 122 | 136 | ||
| 123 | 137 | Set `VEV_ENV=<name>` to move every vev file into `.dev/<name>/` under the current directory. Use it to test a build without touching your real sessions. | |
| @@ -37,6 +37,9 @@ nav.overflow-sessions = off | |||||
| 37 | # Show the focused pane's terminal title in tab labels; off keeps the process name only. | 37 | # Show the focused pane's terminal title in tab labels; off keeps the process name only. | ||
| 38 | tabs.terminal-title = on | 38 | tabs.terminal-title = on | ||
| 39 | 39 | ||||
| 40 | + | # Remove a numbered (ephemeral) session when its last client exits. | |||
| 41 | + | ephemeral.close-on-exit = on | |||
| 42 | + | ||||
| 40 | # Rebindable actions. Leave a line out to keep its built-in binding. | 43 | # Rebindable actions. Leave a line out to keep its built-in binding. | ||
| 41 | open-palette = alt+space | 44 | open-palette = alt+space | ||
| 42 | toggle-floating-pane = alt+f | 45 | toggle-floating-pane = alt+f | ||
| @@ -118,6 +121,17 @@ code.yank-last-notification = YLN | |||||
| 118 | - A reload applies to existing panes right away. Lowering a limit drops the oldest rows. | 121 | - A reload applies to existing panes right away. Lowering a limit drops the oldest rows. | ||
| 119 | - Idle panes compress old history in the background. This saves memory but does not raise the limit. | 122 | - Idle panes compress old history in the background. This saves memory but does not raise the limit. | ||
| 120 | 123 | ||||
| 124 | + | ## Ephemeral sessions | |||
| 125 | + | ||||
| 126 | + | With `ephemeral.close-on-exit = on` (default), a numbered session is removed once its last client is gone for good: | |||
| 127 | + | ||||
| 128 | + | - The client process ends (terminal closed, SIGHUP, or SIGTERM). The client tells the daemon on a best-effort basis; if that message is lost, the rule below applies. | |||
| 129 | + | - A client that lost its connection does not reconnect before the resume window (15 minutes) expires. | |||
| 130 | + | ||||
| 131 | + | The session stays while any other client is attached. A detach (keybinding, palette, or picker switch) keeps it, and so does a network drop within the resume window. Named sessions are never affected. Set `off` to keep numbered sessions until you kill them. | |||
| 132 | + | ||||
| 133 | + | Closing the terminal of a client running inside SSH also sends SIGHUP, so a dropped SSH connection removes the session. Use a named session for work you want to resume. | |||
| 134 | + | ||||
| 121 | ## Development environments | 135 | ## Development environments | ||
| 122 | 136 | ||||
| 123 | Set `VEV_ENV=<name>` to move every vev file into `.dev/<name>/` under the current directory. Use it to test a build without touching your real sessions. | 137 | Set `VEV_ENV=<name>` to move every vev file into `.dev/<name>/` under the current directory. Use it to test a build without touching your real sessions. | ||
M internal/adapters/config/config.go +9 -0
| @@ -35,6 +35,7 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||
| 35 | 35 | seenPaletteKeys := make(map[string]bool) | |
| 36 | 36 | seenNavKeys := make(map[string]bool) | |
| 37 | 37 | seenTabsKeys := make(map[string]bool) | |
| 38 | + | seenEphemeralKeys := make(map[string]bool) | |
| 38 | 39 | seenScrollbackKeys := make(map[string]bool) | |
| 39 | 40 | seenWebKeys := make(map[string]bool) | |
| 40 | 41 | ||
| @@ -153,6 +154,14 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||
| 153 | 154 | continue | |
| 154 | 155 | } | |
| 155 | 156 | cfg.Tabs.TerminalTitle = on | |
| 157 | + | case key == "ephemeral.close-on-exit": | |
| 158 | + | warnings = warnDuplicateKey(warnings, seenEphemeralKeys, key, lineNo) | |
| 159 | + | on, ok := parseOnOff(value) | |
| 160 | + | if !ok { | |
| 161 | + | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid ephemeral.close-on-exit %q", value)}) | |
| 162 | + | continue | |
| 163 | + | } | |
| 164 | + | cfg.Ephemeral.CloseOnExit = on | |
| 156 | 165 | case key == "copy.word-separators": | |
| 157 | 166 | warnings = warnDuplicateKey(warnings, seenCopyKeys, key, lineNo) | |
| 158 | 167 | separators, ok := parseConfigString(value) | |
| @@ -35,6 +35,7 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||||
| 35 | seenPaletteKeys := make(map[string]bool) | 35 | seenPaletteKeys := make(map[string]bool) | ||
| 36 | seenNavKeys := make(map[string]bool) | 36 | seenNavKeys := make(map[string]bool) | ||
| 37 | seenTabsKeys := make(map[string]bool) | 37 | seenTabsKeys := make(map[string]bool) | ||
| 38 | + | seenEphemeralKeys := make(map[string]bool) | |||
| 38 | seenScrollbackKeys := make(map[string]bool) | 39 | seenScrollbackKeys := make(map[string]bool) | ||
| 39 | seenWebKeys := make(map[string]bool) | 40 | seenWebKeys := make(map[string]bool) | ||
| 40 | 41 | ||||
| @@ -153,6 +154,14 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) { | |||||
| 153 | continue | 154 | continue | ||
| 154 | } | 155 | } | ||
| 155 | cfg.Tabs.TerminalTitle = on | 156 | cfg.Tabs.TerminalTitle = on | ||
| 157 | + | case key == "ephemeral.close-on-exit": | |||
| 158 | + | warnings = warnDuplicateKey(warnings, seenEphemeralKeys, key, lineNo) | |||
| 159 | + | on, ok := parseOnOff(value) | |||
| 160 | + | if !ok { | |||
| 161 | + | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid ephemeral.close-on-exit %q", value)}) | |||
| 162 | + | continue | |||
| 163 | + | } | |||
| 164 | + | cfg.Ephemeral.CloseOnExit = on | |||
| 156 | case key == "copy.word-separators": | 165 | case key == "copy.word-separators": | ||
| 157 | warnings = warnDuplicateKey(warnings, seenCopyKeys, key, lineNo) | 166 | warnings = warnDuplicateKey(warnings, seenCopyKeys, key, lineNo) | ||
| 158 | separators, ok := parseConfigString(value) | 167 | separators, ok := parseConfigString(value) | ||
M internal/adapters/config/config_test.go +43 -0
| @@ -349,6 +349,9 @@ func TestParse(t *testing.T) { | |||
| 349 | 349 | if !tt.want.Tabs.TerminalTitle { | |
| 350 | 350 | tt.want.Tabs = domain.Defaults().Tabs | |
| 351 | 351 | } | |
| 352 | + | if !tt.want.Ephemeral.CloseOnExit { | |
| 353 | + | tt.want.Ephemeral = domain.Defaults().Ephemeral | |
| 354 | + | } | |
| 352 | 355 | if tt.want.Snapshot.RestoreProcesses == nil && !tt.want.Snapshot.RestoreProcessesSet { | |
| 353 | 356 | tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses()...) | |
| 354 | 357 | } | |
| @@ -477,6 +480,46 @@ func TestParseTabsTerminalTitle(t *testing.T) { | |||
| 477 | 480 | } | |
| 478 | 481 | } | |
| 479 | 482 | ||
| 483 | + | func TestParseEphemeralCloseOnExit(t *testing.T) { | |
| 484 | + | t.Parallel() | |
| 485 | + | ||
| 486 | + | tests := []struct { | |
| 487 | + | name string | |
| 488 | + | input string | |
| 489 | + | want bool | |
| 490 | + | wantWarnings []domain.Warning | |
| 491 | + | }{ | |
| 492 | + | {name: "absent key defaults on", input: "theme = dark\n", want: true}, | |
| 493 | + | {name: "on", input: "ephemeral.close-on-exit = on\n", want: true}, | |
| 494 | + | {name: "off", input: "ephemeral.close-on-exit = off\n", want: false}, | |
| 495 | + | { | |
| 496 | + | name: "invalid value warns and keeps default", | |
| 497 | + | input: "ephemeral.close-on-exit = kill\n", | |
| 498 | + | want: true, | |
| 499 | + | wantWarnings: []domain.Warning{ | |
| 500 | + | {Line: 1, Msg: "invalid ephemeral.close-on-exit \"kill\""}, | |
| 501 | + | }, | |
| 502 | + | }, | |
| 503 | + | } | |
| 504 | + | ||
| 505 | + | for _, tt := range tests { | |
| 506 | + | t.Run(tt.name, func(t *testing.T) { | |
| 507 | + | t.Parallel() | |
| 508 | + | ||
| 509 | + | got, warnings, err := Parse(strings.NewReader(tt.input)) | |
| 510 | + | if err != nil { | |
| 511 | + | t.Fatalf("Parse() error = %v", err) | |
| 512 | + | } | |
| 513 | + | if got.Ephemeral.CloseOnExit != tt.want { | |
| 514 | + | t.Fatalf("Parse() Ephemeral.CloseOnExit = %v, want %v", got.Ephemeral.CloseOnExit, tt.want) | |
| 515 | + | } | |
| 516 | + | if !reflect.DeepEqual(warnings, tt.wantWarnings) { | |
| 517 | + | t.Fatalf("Parse() warnings = %#v, want %#v", warnings, tt.wantWarnings) | |
| 518 | + | } | |
| 519 | + | }) | |
| 520 | + | } | |
| 521 | + | } | |
| 522 | + | ||
| 480 | 523 | func TestParseNavigationOverflow(t *testing.T) { | |
| 481 | 524 | t.Parallel() | |
| 482 | 525 | ||
| @@ -349,6 +349,9 @@ func TestParse(t *testing.T) { | |||||
| 349 | if !tt.want.Tabs.TerminalTitle { | 349 | if !tt.want.Tabs.TerminalTitle { | ||
| 350 | tt.want.Tabs = domain.Defaults().Tabs | 350 | tt.want.Tabs = domain.Defaults().Tabs | ||
| 351 | } | 351 | } | ||
| 352 | + | if !tt.want.Ephemeral.CloseOnExit { | |||
| 353 | + | tt.want.Ephemeral = domain.Defaults().Ephemeral | |||
| 354 | + | } | |||
| 352 | if tt.want.Snapshot.RestoreProcesses == nil && !tt.want.Snapshot.RestoreProcessesSet { | 355 | if tt.want.Snapshot.RestoreProcesses == nil && !tt.want.Snapshot.RestoreProcessesSet { | ||
| 353 | tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses()...) | 356 | tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses()...) | ||
| 354 | } | 357 | } | ||
| @@ -477,6 +480,46 @@ func TestParseTabsTerminalTitle(t *testing.T) { | |||||
| 477 | } | 480 | } | ||
| 478 | } | 481 | } | ||
| 479 | 482 | ||||
| 483 | + | func TestParseEphemeralCloseOnExit(t *testing.T) { | |||
| 484 | + | t.Parallel() | |||
| 485 | + | ||||
| 486 | + | tests := []struct { | |||
| 487 | + | name string | |||
| 488 | + | input string | |||
| 489 | + | want bool | |||
| 490 | + | wantWarnings []domain.Warning | |||
| 491 | + | }{ | |||
| 492 | + | {name: "absent key defaults on", input: "theme = dark\n", want: true}, | |||
| 493 | + | {name: "on", input: "ephemeral.close-on-exit = on\n", want: true}, | |||
| 494 | + | {name: "off", input: "ephemeral.close-on-exit = off\n", want: false}, | |||
| 495 | + | { | |||
| 496 | + | name: "invalid value warns and keeps default", | |||
| 497 | + | input: "ephemeral.close-on-exit = kill\n", | |||
| 498 | + | want: true, | |||
| 499 | + | wantWarnings: []domain.Warning{ | |||
| 500 | + | {Line: 1, Msg: "invalid ephemeral.close-on-exit \"kill\""}, | |||
| 501 | + | }, | |||
| 502 | + | }, | |||
| 503 | + | } | |||
| 504 | + | ||||
| 505 | + | for _, tt := range tests { | |||
| 506 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 507 | + | t.Parallel() | |||
| 508 | + | ||||
| 509 | + | got, warnings, err := Parse(strings.NewReader(tt.input)) | |||
| 510 | + | if err != nil { | |||
| 511 | + | t.Fatalf("Parse() error = %v", err) | |||
| 512 | + | } | |||
| 513 | + | if got.Ephemeral.CloseOnExit != tt.want { | |||
| 514 | + | t.Fatalf("Parse() Ephemeral.CloseOnExit = %v, want %v", got.Ephemeral.CloseOnExit, tt.want) | |||
| 515 | + | } | |||
| 516 | + | if !reflect.DeepEqual(warnings, tt.wantWarnings) { | |||
| 517 | + | t.Fatalf("Parse() warnings = %#v, want %#v", warnings, tt.wantWarnings) | |||
| 518 | + | } | |||
| 519 | + | }) | |||
| 520 | + | } | |||
| 521 | + | } | |||
| 522 | + | ||||
| 480 | func TestParseNavigationOverflow(t *testing.T) { | 523 | func TestParseNavigationOverflow(t *testing.T) { | ||
| 481 | t.Parallel() | 524 | t.Parallel() | ||
| 482 | 525 | ||||
M internal/adapters/sessionwire/client_test.go +1 -0
| @@ -22,6 +22,7 @@ func TestClientConnectionEncodesEveryClientMessage(t *testing.T) { | |||
| 22 | 22 | {name: "input", message: protocol.Input{InputSeq: 1, Data: []byte("x")}}, | |
| 23 | 23 | {name: "resize", message: protocol.Resize{Size: domain.Size{Cols: 80, Rows: 24}}}, | |
| 24 | 24 | {name: "detach", message: protocol.Detach{}}, | |
| 25 | + | {name: "closed detach", message: protocol.Detach{Closed: true}}, | |
| 25 | 26 | {name: "ping", message: protocol.Ping{}}, | |
| 26 | 27 | {name: "list", message: protocol.List{}}, | |
| 27 | 28 | {name: "kill", message: protocol.Kill{RequestID: 1, Name: "work"}}, | |
| @@ -22,6 +22,7 @@ func TestClientConnectionEncodesEveryClientMessage(t *testing.T) { | |||||
| 22 | {name: "input", message: protocol.Input{InputSeq: 1, Data: []byte("x")}}, | 22 | {name: "input", message: protocol.Input{InputSeq: 1, Data: []byte("x")}}, | ||
| 23 | {name: "resize", message: protocol.Resize{Size: domain.Size{Cols: 80, Rows: 24}}}, | 23 | {name: "resize", message: protocol.Resize{Size: domain.Size{Cols: 80, Rows: 24}}}, | ||
| 24 | {name: "detach", message: protocol.Detach{}}, | 24 | {name: "detach", message: protocol.Detach{}}, | ||
| 25 | + | {name: "closed detach", message: protocol.Detach{Closed: true}}, | |||
| 25 | {name: "ping", message: protocol.Ping{}}, | 26 | {name: "ping", message: protocol.Ping{}}, | ||
| 26 | {name: "list", message: protocol.List{}}, | 27 | {name: "list", message: protocol.List{}}, | ||
| 27 | {name: "kill", message: protocol.Kill{RequestID: 1, Name: "work"}}, | 28 | {name: "kill", message: protocol.Kill{RequestID: 1, Name: "work"}}, | ||
M internal/adapters/sessionwire/proto_envelope.go +2 -2
| @@ -67,7 +67,7 @@ func encodeProtoClient(message protocol.ClientMessage) (*wire.ClientEnvelope, er | |||
| 67 | 67 | } | |
| 68 | 68 | return encodeProtoClient(*m) | |
| 69 | 69 | case protocol.Detach: | |
| 70 | - | return &wire.ClientEnvelope{Payload: &wire.ClientEnvelope_Detach{Detach: &wire.Detach{}}}, nil | |
| 70 | + | return &wire.ClientEnvelope{Payload: &wire.ClientEnvelope_Detach{Detach: &wire.Detach{Closed: m.Closed}}}, nil | |
| 71 | 71 | case *protocol.Detach: | |
| 72 | 72 | if m == nil { | |
| 73 | 73 | return nil, ErrInvalidMessage | |
| @@ -620,7 +620,7 @@ func decodeProtoClient(envelope *wire.ClientEnvelope) (protocol.ClientMessage, e | |||
| 620 | 620 | case *wire.ClientEnvelope_Resize: | |
| 621 | 621 | return resizeFromWire(payload.Resize) | |
| 622 | 622 | case *wire.ClientEnvelope_Detach: | |
| 623 | - | return protocol.Detach{}, nil | |
| 623 | + | return protocol.Detach{Closed: payload.Detach.GetClosed()}, nil | |
| 624 | 624 | case *wire.ClientEnvelope_Ping: | |
| 625 | 625 | return protocol.Ping{}, nil | |
| 626 | 626 | case *wire.ClientEnvelope_List: | |
| @@ -67,7 +67,7 @@ func encodeProtoClient(message protocol.ClientMessage) (*wire.ClientEnvelope, er | |||||
| 67 | } | 67 | } | ||
| 68 | return encodeProtoClient(*m) | 68 | return encodeProtoClient(*m) | ||
| 69 | case protocol.Detach: | 69 | case protocol.Detach: | ||
| 70 | - | return &wire.ClientEnvelope{Payload: &wire.ClientEnvelope_Detach{Detach: &wire.Detach{}}}, nil | 70 | + | return &wire.ClientEnvelope{Payload: &wire.ClientEnvelope_Detach{Detach: &wire.Detach{Closed: m.Closed}}}, nil |
| 71 | case *protocol.Detach: | 71 | case *protocol.Detach: | ||
| 72 | if m == nil { | 72 | if m == nil { | ||
| 73 | return nil, ErrInvalidMessage | 73 | return nil, ErrInvalidMessage | ||
| @@ -620,7 +620,7 @@ func decodeProtoClient(envelope *wire.ClientEnvelope) (protocol.ClientMessage, e | |||||
| 620 | case *wire.ClientEnvelope_Resize: | 620 | case *wire.ClientEnvelope_Resize: | ||
| 621 | return resizeFromWire(payload.Resize) | 621 | return resizeFromWire(payload.Resize) | ||
| 622 | case *wire.ClientEnvelope_Detach: | 622 | case *wire.ClientEnvelope_Detach: | ||
| 623 | - | return protocol.Detach{}, nil | 623 | + | return protocol.Detach{Closed: payload.Detach.GetClosed()}, nil |
| 624 | case *wire.ClientEnvelope_Ping: | 624 | case *wire.ClientEnvelope_Ping: | ||
| 625 | return protocol.Ping{}, nil | 625 | return protocol.Ping{}, nil | ||
| 626 | case *wire.ClientEnvelope_List: | 626 | case *wire.ClientEnvelope_List: | ||
M internal/adapters/sessionwire/server_test.go +1 -0
| @@ -26,6 +26,7 @@ func TestServerConnectionDecodesEveryClientMessage(t *testing.T) { | |||
| 26 | 26 | {name: "input", want: protocol.Input{InputSeq: 1, Data: []byte("x")}}, | |
| 27 | 27 | {name: "resize", want: protocol.Resize{Size: domain.Size{Cols: 80, Rows: 24}}}, | |
| 28 | 28 | {name: "detach", want: protocol.Detach{}}, | |
| 29 | + | {name: "closed detach", want: protocol.Detach{Closed: true}}, | |
| 29 | 30 | {name: "ping", want: protocol.Ping{}}, | |
| 30 | 31 | {name: "list", want: protocol.List{}}, | |
| 31 | 32 | {name: "kill", want: protocol.Kill{RequestID: 1, Name: "work"}}, | |
| @@ -26,6 +26,7 @@ func TestServerConnectionDecodesEveryClientMessage(t *testing.T) { | |||||
| 26 | {name: "input", want: protocol.Input{InputSeq: 1, Data: []byte("x")}}, | 26 | {name: "input", want: protocol.Input{InputSeq: 1, Data: []byte("x")}}, | ||
| 27 | {name: "resize", want: protocol.Resize{Size: domain.Size{Cols: 80, Rows: 24}}}, | 27 | {name: "resize", want: protocol.Resize{Size: domain.Size{Cols: 80, Rows: 24}}}, | ||
| 28 | {name: "detach", want: protocol.Detach{}}, | 28 | {name: "detach", want: protocol.Detach{}}, | ||
| 29 | + | {name: "closed detach", want: protocol.Detach{Closed: true}}, | |||
| 29 | {name: "ping", want: protocol.Ping{}}, | 30 | {name: "ping", want: protocol.Ping{}}, | ||
| 30 | {name: "list", want: protocol.List{}}, | 31 | {name: "list", want: protocol.List{}}, | ||
| 31 | {name: "kill", want: protocol.Kill{RequestID: 1, Name: "work"}}, | 32 | {name: "kill", want: protocol.Kill{RequestID: 1, Name: "work"}}, | ||
A internal/app/client_signals.go +37 -0
| @@ -0,0 +1,37 @@ | |||
| 1 | + | package app | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "os" | |
| 6 | + | "os/signal" | |
| 7 | + | "syscall" | |
| 8 | + | ||
| 9 | + | "github.com/bnema/vev/internal/usecase/client" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | // clientSignalContext cancels an attached client on process signals. SIGHUP | |
| 13 | + | // (terminal closed) and SIGTERM use client.ErrClientClosed as the cause so the | |
| 14 | + | // attachment reports Detach{Closed} and the daemon applies | |
| 15 | + | // ephemeral.close-on-exit. SIGINT keeps a plain cancellation. | |
| 16 | + | func clientSignalContext(parent context.Context) (context.Context, context.CancelFunc) { | |
| 17 | + | ctx, cancel := context.WithCancelCause(parent) | |
| 18 | + | signals := make(chan os.Signal, 1) | |
| 19 | + | signal.Notify(signals, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) | |
| 20 | + | done := make(chan struct{}) | |
| 21 | + | go func() { | |
| 22 | + | select { | |
| 23 | + | case sig := <-signals: | |
| 24 | + | if sig == syscall.SIGINT { | |
| 25 | + | cancel(context.Canceled) | |
| 26 | + | return | |
| 27 | + | } | |
| 28 | + | cancel(client.ErrClientClosed) | |
| 29 | + | case <-done: | |
| 30 | + | } | |
| 31 | + | }() | |
| 32 | + | return ctx, func() { | |
| 33 | + | signal.Stop(signals) | |
| 34 | + | close(done) | |
| 35 | + | cancel(context.Canceled) | |
| 36 | + | } | |
| 37 | + | } | |
| @@ -0,0 +1,37 @@ | |||||
| 1 | + | package app | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "os" | |||
| 6 | + | "os/signal" | |||
| 7 | + | "syscall" | |||
| 8 | + | ||||
| 9 | + | "github.com/bnema/vev/internal/usecase/client" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | // clientSignalContext cancels an attached client on process signals. SIGHUP | |||
| 13 | + | // (terminal closed) and SIGTERM use client.ErrClientClosed as the cause so the | |||
| 14 | + | // attachment reports Detach{Closed} and the daemon applies | |||
| 15 | + | // ephemeral.close-on-exit. SIGINT keeps a plain cancellation. | |||
| 16 | + | func clientSignalContext(parent context.Context) (context.Context, context.CancelFunc) { | |||
| 17 | + | ctx, cancel := context.WithCancelCause(parent) | |||
| 18 | + | signals := make(chan os.Signal, 1) | |||
| 19 | + | signal.Notify(signals, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) | |||
| 20 | + | done := make(chan struct{}) | |||
| 21 | + | go func() { | |||
| 22 | + | select { | |||
| 23 | + | case sig := <-signals: | |||
| 24 | + | if sig == syscall.SIGINT { | |||
| 25 | + | cancel(context.Canceled) | |||
| 26 | + | return | |||
| 27 | + | } | |||
| 28 | + | cancel(client.ErrClientClosed) | |||
| 29 | + | case <-done: | |||
| 30 | + | } | |||
| 31 | + | }() | |||
| 32 | + | return ctx, func() { | |||
| 33 | + | signal.Stop(signals) | |||
| 34 | + | close(done) | |||
| 35 | + | cancel(context.Canceled) | |||
| 36 | + | } | |||
| 37 | + | } | |||
M internal/app/run.go +1 -1
| @@ -883,7 +883,7 @@ func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (re | |||
| 883 | 883 | } | |
| 884 | 884 | return errors.New("vev: sessions should be nested with care; unset VEV to force") | |
| 885 | 885 | } | |
| 886 | - | ctx, stop := signal.NotifyContext(ctx, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) | |
| 886 | + | ctx, stop := clientSignalContext(ctx) | |
| 887 | 887 | defer stop() | |
| 888 | 888 | log, logCloser, err := configureLogging(logging.Client, false) | |
| 889 | 889 | if err != nil { | |
| @@ -883,7 +883,7 @@ func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) (re | |||||
| 883 | } | 883 | } | ||
| 884 | return errors.New("vev: sessions should be nested with care; unset VEV to force") | 884 | return errors.New("vev: sessions should be nested with care; unset VEV to force") | ||
| 885 | } | 885 | } | ||
| 886 | - | ctx, stop := signal.NotifyContext(ctx, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) | 886 | + | ctx, stop := clientSignalContext(ctx) |
| 887 | defer stop() | 887 | defer stop() | ||
| 888 | log, logCloser, err := configureLogging(logging.Client, false) | 888 | log, logCloser, err := configureLogging(logging.Client, false) | ||
| 889 | if err != nil { | 889 | if err != nil { | ||
M internal/app/ui_driver.go +1 -3
| @@ -6,12 +6,10 @@ import ( | |||
| 6 | 6 | "fmt" | |
| 7 | 7 | "io" | |
| 8 | 8 | "os" | |
| 9 | - | "os/signal" | |
| 10 | 9 | "path/filepath" | |
| 11 | 10 | "strconv" | |
| 12 | 11 | "strings" | |
| 13 | 12 | "sync" | |
| 14 | - | "syscall" | |
| 15 | 13 | "time" | |
| 16 | 14 | ||
| 17 | 15 | "github.com/bnema/vev/internal/adapters/clock" | |
| @@ -433,7 +431,7 @@ func runAttachWithOptions(ctx context.Context, intent uint8, name, remoteTarget | |||
| 433 | 431 | if options.control { | |
| 434 | 432 | options.observe = true | |
| 435 | 433 | } | |
| 436 | - | ctx, stop := signal.NotifyContext(ctx, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) | |
| 434 | + | ctx, stop := clientSignalContext(ctx) | |
| 437 | 435 | defer stop() | |
| 438 | 436 | log, logCloser, err := configureLogging(logging.Client, false) | |
| 439 | 437 | if err != nil { | |
| @@ -6,12 +6,10 @@ import ( | |||||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | "io" | 7 | "io" | ||
| 8 | "os" | 8 | "os" | ||
| 9 | - | "os/signal" | |||
| 10 | "path/filepath" | 9 | "path/filepath" | ||
| 11 | "strconv" | 10 | "strconv" | ||
| 12 | "strings" | 11 | "strings" | ||
| 13 | "sync" | 12 | "sync" | ||
| 14 | - | "syscall" | |||
| 15 | "time" | 13 | "time" | ||
| 16 | 14 | ||||
| 17 | "github.com/bnema/vev/internal/adapters/clock" | 15 | "github.com/bnema/vev/internal/adapters/clock" | ||
| @@ -433,7 +431,7 @@ func runAttachWithOptions(ctx context.Context, intent uint8, name, remoteTarget | |||||
| 433 | if options.control { | 431 | if options.control { | ||
| 434 | options.observe = true | 432 | options.observe = true | ||
| 435 | } | 433 | } | ||
| 436 | - | ctx, stop := signal.NotifyContext(ctx, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) | 434 | + | ctx, stop := clientSignalContext(ctx) |
| 437 | defer stop() | 435 | defer stop() | ||
| 438 | log, logCloser, err := configureLogging(logging.Client, false) | 436 | log, logCloser, err := configureLogging(logging.Client, false) | ||
| 439 | if err != nil { | 437 | if err != nil { | ||
M internal/domain/config.go +12 -0
| @@ -83,6 +83,14 @@ type TabsConfig struct { | |||
| 83 | 83 | TerminalTitle bool | |
| 84 | 84 | } | |
| 85 | 85 | ||
| 86 | + | // EphemeralConfig contains numbered-session lifetime settings. | |
| 87 | + | type EphemeralConfig struct { | |
| 88 | + | // CloseOnExit removes an ephemeral session once its last attachment ends | |
| 89 | + | // definitively: a closed client process (Detach{Closed}), or an expired | |
| 90 | + | // park or suspension. A user detach keeps it. | |
| 91 | + | CloseOnExit bool | |
| 92 | + | } | |
| 93 | + | ||
| 86 | 94 | // ScrollbackConfig is vev's per-pane retention policy. The terminal library | |
| 87 | 95 | // only enforces these application-selected limits. Zero megabytes disables | |
| 88 | 96 | // history; zero lines removes the additional line ceiling. | |
| @@ -122,6 +130,7 @@ type Config struct { | |||
| 122 | 130 | Floating FloatingConfig | |
| 123 | 131 | Nav NavConfig | |
| 124 | 132 | Tabs TabsConfig | |
| 133 | + | Ephemeral EphemeralConfig | |
| 125 | 134 | Scrollback ScrollbackConfig | |
| 126 | 135 | } | |
| 127 | 136 | ||
| @@ -160,6 +169,9 @@ func Defaults() Config { | |||
| 160 | 169 | Tabs: TabsConfig{ | |
| 161 | 170 | TerminalTitle: true, | |
| 162 | 171 | }, | |
| 172 | + | Ephemeral: EphemeralConfig{ | |
| 173 | + | CloseOnExit: true, | |
| 174 | + | }, | |
| 163 | 175 | Scrollback: DefaultScrollbackConfig(), | |
| 164 | 176 | } | |
| 165 | 177 | } | |
| @@ -83,6 +83,14 @@ type TabsConfig struct { | |||||
| 83 | TerminalTitle bool | 83 | TerminalTitle bool | ||
| 84 | } | 84 | } | ||
| 85 | 85 | ||||
| 86 | + | // EphemeralConfig contains numbered-session lifetime settings. | |||
| 87 | + | type EphemeralConfig struct { | |||
| 88 | + | // CloseOnExit removes an ephemeral session once its last attachment ends | |||
| 89 | + | // definitively: a closed client process (Detach{Closed}), or an expired | |||
| 90 | + | // park or suspension. A user detach keeps it. | |||
| 91 | + | CloseOnExit bool | |||
| 92 | + | } | |||
| 93 | + | ||||
| 86 | // ScrollbackConfig is vev's per-pane retention policy. The terminal library | 94 | // ScrollbackConfig is vev's per-pane retention policy. The terminal library | ||
| 87 | // only enforces these application-selected limits. Zero megabytes disables | 95 | // only enforces these application-selected limits. Zero megabytes disables | ||
| 88 | // history; zero lines removes the additional line ceiling. | 96 | // history; zero lines removes the additional line ceiling. | ||
| @@ -122,6 +130,7 @@ type Config struct { | |||||
| 122 | Floating FloatingConfig | 130 | Floating FloatingConfig | ||
| 123 | Nav NavConfig | 131 | Nav NavConfig | ||
| 124 | Tabs TabsConfig | 132 | Tabs TabsConfig | ||
| 133 | + | Ephemeral EphemeralConfig | |||
| 125 | Scrollback ScrollbackConfig | 134 | Scrollback ScrollbackConfig | ||
| 126 | } | 135 | } | ||
| 127 | 136 | ||||
| @@ -160,6 +169,9 @@ func Defaults() Config { | |||||
| 160 | Tabs: TabsConfig{ | 169 | Tabs: TabsConfig{ | ||
| 161 | TerminalTitle: true, | 170 | TerminalTitle: true, | ||
| 162 | }, | 171 | }, | ||
| 172 | + | Ephemeral: EphemeralConfig{ | |||
| 173 | + | CloseOnExit: true, | |||
| 174 | + | }, | |||
| 163 | Scrollback: DefaultScrollbackConfig(), | 175 | Scrollback: DefaultScrollbackConfig(), | ||
| 164 | } | 176 | } | ||
| 165 | } | 177 | } | ||
M internal/protocol/session.go +5 -1
| @@ -170,7 +170,11 @@ const ( | |||
| 170 | 170 | ClientNoticeLinkConnected uint8 = 4 | |
| 171 | 171 | ) | |
| 172 | 172 | ||
| 173 | - | type Detach struct{} | |
| 173 | + | // Detach releases the attachment. Closed reports that the client process is | |
| 174 | + | // ending, so the daemon applies the ephemeral close-on-exit policy. | |
| 175 | + | type Detach struct { | |
| 176 | + | Closed bool | |
| 177 | + | } | |
| 174 | 178 | type Ping struct{} | |
| 175 | 179 | type Pong struct{} | |
| 176 | 180 | ||
| @@ -170,7 +170,11 @@ const ( | |||||
| 170 | ClientNoticeLinkConnected uint8 = 4 | 170 | ClientNoticeLinkConnected uint8 = 4 | ||
| 171 | ) | 171 | ) | ||
| 172 | 172 | ||||
| 173 | - | type Detach struct{} | 173 | + | // Detach releases the attachment. Closed reports that the client process is |
| 174 | + | // ending, so the daemon applies the ephemeral close-on-exit policy. | |||
| 175 | + | type Detach struct { | |||
| 176 | + | Closed bool | |||
| 177 | + | } | |||
| 174 | type Ping struct{} | 178 | type Ping struct{} | ||
| 175 | type Pong struct{} | 179 | type Pong struct{} | ||
| 176 | 180 | ||||
M internal/protocol/wire/schema/session.proto +5 -2
| @@ -118,8 +118,11 @@ message Detached { | |||
| 118 | 118 | uint32 reason = 1; | |
| 119 | 119 | } | |
| 120 | 120 | ||
| 121 | - | // Detach asks the daemon to release the attachment. Empty marker. | |
| 122 | - | message Detach {} | |
| 121 | + | // Detach asks the daemon to release the attachment. closed reports that the | |
| 122 | + | // client process is ending (terminal closed or terminated), not a user detach. | |
| 123 | + | message Detach { | |
| 124 | + | bool closed = 1; | |
| 125 | + | } | |
| 123 | 126 | ||
| 124 | 127 | // Ping / Pong are empty keepalive markers. | |
| 125 | 128 | message Ping {} | |
| @@ -118,8 +118,11 @@ message Detached { | |||||
| 118 | uint32 reason = 1; | 118 | uint32 reason = 1; | ||
| 119 | } | 119 | } | ||
| 120 | 120 | ||||
| 121 | - | // Detach asks the daemon to release the attachment. Empty marker. | 121 | + | // Detach asks the daemon to release the attachment. closed reports that the |
| 122 | - | message Detach {} | 122 | + | // client process is ending (terminal closed or terminated), not a user detach. |
| 123 | + | message Detach { | |||
| 124 | + | bool closed = 1; | |||
| 125 | + | } | |||
| 123 | 126 | ||||
| 124 | // Ping / Pong are empty keepalive markers. | 127 | // Ping / Pong are empty keepalive markers. | ||
| 125 | message Ping {} | 128 | message Ping {} | ||
M internal/protocol/wire/session.pb.go +13 -3 generated file: collapsed
M internal/usecase/client/attachment_session.go +30 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "fmt" | |
| 7 | 7 | "io" | |
| 8 | 8 | "slices" | |
| 9 | + | "time" | |
| 9 | 10 | ||
| 10 | 11 | "github.com/bnema/vev/internal/domain" | |
| 11 | 12 | "github.com/bnema/vev/internal/ports" | |
| @@ -27,6 +28,15 @@ import ( | |||
| 27 | 28 | // AttachmentForeground. Input, geometry and lifecycle sends therefore retain | |
| 28 | 29 | // the foreground token and stop when the grant is superseded. | |
| 29 | 30 | ||
| 31 | + | // closedDetachTimeout bounds the final Detach{Closed} send when the client | |
| 32 | + | // process is ending, so a slow broker never delays terminal shutdown. | |
| 33 | + | const closedDetachTimeout = 300 * time.Millisecond | |
| 34 | + | ||
| 35 | + | // ErrClientClosed is the cancellation cause composition uses when the client | |
| 36 | + | // process is ending (SIGHUP/SIGTERM). The attachment then sends Detach{Closed} | |
| 37 | + | // so the daemon can apply ephemeral.close-on-exit instead of parking. | |
| 38 | + | var ErrClientClosed = errors.New("client: process closed") | |
| 39 | + | ||
| 30 | 40 | var ( | |
| 31 | 41 | // errAttachmentDeadline is the stable local sentinel of an expired | |
| 32 | 42 | // attachment budget. The supervisor wraps it in a typed timeout. | |
| @@ -777,6 +787,9 @@ func (w *sessionAttachmentWorker) awaitServer(ctx context.Context, fg Attachment | |||
| 777 | 787 | // application failure on the stream. | |
| 778 | 788 | func (w *sessionAttachmentWorker) settle(ctx context.Context, fg AttachmentForeground, stream ports.BrokerLogicalConnection, token AttachmentToken, err error) AttachmentEvent { | |
| 779 | 789 | if ctxErr := ctx.Err(); ctxErr != nil { | |
| 790 | + | if errors.Is(context.Cause(ctx), ErrClientClosed) { | |
| 791 | + | w.sendClosed(stream) | |
| 792 | + | } | |
| 780 | 793 | return AttachmentEvent{Token: token, Kind: AttachmentEventFailed, Err: errors.Join(ctxErr, err)} | |
| 781 | 794 | } | |
| 782 | 795 | if cause := streamErr(stream); cause != nil { | |
| @@ -791,6 +804,23 @@ func (w *sessionAttachmentWorker) settle(ctx context.Context, fg AttachmentForeg | |||
| 791 | 804 | return AttachmentEvent{Token: token, Kind: AttachmentEventEnded} | |
| 792 | 805 | } | |
| 793 | 806 | ||
| 807 | + | // sendClosed makes a best-effort, bounded attempt to tell the daemon that this | |
| 808 | + | // client process is ending. It bypasses send's cancellation because the run | |
| 809 | + | // context is already done; the daemon parks the attachment if it never arrives. | |
| 810 | + | func (w *sessionAttachmentWorker) sendClosed(stream ports.BrokerLogicalConnection) { | |
| 811 | + | if supervisorNil(stream) { | |
| 812 | + | return | |
| 813 | + | } | |
| 814 | + | sent := make(chan error, 1) | |
| 815 | + | go func() { sent <- stream.SendClient(protocol.Detach{Closed: true}) }() | |
| 816 | + | timer := w.cfg.Clock.NewTimer(closedDetachTimeout) | |
| 817 | + | defer stopSupervisorTimer(timer) | |
| 818 | + | select { | |
| 819 | + | case <-sent: | |
| 820 | + | case <-timer.C(): | |
| 821 | + | } | |
| 822 | + | } | |
| 823 | + | ||
| 794 | 824 | // streamErr reads the logical stream's stable terminal cause without treating a | |
| 795 | 825 | // nil stream as a failure. | |
| 796 | 826 | func streamErr(stream ports.BrokerLogicalConnection) error { | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | "io" | 7 | "io" | ||
| 8 | "slices" | 8 | "slices" | ||
| 9 | + | "time" | |||
| 9 | 10 | ||||
| 10 | "github.com/bnema/vev/internal/domain" | 11 | "github.com/bnema/vev/internal/domain" | ||
| 11 | "github.com/bnema/vev/internal/ports" | 12 | "github.com/bnema/vev/internal/ports" | ||
| @@ -27,6 +28,15 @@ import ( | |||||
| 27 | // AttachmentForeground. Input, geometry and lifecycle sends therefore retain | 28 | // AttachmentForeground. Input, geometry and lifecycle sends therefore retain | ||
| 28 | // the foreground token and stop when the grant is superseded. | 29 | // the foreground token and stop when the grant is superseded. | ||
| 29 | 30 | ||||
| 31 | + | // closedDetachTimeout bounds the final Detach{Closed} send when the client | |||
| 32 | + | // process is ending, so a slow broker never delays terminal shutdown. | |||
| 33 | + | const closedDetachTimeout = 300 * time.Millisecond | |||
| 34 | + | ||||
| 35 | + | // ErrClientClosed is the cancellation cause composition uses when the client | |||
| 36 | + | // process is ending (SIGHUP/SIGTERM). The attachment then sends Detach{Closed} | |||
| 37 | + | // so the daemon can apply ephemeral.close-on-exit instead of parking. | |||
| 38 | + | var ErrClientClosed = errors.New("client: process closed") | |||
| 39 | + | ||||
| 30 | var ( | 40 | var ( | ||
| 31 | // errAttachmentDeadline is the stable local sentinel of an expired | 41 | // errAttachmentDeadline is the stable local sentinel of an expired | ||
| 32 | // attachment budget. The supervisor wraps it in a typed timeout. | 42 | // attachment budget. The supervisor wraps it in a typed timeout. | ||
| @@ -777,6 +787,9 @@ func (w *sessionAttachmentWorker) awaitServer(ctx context.Context, fg Attachment | |||||
| 777 | // application failure on the stream. | 787 | // application failure on the stream. | ||
| 778 | func (w *sessionAttachmentWorker) settle(ctx context.Context, fg AttachmentForeground, stream ports.BrokerLogicalConnection, token AttachmentToken, err error) AttachmentEvent { | 788 | func (w *sessionAttachmentWorker) settle(ctx context.Context, fg AttachmentForeground, stream ports.BrokerLogicalConnection, token AttachmentToken, err error) AttachmentEvent { | ||
| 779 | if ctxErr := ctx.Err(); ctxErr != nil { | 789 | if ctxErr := ctx.Err(); ctxErr != nil { | ||
| 790 | + | if errors.Is(context.Cause(ctx), ErrClientClosed) { | |||
| 791 | + | w.sendClosed(stream) | |||
| 792 | + | } | |||
| 780 | return AttachmentEvent{Token: token, Kind: AttachmentEventFailed, Err: errors.Join(ctxErr, err)} | 793 | return AttachmentEvent{Token: token, Kind: AttachmentEventFailed, Err: errors.Join(ctxErr, err)} | ||
| 781 | } | 794 | } | ||
| 782 | if cause := streamErr(stream); cause != nil { | 795 | if cause := streamErr(stream); cause != nil { | ||
| @@ -791,6 +804,23 @@ func (w *sessionAttachmentWorker) settle(ctx context.Context, fg AttachmentForeg | |||||
| 791 | return AttachmentEvent{Token: token, Kind: AttachmentEventEnded} | 804 | return AttachmentEvent{Token: token, Kind: AttachmentEventEnded} | ||
| 792 | } | 805 | } | ||
| 793 | 806 | ||||
| 807 | + | // sendClosed makes a best-effort, bounded attempt to tell the daemon that this | |||
| 808 | + | // client process is ending. It bypasses send's cancellation because the run | |||
| 809 | + | // context is already done; the daemon parks the attachment if it never arrives. | |||
| 810 | + | func (w *sessionAttachmentWorker) sendClosed(stream ports.BrokerLogicalConnection) { | |||
| 811 | + | if supervisorNil(stream) { | |||
| 812 | + | return | |||
| 813 | + | } | |||
| 814 | + | sent := make(chan error, 1) | |||
| 815 | + | go func() { sent <- stream.SendClient(protocol.Detach{Closed: true}) }() | |||
| 816 | + | timer := w.cfg.Clock.NewTimer(closedDetachTimeout) | |||
| 817 | + | defer stopSupervisorTimer(timer) | |||
| 818 | + | select { | |||
| 819 | + | case <-sent: | |||
| 820 | + | case <-timer.C(): | |||
| 821 | + | } | |||
| 822 | + | } | |||
| 823 | + | ||||
| 794 | // streamErr reads the logical stream's stable terminal cause without treating a | 824 | // streamErr reads the logical stream's stable terminal cause without treating a | ||
| 795 | // nil stream as a failure. | 825 | // nil stream as a failure. | ||
| 796 | func streamErr(stream ports.BrokerLogicalConnection) error { | 826 | func streamErr(stream ports.BrokerLogicalConnection) error { | ||
M internal/usecase/client/attachment_session_test.go +30 -0
| @@ -127,6 +127,36 @@ func TestSessionAttachmentSendCancellationClosesStreamAndJoinsSend(t *testing.T) | |||
| 127 | 127 | } | |
| 128 | 128 | } | |
| 129 | 129 | ||
| 130 | + | func TestSessionAttachmentSettleReportsClosedProcess(t *testing.T) { | |
| 131 | + | tests := []struct { | |
| 132 | + | name string | |
| 133 | + | cause error | |
| 134 | + | wantClosed bool | |
| 135 | + | }{ | |
| 136 | + | {name: "closed process sends closed detach", cause: ErrClientClosed, wantClosed: true}, | |
| 137 | + | {name: "plain cancellation sends nothing", cause: context.Canceled}, | |
| 138 | + | } | |
| 139 | + | for _, tt := range tests { | |
| 140 | + | t.Run(tt.name, func(t *testing.T) { | |
| 141 | + | stream := newSessionTestStream() | |
| 142 | + | ctx, cancel := context.WithCancelCause(context.Background()) | |
| 143 | + | cancel(tt.cause) | |
| 144 | + | w := &sessionAttachmentWorker{cfg: sessionAttachmentConfig{Clock: systemClock{}}} | |
| 145 | + | ||
| 146 | + | event := w.settle(ctx, &attachmentForeground{done: make(chan struct{})}, stream, AttachmentToken{}, nil) | |
| 147 | + | ||
| 148 | + | require.Equal(t, AttachmentEventFailed, event.Kind) | |
| 149 | + | stream.mu.Lock() | |
| 150 | + | defer stream.mu.Unlock() | |
| 151 | + | if tt.wantClosed { | |
| 152 | + | require.Equal(t, []protocol.ClientMessage{protocol.Detach{Closed: true}}, stream.sent) | |
| 153 | + | } else { | |
| 154 | + | require.Empty(t, stream.sent) | |
| 155 | + | } | |
| 156 | + | }) | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | ||
| 130 | 160 | func (s *sessionTestStream) deliver(message protocol.ServerMessage) { | |
| 131 | 161 | select { | |
| 132 | 162 | case s.incoming <- message: | |
| @@ -127,6 +127,36 @@ func TestSessionAttachmentSendCancellationClosesStreamAndJoinsSend(t *testing.T) | |||||
| 127 | } | 127 | } | ||
| 128 | } | 128 | } | ||
| 129 | 129 | ||||
| 130 | + | func TestSessionAttachmentSettleReportsClosedProcess(t *testing.T) { | |||
| 131 | + | tests := []struct { | |||
| 132 | + | name string | |||
| 133 | + | cause error | |||
| 134 | + | wantClosed bool | |||
| 135 | + | }{ | |||
| 136 | + | {name: "closed process sends closed detach", cause: ErrClientClosed, wantClosed: true}, | |||
| 137 | + | {name: "plain cancellation sends nothing", cause: context.Canceled}, | |||
| 138 | + | } | |||
| 139 | + | for _, tt := range tests { | |||
| 140 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 141 | + | stream := newSessionTestStream() | |||
| 142 | + | ctx, cancel := context.WithCancelCause(context.Background()) | |||
| 143 | + | cancel(tt.cause) | |||
| 144 | + | w := &sessionAttachmentWorker{cfg: sessionAttachmentConfig{Clock: systemClock{}}} | |||
| 145 | + | ||||
| 146 | + | event := w.settle(ctx, &attachmentForeground{done: make(chan struct{})}, stream, AttachmentToken{}, nil) | |||
| 147 | + | ||||
| 148 | + | require.Equal(t, AttachmentEventFailed, event.Kind) | |||
| 149 | + | stream.mu.Lock() | |||
| 150 | + | defer stream.mu.Unlock() | |||
| 151 | + | if tt.wantClosed { | |||
| 152 | + | require.Equal(t, []protocol.ClientMessage{protocol.Detach{Closed: true}}, stream.sent) | |||
| 153 | + | } else { | |||
| 154 | + | require.Empty(t, stream.sent) | |||
| 155 | + | } | |||
| 156 | + | }) | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | ||||
| 130 | func (s *sessionTestStream) deliver(message protocol.ServerMessage) { | 160 | func (s *sessionTestStream) deliver(message protocol.ServerMessage) { | ||
| 131 | select { | 161 | select { | ||
| 132 | case s.incoming <- message: | 162 | case s.incoming <- message: | ||
M internal/usecase/daemon/attachment_suspension.go +1 -0
| @@ -367,4 +367,5 @@ func (d *Daemon) expireSuspendedAttachment(record *suspendedAttachmentRetention) | |||
| 367 | 367 | // picker/palette/output cleanup, and the physical transport close all run | |
| 368 | 368 | // outside every architecture lock. Expiry never parks a resume credential. | |
| 369 | 369 | d.finishClientGone(sess, ac, record.transport.transport, true, false) | |
| 370 | + | d.reapAbandonedEphemeral(sess) | |
| 370 | 371 | } | |
| @@ -367,4 +367,5 @@ func (d *Daemon) expireSuspendedAttachment(record *suspendedAttachmentRetention) | |||||
| 367 | // picker/palette/output cleanup, and the physical transport close all run | 367 | // picker/palette/output cleanup, and the physical transport close all run | ||
| 368 | // outside every architecture lock. Expiry never parks a resume credential. | 368 | // outside every architecture lock. Expiry never parks a resume credential. | ||
| 369 | d.finishClientGone(sess, ac, record.transport.transport, true, false) | 369 | d.finishClientGone(sess, ac, record.transport.transport, true, false) | ||
| 370 | + | d.reapAbandonedEphemeral(sess) | |||
| 370 | } | 371 | } | ||
M internal/usecase/daemon/attachment_suspension_expiry_test.go +3 -1
| @@ -11,6 +11,7 @@ import ( | |||
| 11 | 11 | ||
| 12 | 12 | "github.com/stretchr/testify/require" | |
| 13 | 13 | ||
| 14 | + | "github.com/bnema/vev/internal/domain" | |
| 14 | 15 | "github.com/bnema/vev/internal/protocol" | |
| 15 | 16 | ) | |
| 16 | 17 | ||
| @@ -140,6 +141,7 @@ func TestSuspendedAttachmentSafetyExpiryEvictsAfterTimer(t *testing.T) { | |||
| 140 | 141 | ||
| 141 | 142 | func TestSuspendedAttachmentExpiryLeavesEphemeralSessionRegistered(t *testing.T) { | |
| 142 | 143 | d, sess, ac, timer, _ := newSuspendedExpiryFixture(t) | |
| 144 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: false}) | |
| 143 | 145 | sess.mu.Lock() | |
| 144 | 146 | sess.ephemeral = true | |
| 145 | 147 | sess.mu.Unlock() | |
| @@ -148,7 +150,7 @@ func TestSuspendedAttachmentExpiryLeavesEphemeralSessionRegistered(t *testing.T) | |||
| 148 | 150 | d.afterClientGoneDetach = func() { once.Do(func() { close(evicted) }) } | |
| 149 | 151 | timer.ch <- time.Now() | |
| 150 | 152 | awaitTestCompletion(t, evicted, "safety expiry never evicted the ephemeral attachment") | |
| 151 | - | require.Same(t, sess, firstSession(d), "ephemeral sessions follow existing lifetime policy") | |
| 153 | + | require.Same(t, sess, firstSession(d), "with close-on-exit off, ephemeral sessions follow headless lifetime policy") | |
| 152 | 154 | require.Nil(t, ac.currentAttachmentSession()) | |
| 153 | 155 | waitForAttachmentCleanup(t, d) | |
| 154 | 156 | } | |
| @@ -11,6 +11,7 @@ import ( | |||||
| 11 | 11 | ||||
| 12 | "github.com/stretchr/testify/require" | 12 | "github.com/stretchr/testify/require" | ||
| 13 | 13 | ||||
| 14 | + | "github.com/bnema/vev/internal/domain" | |||
| 14 | "github.com/bnema/vev/internal/protocol" | 15 | "github.com/bnema/vev/internal/protocol" | ||
| 15 | ) | 16 | ) | ||
| 16 | 17 | ||||
| @@ -140,6 +141,7 @@ func TestSuspendedAttachmentSafetyExpiryEvictsAfterTimer(t *testing.T) { | |||||
| 140 | 141 | ||||
| 141 | func TestSuspendedAttachmentExpiryLeavesEphemeralSessionRegistered(t *testing.T) { | 142 | func TestSuspendedAttachmentExpiryLeavesEphemeralSessionRegistered(t *testing.T) { | ||
| 142 | d, sess, ac, timer, _ := newSuspendedExpiryFixture(t) | 143 | d, sess, ac, timer, _ := newSuspendedExpiryFixture(t) | ||
| 144 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: false}) | |||
| 143 | sess.mu.Lock() | 145 | sess.mu.Lock() | ||
| 144 | sess.ephemeral = true | 146 | sess.ephemeral = true | ||
| 145 | sess.mu.Unlock() | 147 | sess.mu.Unlock() | ||
| @@ -148,7 +150,7 @@ func TestSuspendedAttachmentExpiryLeavesEphemeralSessionRegistered(t *testing.T) | |||||
| 148 | d.afterClientGoneDetach = func() { once.Do(func() { close(evicted) }) } | 150 | d.afterClientGoneDetach = func() { once.Do(func() { close(evicted) }) } | ||
| 149 | timer.ch <- time.Now() | 151 | timer.ch <- time.Now() | ||
| 150 | awaitTestCompletion(t, evicted, "safety expiry never evicted the ephemeral attachment") | 152 | awaitTestCompletion(t, evicted, "safety expiry never evicted the ephemeral attachment") | ||
| 151 | - | require.Same(t, sess, firstSession(d), "ephemeral sessions follow existing lifetime policy") | 153 | + | require.Same(t, sess, firstSession(d), "with close-on-exit off, ephemeral sessions follow headless lifetime policy") |
| 152 | require.Nil(t, ac.currentAttachmentSession()) | 154 | require.Nil(t, ac.currentAttachmentSession()) | ||
| 153 | waitForAttachmentCleanup(t, d) | 155 | waitForAttachmentCleanup(t, d) | ||
| 154 | } | 156 | } | ||
M internal/usecase/daemon/client_frame_routing.go +10 -2
| @@ -117,8 +117,13 @@ func (d *Daemon) handleAttachmentClientMessage(capability attachmentCapability, | |||
| 117 | 117 | return true | |
| 118 | 118 | } | |
| 119 | 119 | } | |
| 120 | - | if _, detach := message.(protocol.Detach); detach { | |
| 120 | + | if detach, ok := message.(protocol.Detach); ok { | |
| 121 | 121 | d.clientGoneWithoutNotice(capability.sess, capability.ac, capability.transport.transport, true) | |
| 122 | + | if detach.Closed { | |
| 123 | + | // Safe even if this call did not detach: the reap re-checks | |
| 124 | + | // emptiness under the architecture locks. | |
| 125 | + | d.reapAbandonedEphemeral(capability.sess) | |
| 126 | + | } | |
| 122 | 127 | return true | |
| 123 | 128 | } | |
| 124 | 129 | return false | |
| @@ -167,7 +172,10 @@ func (d *Daemon) handleActiveAttachmentClientMessage(capability attachmentCapabi | |||
| 167 | 172 | d.handleClientNoticeForAttachment(effect, message) | |
| 168 | 173 | case protocol.Detach: | |
| 169 | 174 | if effect.current() { | |
| 170 | - | d.clientGoneForAttachment(effect, true) | |
| 175 | + | sess := effect.capability().sess | |
| 176 | + | if d.clientGoneForAttachment(effect, true) && message.Closed { | |
| 177 | + | d.reapAbandonedEphemeral(sess) | |
| 178 | + | } | |
| 171 | 179 | return true | |
| 172 | 180 | } | |
| 173 | 181 | case protocol.Ack: | |
| @@ -117,8 +117,13 @@ func (d *Daemon) handleAttachmentClientMessage(capability attachmentCapability, | |||||
| 117 | return true | 117 | return true | ||
| 118 | } | 118 | } | ||
| 119 | } | 119 | } | ||
| 120 | - | if _, detach := message.(protocol.Detach); detach { | 120 | + | if detach, ok := message.(protocol.Detach); ok { |
| 121 | d.clientGoneWithoutNotice(capability.sess, capability.ac, capability.transport.transport, true) | 121 | d.clientGoneWithoutNotice(capability.sess, capability.ac, capability.transport.transport, true) | ||
| 122 | + | if detach.Closed { | |||
| 123 | + | // Safe even if this call did not detach: the reap re-checks | |||
| 124 | + | // emptiness under the architecture locks. | |||
| 125 | + | d.reapAbandonedEphemeral(capability.sess) | |||
| 126 | + | } | |||
| 122 | return true | 127 | return true | ||
| 123 | } | 128 | } | ||
| 124 | return false | 129 | return false | ||
| @@ -167,7 +172,10 @@ func (d *Daemon) handleActiveAttachmentClientMessage(capability attachmentCapabi | |||||
| 167 | d.handleClientNoticeForAttachment(effect, message) | 172 | d.handleClientNoticeForAttachment(effect, message) | ||
| 168 | case protocol.Detach: | 173 | case protocol.Detach: | ||
| 169 | if effect.current() { | 174 | if effect.current() { | ||
| 170 | - | d.clientGoneForAttachment(effect, true) | 175 | + | sess := effect.capability().sess |
| 176 | + | if d.clientGoneForAttachment(effect, true) && message.Closed { | |||
| 177 | + | d.reapAbandonedEphemeral(sess) | |||
| 178 | + | } | |||
| 171 | return true | 179 | return true | ||
| 172 | } | 180 | } | ||
| 173 | case protocol.Ack: | 181 | case protocol.Ack: | ||
M internal/usecase/daemon/config.go +9 -0
| @@ -66,6 +66,8 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||
| 66 | 66 | d.navConfig.Store(&nav) | |
| 67 | 67 | tabs := cfg.Tabs | |
| 68 | 68 | d.tabsConfig.Store(&tabs) | |
| 69 | + | ephemeral := cfg.Ephemeral | |
| 70 | + | d.ephemeralConfig.Store(&ephemeral) | |
| 69 | 71 | d.storeThemeConfig(cfg) | |
| 70 | 72 | d.applyHistoryLimits() | |
| 71 | 73 | barChanged := false | |
| @@ -285,6 +287,13 @@ func (d *Daemon) currentTabsConfig() domain.TabsConfig { | |||
| 285 | 287 | return domain.Defaults().Tabs | |
| 286 | 288 | } | |
| 287 | 289 | ||
| 290 | + | func (d *Daemon) currentEphemeralConfig() domain.EphemeralConfig { | |
| 291 | + | if cfg := d.ephemeralConfig.Load(); cfg != nil { | |
| 292 | + | return *cfg | |
| 293 | + | } | |
| 294 | + | return domain.Defaults().Ephemeral | |
| 295 | + | } | |
| 296 | + | ||
| 288 | 297 | func (d *Daemon) currentFloatingConfig() domain.FloatingConfig { | |
| 289 | 298 | if cfg := d.floatingConfig.Load(); cfg != nil { | |
| 290 | 299 | return *cfg | |
| @@ -66,6 +66,8 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||||
| 66 | d.navConfig.Store(&nav) | 66 | d.navConfig.Store(&nav) | ||
| 67 | tabs := cfg.Tabs | 67 | tabs := cfg.Tabs | ||
| 68 | d.tabsConfig.Store(&tabs) | 68 | d.tabsConfig.Store(&tabs) | ||
| 69 | + | ephemeral := cfg.Ephemeral | |||
| 70 | + | d.ephemeralConfig.Store(&ephemeral) | |||
| 69 | d.storeThemeConfig(cfg) | 71 | d.storeThemeConfig(cfg) | ||
| 70 | d.applyHistoryLimits() | 72 | d.applyHistoryLimits() | ||
| 71 | barChanged := false | 73 | barChanged := false | ||
| @@ -285,6 +287,13 @@ func (d *Daemon) currentTabsConfig() domain.TabsConfig { | |||||
| 285 | return domain.Defaults().Tabs | 287 | return domain.Defaults().Tabs | ||
| 286 | } | 288 | } | ||
| 287 | 289 | ||||
| 290 | + | func (d *Daemon) currentEphemeralConfig() domain.EphemeralConfig { | |||
| 291 | + | if cfg := d.ephemeralConfig.Load(); cfg != nil { | |||
| 292 | + | return *cfg | |||
| 293 | + | } | |||
| 294 | + | return domain.Defaults().Ephemeral | |||
| 295 | + | } | |||
| 296 | + | ||||
| 288 | func (d *Daemon) currentFloatingConfig() domain.FloatingConfig { | 297 | func (d *Daemon) currentFloatingConfig() domain.FloatingConfig { | ||
| 289 | if cfg := d.floatingConfig.Load(); cfg != nil { | 298 | if cfg := d.floatingConfig.Load(); cfg != nil { | ||
| 290 | return *cfg | 299 | return *cfg | ||
M internal/usecase/daemon/daemon.go +1 -0
| @@ -355,6 +355,7 @@ type Daemon struct { | |||
| 355 | 355 | paletteConfig atomic.Pointer[domain.PaletteConfig] | |
| 356 | 356 | navConfig atomic.Pointer[domain.NavConfig] | |
| 357 | 357 | tabsConfig atomic.Pointer[domain.TabsConfig] | |
| 358 | + | ephemeralConfig atomic.Pointer[domain.EphemeralConfig] | |
| 358 | 359 | scrollbackConfig atomic.Pointer[domain.ScrollbackConfig] | |
| 359 | 360 | themeConfig atomic.Pointer[themeConfigSnapshot] | |
| 360 | 361 | barScripts *barScriptState | |
| @@ -355,6 +355,7 @@ type Daemon struct { | |||||
| 355 | paletteConfig atomic.Pointer[domain.PaletteConfig] | 355 | paletteConfig atomic.Pointer[domain.PaletteConfig] | ||
| 356 | navConfig atomic.Pointer[domain.NavConfig] | 356 | navConfig atomic.Pointer[domain.NavConfig] | ||
| 357 | tabsConfig atomic.Pointer[domain.TabsConfig] | 357 | tabsConfig atomic.Pointer[domain.TabsConfig] | ||
| 358 | + | ephemeralConfig atomic.Pointer[domain.EphemeralConfig] | |||
| 358 | scrollbackConfig atomic.Pointer[domain.ScrollbackConfig] | 359 | scrollbackConfig atomic.Pointer[domain.ScrollbackConfig] | ||
| 359 | themeConfig atomic.Pointer[themeConfigSnapshot] | 360 | themeConfig atomic.Pointer[themeConfigSnapshot] | ||
| 360 | barScripts *barScriptState | 361 | barScripts *barScriptState | ||
A internal/usecase/daemon/ephemeral_close.go +49 -0
| @@ -0,0 +1,49 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "errors" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/vev/internal/protocol" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | // reapAbandonedEphemeral removes an ephemeral session whose last attachment | |
| 10 | + | // ended definitively (a closed client process, or an expired park or | |
| 11 | + | // suspension) when ephemeral.close-on-exit is on. A user detach or picker | |
| 12 | + | // switch never calls it: those keep the session for later reattach. | |
| 13 | + | // | |
| 14 | + | // The emptiness and ephemeral checks run under the architecture locks at | |
| 15 | + | // snapshot and publication, so any live, suspended, parked, or parking | |
| 16 | + | // attachment, or a concurrent rename into a named session, keeps it. | |
| 17 | + | // Callers must not hold daemon, session, or attachment locks. | |
| 18 | + | func (d *Daemon) reapAbandonedEphemeral(sess *session) { | |
| 19 | + | if sess == nil || !d.currentEphemeralConfig().CloseOnExit { | |
| 20 | + | return | |
| 21 | + | } | |
| 22 | + | err := d.killEphemeralSessionIfEmpty(sess, protocol.ReasonSessionKilled) | |
| 23 | + | switch { | |
| 24 | + | case errors.Is(err, errSessionKillParticipantsChanged): | |
| 25 | + | // An attachment or rename won the race: the session stays by design. | |
| 26 | + | d.log.Debug("ephemeral close-on-exit skipped", "session", sess.nameSnapshot()) | |
| 27 | + | case err != nil: | |
| 28 | + | d.log.Warn("ephemeral close-on-exit failed", "err", err, "session", sess.nameSnapshot()) | |
| 29 | + | case !d.sessionRegistered(sess): | |
| 30 | + | d.log.Info("ephemeral session gone after last attachment ended", "session", sess.nameSnapshot()) | |
| 31 | + | } | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | // sessionHasPendingResumeLocked reports whether sess still retains a parked or | |
| 35 | + | // in-flight parking attachment that a reconnecting client may resume. Caller | |
| 36 | + | // holds d.mu. | |
| 37 | + | func (d *Daemon) sessionHasPendingResumeLocked(sess *session) bool { | |
| 38 | + | for _, parked := range d.parked { | |
| 39 | + | if parked.sess == sess { | |
| 40 | + | return true | |
| 41 | + | } | |
| 42 | + | } | |
| 43 | + | for _, parking := range d.parking { | |
| 44 | + | if parking.sess == sess { | |
| 45 | + | return true | |
| 46 | + | } | |
| 47 | + | } | |
| 48 | + | return false | |
| 49 | + | } | |
| @@ -0,0 +1,49 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "errors" | |||
| 5 | + | ||||
| 6 | + | "github.com/bnema/vev/internal/protocol" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | // reapAbandonedEphemeral removes an ephemeral session whose last attachment | |||
| 10 | + | // ended definitively (a closed client process, or an expired park or | |||
| 11 | + | // suspension) when ephemeral.close-on-exit is on. A user detach or picker | |||
| 12 | + | // switch never calls it: those keep the session for later reattach. | |||
| 13 | + | // | |||
| 14 | + | // The emptiness and ephemeral checks run under the architecture locks at | |||
| 15 | + | // snapshot and publication, so any live, suspended, parked, or parking | |||
| 16 | + | // attachment, or a concurrent rename into a named session, keeps it. | |||
| 17 | + | // Callers must not hold daemon, session, or attachment locks. | |||
| 18 | + | func (d *Daemon) reapAbandonedEphemeral(sess *session) { | |||
| 19 | + | if sess == nil || !d.currentEphemeralConfig().CloseOnExit { | |||
| 20 | + | return | |||
| 21 | + | } | |||
| 22 | + | err := d.killEphemeralSessionIfEmpty(sess, protocol.ReasonSessionKilled) | |||
| 23 | + | switch { | |||
| 24 | + | case errors.Is(err, errSessionKillParticipantsChanged): | |||
| 25 | + | // An attachment or rename won the race: the session stays by design. | |||
| 26 | + | d.log.Debug("ephemeral close-on-exit skipped", "session", sess.nameSnapshot()) | |||
| 27 | + | case err != nil: | |||
| 28 | + | d.log.Warn("ephemeral close-on-exit failed", "err", err, "session", sess.nameSnapshot()) | |||
| 29 | + | case !d.sessionRegistered(sess): | |||
| 30 | + | d.log.Info("ephemeral session gone after last attachment ended", "session", sess.nameSnapshot()) | |||
| 31 | + | } | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | // sessionHasPendingResumeLocked reports whether sess still retains a parked or | |||
| 35 | + | // in-flight parking attachment that a reconnecting client may resume. Caller | |||
| 36 | + | // holds d.mu. | |||
| 37 | + | func (d *Daemon) sessionHasPendingResumeLocked(sess *session) bool { | |||
| 38 | + | for _, parked := range d.parked { | |||
| 39 | + | if parked.sess == sess { | |||
| 40 | + | return true | |||
| 41 | + | } | |||
| 42 | + | } | |||
| 43 | + | for _, parking := range d.parking { | |||
| 44 | + | if parking.sess == sess { | |||
| 45 | + | return true | |||
| 46 | + | } | |||
| 47 | + | } | |||
| 48 | + | return false | |||
| 49 | + | } | |||
A internal/usecase/daemon/ephemeral_close_test.go +146 -0
| @@ -0,0 +1,146 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/vev/internal/domain" | |
| 8 | + | "github.com/bnema/vev/internal/protocol" | |
| 9 | + | "github.com/stretchr/testify/require" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func TestEphemeralCloseOnExit(t *testing.T) { | |
| 13 | + | tests := []struct { | |
| 14 | + | name string | |
| 15 | + | closeOnExit bool | |
| 16 | + | intent uint8 | |
| 17 | + | sessionName string | |
| 18 | + | clients int | |
| 19 | + | renamed bool | |
| 20 | + | wantRemoved bool | |
| 21 | + | }{ | |
| 22 | + | {name: "last ephemeral client removes session", closeOnExit: true, intent: protocol.IntentEphemeral, clients: 1, wantRemoved: true}, | |
| 23 | + | {name: "option off keeps session", closeOnExit: false, intent: protocol.IntentEphemeral, clients: 1}, | |
| 24 | + | {name: "named session is never removed", closeOnExit: true, intent: protocol.IntentNew, sessionName: "work", clients: 1}, | |
| 25 | + | {name: "renamed session is never removed", closeOnExit: true, intent: protocol.IntentEphemeral, clients: 1, renamed: true}, | |
| 26 | + | {name: "remaining client keeps session", closeOnExit: true, intent: protocol.IntentEphemeral, clients: 2}, | |
| 27 | + | } | |
| 28 | + | for _, tt := range tests { | |
| 29 | + | t.Run(tt.name, func(t *testing.T) { | |
| 30 | + | pty, release := newBlockingPTY(t) | |
| 31 | + | defer release() | |
| 32 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 33 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: tt.closeOnExit}) | |
| 34 | + | ||
| 35 | + | tr := &closeTrackingTransport{} | |
| 36 | + | sess, ac, err := d.route(helloResumeCapable(tt.intent, tt.sessionName, 0), tr) | |
| 37 | + | require.NoError(t, err) | |
| 38 | + | for i := 1; i < tt.clients; i++ { | |
| 39 | + | _, _, err := d.route(helloResumeCapable(protocol.IntentAttach, sess.nameSnapshot(), 0), &closeTrackingTransport{}) | |
| 40 | + | require.NoError(t, err) | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | d.clientGone(sess, ac, tr, true) | |
| 44 | + | if tt.renamed { | |
| 45 | + | require.NoError(t, d.renameSession(sess, "kept")) | |
| 46 | + | } | |
| 47 | + | d.reapAbandonedEphemeral(sess) | |
| 48 | + | ||
| 49 | + | require.Equal(t, !tt.wantRemoved, d.sessionRegistered(sess)) | |
| 50 | + | if !tt.wantRemoved { | |
| 51 | + | require.NoError(t, d.killSession(sess, protocol.ReasonSessionKilled, false)) | |
| 52 | + | } | |
| 53 | + | d.sessWg.Wait() | |
| 54 | + | }) | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func TestEphemeralCloseOnExitWaitsForParkExpiry(t *testing.T) { | |
| 59 | + | tests := []struct { | |
| 60 | + | name string | |
| 61 | + | otherClient bool | |
| 62 | + | wantRemoved bool | |
| 63 | + | }{ | |
| 64 | + | {name: "expired park removes abandoned session", wantRemoved: true}, | |
| 65 | + | {name: "expired park keeps session with another client", otherClient: true}, | |
| 66 | + | } | |
| 67 | + | for _, tt := range tests { | |
| 68 | + | t.Run(tt.name, func(t *testing.T) { | |
| 69 | + | clock := &signalClock{timers: make(chan *signalTimer, 8)} | |
| 70 | + | pty, release := newBlockingPTY(t) | |
| 71 | + | defer release() | |
| 72 | + | d := newTestDaemon(t, newFactory(t, pty), clock) | |
| 73 | + | ||
| 74 | + | tr := &closeTrackingTransport{} | |
| 75 | + | sess, ac, err := d.route(helloResumeCapable(protocol.IntentEphemeral, "", 0), tr) | |
| 76 | + | require.NoError(t, err) | |
| 77 | + | if tt.otherClient { | |
| 78 | + | _, _, err := d.route(helloResumeCapable(protocol.IntentAttach, sess.nameSnapshot(), 0), &closeTrackingTransport{}) | |
| 79 | + | require.NoError(t, err) | |
| 80 | + | } | |
| 81 | + | token := ac.resumeToken | |
| 82 | + | ||
| 83 | + | // A link loss parks: the session must survive so the client can resume. | |
| 84 | + | d.clientGone(sess, ac, tr, false) | |
| 85 | + | <-clock.timers | |
| 86 | + | d.reapAbandonedEphemeral(sess) | |
| 87 | + | require.True(t, d.sessionRegistered(sess), "parked attachment keeps the ephemeral session") | |
| 88 | + | ||
| 89 | + | d.mu.Lock() | |
| 90 | + | parked := d.parked[token] | |
| 91 | + | d.mu.Unlock() | |
| 92 | + | require.NotNil(t, parked) | |
| 93 | + | // Expire directly instead of firing the timer, so no watcher | |
| 94 | + | // goroutine races the assertion below. | |
| 95 | + | d.expireParked(token, parked) | |
| 96 | + | ||
| 97 | + | require.Equal(t, !tt.wantRemoved, d.sessionRegistered(sess)) | |
| 98 | + | if !tt.wantRemoved { | |
| 99 | + | require.NoError(t, d.killSession(sess, protocol.ReasonSessionKilled, false)) | |
| 100 | + | } | |
| 101 | + | d.sessWg.Wait() | |
| 102 | + | }) | |
| 103 | + | } | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | func TestDetachFrameEphemeralCloseOnExit(t *testing.T) { | |
| 107 | + | tests := []struct { | |
| 108 | + | name string | |
| 109 | + | closeOnExit bool | |
| 110 | + | intent uint8 | |
| 111 | + | sessionName string | |
| 112 | + | detach protocol.Detach | |
| 113 | + | wantRemoved bool | |
| 114 | + | }{ | |
| 115 | + | {name: "closed client removes session", closeOnExit: true, intent: protocol.IntentEphemeral, detach: protocol.Detach{Closed: true}, wantRemoved: true}, | |
| 116 | + | {name: "user detach keeps session", closeOnExit: true, intent: protocol.IntentEphemeral, detach: protocol.Detach{}}, | |
| 117 | + | {name: "option off keeps session", closeOnExit: false, intent: protocol.IntentEphemeral, detach: protocol.Detach{Closed: true}}, | |
| 118 | + | {name: "named session is kept", closeOnExit: true, intent: protocol.IntentNew, sessionName: "work", detach: protocol.Detach{Closed: true}}, | |
| 119 | + | } | |
| 120 | + | for _, tt := range tests { | |
| 121 | + | t.Run(tt.name, func(t *testing.T) { | |
| 122 | + | pty, release := newBlockingPTY(t) | |
| 123 | + | defer release() | |
| 124 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 125 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: tt.closeOnExit}) | |
| 126 | + | tr, sends, _ := newConn(t, | |
| 127 | + | mustHello(tt.intent, tt.sessionName, domain.Size{Cols: 80, Rows: 24}), | |
| 128 | + | mustClientEnvelope(tt.detach), | |
| 129 | + | ) | |
| 130 | + | ||
| 131 | + | var hg sync.WaitGroup | |
| 132 | + | hg.Go(func() { d.handleConn(tr) }) | |
| 133 | + | awaitFrame(t, sends, "Welcome") | |
| 134 | + | hg.Wait() | |
| 135 | + | ||
| 136 | + | if tt.wantRemoved { | |
| 137 | + | require.Equal(t, 0, sessionCount(d)) | |
| 138 | + | } else { | |
| 139 | + | require.Equal(t, 1, sessionCount(d)) | |
| 140 | + | require.NoError(t, d.killSession(firstSession(d), protocol.ReasonSessionKilled, false)) | |
| 141 | + | } | |
| 142 | + | d.sessWg.Wait() | |
| 143 | + | d.waitNotifies() | |
| 144 | + | }) | |
| 145 | + | } | |
| 146 | + | } | |
| @@ -0,0 +1,146 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/bnema/vev/internal/domain" | |||
| 8 | + | "github.com/bnema/vev/internal/protocol" | |||
| 9 | + | "github.com/stretchr/testify/require" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | func TestEphemeralCloseOnExit(t *testing.T) { | |||
| 13 | + | tests := []struct { | |||
| 14 | + | name string | |||
| 15 | + | closeOnExit bool | |||
| 16 | + | intent uint8 | |||
| 17 | + | sessionName string | |||
| 18 | + | clients int | |||
| 19 | + | renamed bool | |||
| 20 | + | wantRemoved bool | |||
| 21 | + | }{ | |||
| 22 | + | {name: "last ephemeral client removes session", closeOnExit: true, intent: protocol.IntentEphemeral, clients: 1, wantRemoved: true}, | |||
| 23 | + | {name: "option off keeps session", closeOnExit: false, intent: protocol.IntentEphemeral, clients: 1}, | |||
| 24 | + | {name: "named session is never removed", closeOnExit: true, intent: protocol.IntentNew, sessionName: "work", clients: 1}, | |||
| 25 | + | {name: "renamed session is never removed", closeOnExit: true, intent: protocol.IntentEphemeral, clients: 1, renamed: true}, | |||
| 26 | + | {name: "remaining client keeps session", closeOnExit: true, intent: protocol.IntentEphemeral, clients: 2}, | |||
| 27 | + | } | |||
| 28 | + | for _, tt := range tests { | |||
| 29 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 30 | + | pty, release := newBlockingPTY(t) | |||
| 31 | + | defer release() | |||
| 32 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 33 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: tt.closeOnExit}) | |||
| 34 | + | ||||
| 35 | + | tr := &closeTrackingTransport{} | |||
| 36 | + | sess, ac, err := d.route(helloResumeCapable(tt.intent, tt.sessionName, 0), tr) | |||
| 37 | + | require.NoError(t, err) | |||
| 38 | + | for i := 1; i < tt.clients; i++ { | |||
| 39 | + | _, _, err := d.route(helloResumeCapable(protocol.IntentAttach, sess.nameSnapshot(), 0), &closeTrackingTransport{}) | |||
| 40 | + | require.NoError(t, err) | |||
| 41 | + | } | |||
| 42 | + | ||||
| 43 | + | d.clientGone(sess, ac, tr, true) | |||
| 44 | + | if tt.renamed { | |||
| 45 | + | require.NoError(t, d.renameSession(sess, "kept")) | |||
| 46 | + | } | |||
| 47 | + | d.reapAbandonedEphemeral(sess) | |||
| 48 | + | ||||
| 49 | + | require.Equal(t, !tt.wantRemoved, d.sessionRegistered(sess)) | |||
| 50 | + | if !tt.wantRemoved { | |||
| 51 | + | require.NoError(t, d.killSession(sess, protocol.ReasonSessionKilled, false)) | |||
| 52 | + | } | |||
| 53 | + | d.sessWg.Wait() | |||
| 54 | + | }) | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | func TestEphemeralCloseOnExitWaitsForParkExpiry(t *testing.T) { | |||
| 59 | + | tests := []struct { | |||
| 60 | + | name string | |||
| 61 | + | otherClient bool | |||
| 62 | + | wantRemoved bool | |||
| 63 | + | }{ | |||
| 64 | + | {name: "expired park removes abandoned session", wantRemoved: true}, | |||
| 65 | + | {name: "expired park keeps session with another client", otherClient: true}, | |||
| 66 | + | } | |||
| 67 | + | for _, tt := range tests { | |||
| 68 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 69 | + | clock := &signalClock{timers: make(chan *signalTimer, 8)} | |||
| 70 | + | pty, release := newBlockingPTY(t) | |||
| 71 | + | defer release() | |||
| 72 | + | d := newTestDaemon(t, newFactory(t, pty), clock) | |||
| 73 | + | ||||
| 74 | + | tr := &closeTrackingTransport{} | |||
| 75 | + | sess, ac, err := d.route(helloResumeCapable(protocol.IntentEphemeral, "", 0), tr) | |||
| 76 | + | require.NoError(t, err) | |||
| 77 | + | if tt.otherClient { | |||
| 78 | + | _, _, err := d.route(helloResumeCapable(protocol.IntentAttach, sess.nameSnapshot(), 0), &closeTrackingTransport{}) | |||
| 79 | + | require.NoError(t, err) | |||
| 80 | + | } | |||
| 81 | + | token := ac.resumeToken | |||
| 82 | + | ||||
| 83 | + | // A link loss parks: the session must survive so the client can resume. | |||
| 84 | + | d.clientGone(sess, ac, tr, false) | |||
| 85 | + | <-clock.timers | |||
| 86 | + | d.reapAbandonedEphemeral(sess) | |||
| 87 | + | require.True(t, d.sessionRegistered(sess), "parked attachment keeps the ephemeral session") | |||
| 88 | + | ||||
| 89 | + | d.mu.Lock() | |||
| 90 | + | parked := d.parked[token] | |||
| 91 | + | d.mu.Unlock() | |||
| 92 | + | require.NotNil(t, parked) | |||
| 93 | + | // Expire directly instead of firing the timer, so no watcher | |||
| 94 | + | // goroutine races the assertion below. | |||
| 95 | + | d.expireParked(token, parked) | |||
| 96 | + | ||||
| 97 | + | require.Equal(t, !tt.wantRemoved, d.sessionRegistered(sess)) | |||
| 98 | + | if !tt.wantRemoved { | |||
| 99 | + | require.NoError(t, d.killSession(sess, protocol.ReasonSessionKilled, false)) | |||
| 100 | + | } | |||
| 101 | + | d.sessWg.Wait() | |||
| 102 | + | }) | |||
| 103 | + | } | |||
| 104 | + | } | |||
| 105 | + | ||||
| 106 | + | func TestDetachFrameEphemeralCloseOnExit(t *testing.T) { | |||
| 107 | + | tests := []struct { | |||
| 108 | + | name string | |||
| 109 | + | closeOnExit bool | |||
| 110 | + | intent uint8 | |||
| 111 | + | sessionName string | |||
| 112 | + | detach protocol.Detach | |||
| 113 | + | wantRemoved bool | |||
| 114 | + | }{ | |||
| 115 | + | {name: "closed client removes session", closeOnExit: true, intent: protocol.IntentEphemeral, detach: protocol.Detach{Closed: true}, wantRemoved: true}, | |||
| 116 | + | {name: "user detach keeps session", closeOnExit: true, intent: protocol.IntentEphemeral, detach: protocol.Detach{}}, | |||
| 117 | + | {name: "option off keeps session", closeOnExit: false, intent: protocol.IntentEphemeral, detach: protocol.Detach{Closed: true}}, | |||
| 118 | + | {name: "named session is kept", closeOnExit: true, intent: protocol.IntentNew, sessionName: "work", detach: protocol.Detach{Closed: true}}, | |||
| 119 | + | } | |||
| 120 | + | for _, tt := range tests { | |||
| 121 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 122 | + | pty, release := newBlockingPTY(t) | |||
| 123 | + | defer release() | |||
| 124 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 125 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: tt.closeOnExit}) | |||
| 126 | + | tr, sends, _ := newConn(t, | |||
| 127 | + | mustHello(tt.intent, tt.sessionName, domain.Size{Cols: 80, Rows: 24}), | |||
| 128 | + | mustClientEnvelope(tt.detach), | |||
| 129 | + | ) | |||
| 130 | + | ||||
| 131 | + | var hg sync.WaitGroup | |||
| 132 | + | hg.Go(func() { d.handleConn(tr) }) | |||
| 133 | + | awaitFrame(t, sends, "Welcome") | |||
| 134 | + | hg.Wait() | |||
| 135 | + | ||||
| 136 | + | if tt.wantRemoved { | |||
| 137 | + | require.Equal(t, 0, sessionCount(d)) | |||
| 138 | + | } else { | |||
| 139 | + | require.Equal(t, 1, sessionCount(d)) | |||
| 140 | + | require.NoError(t, d.killSession(firstSession(d), protocol.ReasonSessionKilled, false)) | |||
| 141 | + | } | |||
| 142 | + | d.sessWg.Wait() | |||
| 143 | + | d.waitNotifies() | |||
| 144 | + | }) | |||
| 145 | + | } | |||
| 146 | + | } | |||
M internal/usecase/daemon/resume.go +1 -0
| @@ -300,6 +300,7 @@ func (d *Daemon) expireParked(token uint64, parked *parkedAttachment) { | |||
| 300 | 300 | d.mu.Unlock() | |
| 301 | 301 | d.closePaletteIfCurrent(parked.ac, parked.paletteGeneration) | |
| 302 | 302 | d.log.Warn("parked client expired", "session", parked.sess.nameSnapshot()) | |
| 303 | + | d.reapAbandonedEphemeral(parked.sess) | |
| 303 | 304 | return | |
| 304 | 305 | } | |
| 305 | 306 | d.mu.Unlock() | |
| @@ -300,6 +300,7 @@ func (d *Daemon) expireParked(token uint64, parked *parkedAttachment) { | |||||
| 300 | d.mu.Unlock() | 300 | d.mu.Unlock() | ||
| 301 | d.closePaletteIfCurrent(parked.ac, parked.paletteGeneration) | 301 | d.closePaletteIfCurrent(parked.ac, parked.paletteGeneration) | ||
| 302 | d.log.Warn("parked client expired", "session", parked.sess.nameSnapshot()) | 302 | d.log.Warn("parked client expired", "session", parked.sess.nameSnapshot()) | ||
| 303 | + | d.reapAbandonedEphemeral(parked.sess) | |||
| 303 | return | 304 | return | ||
| 304 | } | 305 | } | ||
| 305 | d.mu.Unlock() | 306 | d.mu.Unlock() | ||
M internal/usecase/daemon/resume_test.go +2 -1
| @@ -1390,11 +1390,12 @@ func TestDiscardingParkedAttachmentDoesNotOwnPreviousSession(t *testing.T) { | |||
| 1390 | 1390 | } | |
| 1391 | 1391 | } | |
| 1392 | 1392 | ||
| 1393 | - | func TestEphemeralParkExpiryKeepsSession(t *testing.T) { | |
| 1393 | + | func TestEphemeralParkExpiryKeepsSessionWhenCloseOnExitOff(t *testing.T) { | |
| 1394 | 1394 | clk := &signalClock{timers: make(chan *signalTimer, 8)} | |
| 1395 | 1395 | pty, release := newBlockingPTY(t) | |
| 1396 | 1396 | defer release() | |
| 1397 | 1397 | d := newTestDaemon(t, newFactory(t, pty), clk) | |
| 1398 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: false}) | |
| 1398 | 1399 | ||
| 1399 | 1400 | tr := &closeTrackingTransport{} | |
| 1400 | 1401 | sess, ac, err := d.route(helloResumeCapable(protocol.IntentEphemeral, "", 0), tr) | |
| @@ -1390,11 +1390,12 @@ func TestDiscardingParkedAttachmentDoesNotOwnPreviousSession(t *testing.T) { | |||||
| 1390 | } | 1390 | } | ||
| 1391 | } | 1391 | } | ||
| 1392 | 1392 | ||||
| 1393 | - | func TestEphemeralParkExpiryKeepsSession(t *testing.T) { | 1393 | + | func TestEphemeralParkExpiryKeepsSessionWhenCloseOnExitOff(t *testing.T) { |
| 1394 | clk := &signalClock{timers: make(chan *signalTimer, 8)} | 1394 | clk := &signalClock{timers: make(chan *signalTimer, 8)} | ||
| 1395 | pty, release := newBlockingPTY(t) | 1395 | pty, release := newBlockingPTY(t) | ||
| 1396 | defer release() | 1396 | defer release() | ||
| 1397 | d := newTestDaemon(t, newFactory(t, pty), clk) | 1397 | d := newTestDaemon(t, newFactory(t, pty), clk) | ||
| 1398 | + | d.ephemeralConfig.Store(&domain.EphemeralConfig{CloseOnExit: false}) | |||
| 1398 | 1399 | ||||
| 1399 | tr := &closeTrackingTransport{} | 1400 | tr := &closeTrackingTransport{} | ||
| 1400 | sess, ac, err := d.route(helloResumeCapable(protocol.IntentEphemeral, "", 0), tr) | 1401 | sess, ac, err := d.route(helloResumeCapable(protocol.IntentEphemeral, "", 0), tr) | ||
M internal/usecase/daemon/session.go +31 -9
| @@ -1363,6 +1363,9 @@ type sessionKillParticipants struct { | |||
| 1363 | 1363 | source *session | |
| 1364 | 1364 | sourceToken *attachmentCapability | |
| 1365 | 1365 | target *session | |
| 1366 | + | // requireEphemeral aborts the kill if the target stopped being ephemeral | |
| 1367 | + | // (a concurrent rename) between the snapshot and publication. | |
| 1368 | + | requireEphemeral bool | |
| 1366 | 1369 | } | |
| 1367 | 1370 | ||
| 1368 | 1371 | func (d *Daemon) killSessionForAttachment(sess *session, reason uint8, purge bool, effect *attachmentEffect, action string) error { | |
| @@ -1376,8 +1379,8 @@ func (d *Daemon) killSessionForAttachment(sess *session, reason uint8, purge boo | |||
| 1376 | 1379 | // architecture locks, but releases every lock before any attachment gate is frozen or | |
| 1377 | 1380 | // drained. The initiator is part of the same deduplicated global gate set even | |
| 1378 | 1381 | // when it belongs to a different source session. | |
| 1379 | - | func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *sessionKillAdmission, requireEmpty bool) (sessionKillParticipants, bool) { | |
| 1380 | - | snapshot := sessionKillParticipants{target: target} | |
| 1382 | + | func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *sessionKillAdmission, condition sessionKillCondition) (sessionKillParticipants, bool) { | |
| 1383 | + | snapshot := sessionKillParticipants{target: target, requireEphemeral: condition.ephemeral} | |
| 1381 | 1384 | if admission != nil { | |
| 1382 | 1385 | token := admission.capability | |
| 1383 | 1386 | if token.sess == nil || token.ac == nil || token.transport.transport == nil { | |
| @@ -1401,7 +1404,8 @@ func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *ses | |||
| 1401 | 1404 | source = target | |
| 1402 | 1405 | } | |
| 1403 | 1406 | unlockSessions := lockAttachmentSessions(source, target) | |
| 1404 | - | if requireEmpty && len(target.attachments) != 0 { | |
| 1407 | + | if condition.empty && (len(target.attachments) != 0 || d.sessionHasPendingResumeLocked(target)) || | |
| 1408 | + | condition.ephemeral && !target.ephemeral { | |
| 1405 | 1409 | unlockSessions() | |
| 1406 | 1410 | d.notices.routingMu.Unlock() | |
| 1407 | 1411 | d.mu.Unlock() | |
| @@ -1483,6 +1487,9 @@ func (d *Daemon) sessionKillParticipantsCurrent(snapshot sessionKillParticipants | |||
| 1483 | 1487 | // session and coordinator locks. | |
| 1484 | 1488 | func sessionKillParticipantsCurrentLocked(snapshot sessionKillParticipants, sourceCoordinator *renderCoordinator) bool { | |
| 1485 | 1489 | target := snapshot.target | |
| 1490 | + | if snapshot.requireEphemeral && !target.ephemeral { | |
| 1491 | + | return false | |
| 1492 | + | } | |
| 1486 | 1493 | if len(target.attachments) != len(snapshot.attachments) { | |
| 1487 | 1494 | return false | |
| 1488 | 1495 | } | |
| @@ -1683,24 +1690,39 @@ func teardownDeadlineElapsed(deadline *snapshotShutdownDeadline) bool { | |||
| 1683 | 1690 | // mistaken for a completed one. Ordinary callers that pass nil preserve their | |
| 1684 | 1691 | // previous behavior exactly. | |
| 1685 | 1692 | func (d *Daemon) killSessionWithSnapshotDeadline(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission) error { | |
| 1686 | - | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, deadline, admission, false) | |
| 1693 | + | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, deadline, admission, sessionKillCondition{}) | |
| 1694 | + | } | |
| 1695 | + | ||
| 1696 | + | // sessionKillCondition narrows a kill to a target state that is checked under | |
| 1697 | + | // the architecture locks at snapshot and again at publication. | |
| 1698 | + | type sessionKillCondition struct { | |
| 1699 | + | // empty requires no attachment and no parked or parking resume credential. | |
| 1700 | + | empty bool | |
| 1701 | + | // ephemeral requires the target to still be an ephemeral session. | |
| 1702 | + | ephemeral bool | |
| 1687 | 1703 | } | |
| 1688 | 1704 | ||
| 1689 | 1705 | // killSessionIfEmpty tears down sess only while its exact registry entry has no | |
| 1690 | - | // attachments. Membership is captured under the normal architecture locks and | |
| 1691 | - | // fenced again at publication, so an attachment winning either side survives. | |
| 1706 | + | // attachments and no parked or parking resume credential. Membership is | |
| 1707 | + | // captured under the normal architecture locks and fenced again at publication, so an attachment winning either side survives. | |
| 1692 | 1708 | func (d *Daemon) killSessionIfEmpty(sess *session, reason uint8, purge bool) error { | |
| 1693 | - | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, nil, nil, true) | |
| 1709 | + | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, nil, nil, sessionKillCondition{empty: true}) | |
| 1710 | + | } | |
| 1711 | + | ||
| 1712 | + | // killEphemeralSessionIfEmpty is killSessionIfEmpty that also refuses a target | |
| 1713 | + | // renamed into a named session after the caller's check. | |
| 1714 | + | func (d *Daemon) killEphemeralSessionIfEmpty(sess *session, reason uint8) error { | |
| 1715 | + | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, false, nil, nil, sessionKillCondition{empty: true, ephemeral: true}) | |
| 1694 | 1716 | } | |
| 1695 | 1717 | ||
| 1696 | - | func (d *Daemon) killSessionWithSnapshotDeadlineAndCondition(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission, requireEmpty bool) error { | |
| 1718 | + | func (d *Daemon) killSessionWithSnapshotDeadlineAndCondition(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission, condition sessionKillCondition) error { | |
| 1697 | 1719 | if sess == nil { | |
| 1698 | 1720 | return nil | |
| 1699 | 1721 | } | |
| 1700 | 1722 | // Discover every attachment whose membership can be invalidated before freezing | |
| 1701 | 1723 | // any one of them. The common gate helper deduplicates and freezes the whole | |
| 1702 | 1724 | // immutable identity set in the same global order used by transitions. | |
| 1703 | - | participants, ok := d.snapshotSessionKillParticipants(sess, admission, requireEmpty) | |
| 1725 | + | participants, ok := d.snapshotSessionKillParticipants(sess, admission, condition) | |
| 1704 | 1726 | if !ok { | |
| 1705 | 1727 | return nil | |
| 1706 | 1728 | } | |
| @@ -1363,6 +1363,9 @@ type sessionKillParticipants struct { | |||||
| 1363 | source *session | 1363 | source *session | ||
| 1364 | sourceToken *attachmentCapability | 1364 | sourceToken *attachmentCapability | ||
| 1365 | target *session | 1365 | target *session | ||
| 1366 | + | // requireEphemeral aborts the kill if the target stopped being ephemeral | |||
| 1367 | + | // (a concurrent rename) between the snapshot and publication. | |||
| 1368 | + | requireEphemeral bool | |||
| 1366 | } | 1369 | } | ||
| 1367 | 1370 | ||||
| 1368 | func (d *Daemon) killSessionForAttachment(sess *session, reason uint8, purge bool, effect *attachmentEffect, action string) error { | 1371 | func (d *Daemon) killSessionForAttachment(sess *session, reason uint8, purge bool, effect *attachmentEffect, action string) error { | ||
| @@ -1376,8 +1379,8 @@ func (d *Daemon) killSessionForAttachment(sess *session, reason uint8, purge boo | |||||
| 1376 | // architecture locks, but releases every lock before any attachment gate is frozen or | 1379 | // architecture locks, but releases every lock before any attachment gate is frozen or | ||
| 1377 | // drained. The initiator is part of the same deduplicated global gate set even | 1380 | // drained. The initiator is part of the same deduplicated global gate set even | ||
| 1378 | // when it belongs to a different source session. | 1381 | // when it belongs to a different source session. | ||
| 1379 | - | func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *sessionKillAdmission, requireEmpty bool) (sessionKillParticipants, bool) { | 1382 | + | func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *sessionKillAdmission, condition sessionKillCondition) (sessionKillParticipants, bool) { |
| 1380 | - | snapshot := sessionKillParticipants{target: target} | 1383 | + | snapshot := sessionKillParticipants{target: target, requireEphemeral: condition.ephemeral} |
| 1381 | if admission != nil { | 1384 | if admission != nil { | ||
| 1382 | token := admission.capability | 1385 | token := admission.capability | ||
| 1383 | if token.sess == nil || token.ac == nil || token.transport.transport == nil { | 1386 | if token.sess == nil || token.ac == nil || token.transport.transport == nil { | ||
| @@ -1401,7 +1404,8 @@ func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *ses | |||||
| 1401 | source = target | 1404 | source = target | ||
| 1402 | } | 1405 | } | ||
| 1403 | unlockSessions := lockAttachmentSessions(source, target) | 1406 | unlockSessions := lockAttachmentSessions(source, target) | ||
| 1404 | - | if requireEmpty && len(target.attachments) != 0 { | 1407 | + | if condition.empty && (len(target.attachments) != 0 || d.sessionHasPendingResumeLocked(target)) || |
| 1408 | + | condition.ephemeral && !target.ephemeral { | |||
| 1405 | unlockSessions() | 1409 | unlockSessions() | ||
| 1406 | d.notices.routingMu.Unlock() | 1410 | d.notices.routingMu.Unlock() | ||
| 1407 | d.mu.Unlock() | 1411 | d.mu.Unlock() | ||
| @@ -1483,6 +1487,9 @@ func (d *Daemon) sessionKillParticipantsCurrent(snapshot sessionKillParticipants | |||||
| 1483 | // session and coordinator locks. | 1487 | // session and coordinator locks. | ||
| 1484 | func sessionKillParticipantsCurrentLocked(snapshot sessionKillParticipants, sourceCoordinator *renderCoordinator) bool { | 1488 | func sessionKillParticipantsCurrentLocked(snapshot sessionKillParticipants, sourceCoordinator *renderCoordinator) bool { | ||
| 1485 | target := snapshot.target | 1489 | target := snapshot.target | ||
| 1490 | + | if snapshot.requireEphemeral && !target.ephemeral { | |||
| 1491 | + | return false | |||
| 1492 | + | } | |||
| 1486 | if len(target.attachments) != len(snapshot.attachments) { | 1493 | if len(target.attachments) != len(snapshot.attachments) { | ||
| 1487 | return false | 1494 | return false | ||
| 1488 | } | 1495 | } | ||
| @@ -1683,24 +1690,39 @@ func teardownDeadlineElapsed(deadline *snapshotShutdownDeadline) bool { | |||||
| 1683 | // mistaken for a completed one. Ordinary callers that pass nil preserve their | 1690 | // mistaken for a completed one. Ordinary callers that pass nil preserve their | ||
| 1684 | // previous behavior exactly. | 1691 | // previous behavior exactly. | ||
| 1685 | func (d *Daemon) killSessionWithSnapshotDeadline(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission) error { | 1692 | func (d *Daemon) killSessionWithSnapshotDeadline(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission) error { | ||
| 1686 | - | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, deadline, admission, false) | 1693 | + | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, deadline, admission, sessionKillCondition{}) |
| 1694 | + | } | |||
| 1695 | + | ||||
| 1696 | + | // sessionKillCondition narrows a kill to a target state that is checked under | |||
| 1697 | + | // the architecture locks at snapshot and again at publication. | |||
| 1698 | + | type sessionKillCondition struct { | |||
| 1699 | + | // empty requires no attachment and no parked or parking resume credential. | |||
| 1700 | + | empty bool | |||
| 1701 | + | // ephemeral requires the target to still be an ephemeral session. | |||
| 1702 | + | ephemeral bool | |||
| 1687 | } | 1703 | } | ||
| 1688 | 1704 | ||||
| 1689 | // killSessionIfEmpty tears down sess only while its exact registry entry has no | 1705 | // killSessionIfEmpty tears down sess only while its exact registry entry has no | ||
| 1690 | - | // attachments. Membership is captured under the normal architecture locks and | 1706 | + | // attachments and no parked or parking resume credential. Membership is |
| 1691 | - | // fenced again at publication, so an attachment winning either side survives. | 1707 | + | // captured under the normal architecture locks and fenced again at publication, so an attachment winning either side survives. |
| 1692 | func (d *Daemon) killSessionIfEmpty(sess *session, reason uint8, purge bool) error { | 1708 | func (d *Daemon) killSessionIfEmpty(sess *session, reason uint8, purge bool) error { | ||
| 1693 | - | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, nil, nil, true) | 1709 | + | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, purge, nil, nil, sessionKillCondition{empty: true}) |
| 1710 | + | } | |||
| 1711 | + | ||||
| 1712 | + | // killEphemeralSessionIfEmpty is killSessionIfEmpty that also refuses a target | |||
| 1713 | + | // renamed into a named session after the caller's check. | |||
| 1714 | + | func (d *Daemon) killEphemeralSessionIfEmpty(sess *session, reason uint8) error { | |||
| 1715 | + | return d.killSessionWithSnapshotDeadlineAndCondition(sess, reason, false, nil, nil, sessionKillCondition{empty: true, ephemeral: true}) | |||
| 1694 | } | 1716 | } | ||
| 1695 | 1717 | ||||
| 1696 | - | func (d *Daemon) killSessionWithSnapshotDeadlineAndCondition(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission, requireEmpty bool) error { | 1718 | + | func (d *Daemon) killSessionWithSnapshotDeadlineAndCondition(sess *session, reason uint8, purge bool, deadline *snapshotShutdownDeadline, admission *sessionKillAdmission, condition sessionKillCondition) error { |
| 1697 | if sess == nil { | 1719 | if sess == nil { | ||
| 1698 | return nil | 1720 | return nil | ||
| 1699 | } | 1721 | } | ||
| 1700 | // Discover every attachment whose membership can be invalidated before freezing | 1722 | // Discover every attachment whose membership can be invalidated before freezing | ||
| 1701 | // any one of them. The common gate helper deduplicates and freezes the whole | 1723 | // any one of them. The common gate helper deduplicates and freezes the whole | ||
| 1702 | // immutable identity set in the same global order used by transitions. | 1724 | // immutable identity set in the same global order used by transitions. | ||
| 1703 | - | participants, ok := d.snapshotSessionKillParticipants(sess, admission, requireEmpty) | 1725 | + | participants, ok := d.snapshotSessionKillParticipants(sess, admission, condition) |
| 1704 | if !ok { | 1726 | if !ok { | ||
| 1705 | return nil | 1727 | return nil | ||
| 1706 | } | 1728 | } | ||