fix(render): release captures before parking client
- commit
- c58a7c1ec6b0c8995a23fc80ddb2a3a458a91896
- author
- brice · · 2026-07-13 06:42 UTC
- parent
- fd5be11
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +62 -0
M internal/usecase/daemon/resume.go +5 -0
| @@ -48,6 +48,11 @@ func (d *Daemon) parkAttachment(sess *session, ac *attachedClient) bool { | |||
| 48 | 48 | if !d.prepareParkAttachment(sess, ac) { | |
| 49 | 49 | return false | |
| 50 | 50 | } | |
| 51 | + | // A parked attachment has no session owner. Release pane snapshots before | |
| 52 | + | // publishing it in the daemon registry so headless pane/session teardown | |
| 53 | + | // cannot leave closed panes retained through its capture cache. This must | |
| 54 | + | // remain outside d.mu: sendMu is ordered before the daemon lock. | |
| 55 | + | ac.clearCaptureFrames() | |
| 51 | 56 | d.mu.Lock() | |
| 52 | 57 | if d.closing { | |
| 53 | 58 | d.mu.Unlock() | |
| @@ -48,6 +48,11 @@ func (d *Daemon) parkAttachment(sess *session, ac *attachedClient) bool { | |||||
| 48 | if !d.prepareParkAttachment(sess, ac) { | 48 | if !d.prepareParkAttachment(sess, ac) { | ||
| 49 | return false | 49 | return false | ||
| 50 | } | 50 | } | ||
| 51 | + | // A parked attachment has no session owner. Release pane snapshots before | |||
| 52 | + | // publishing it in the daemon registry so headless pane/session teardown | |||
| 53 | + | // cannot leave closed panes retained through its capture cache. This must | |||
| 54 | + | // remain outside d.mu: sendMu is ordered before the daemon lock. | |||
| 55 | + | ac.clearCaptureFrames() | |||
| 51 | d.mu.Lock() | 56 | d.mu.Lock() | ||
| 52 | if d.closing { | 57 | if d.closing { | ||
| 53 | d.mu.Unlock() | 58 | d.mu.Unlock() | ||
M internal/usecase/daemon/resume_test.go +57 -0
| @@ -2,6 +2,7 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | + | "strings" | |
| 5 | 6 | "sync" | |
| 6 | 7 | "testing" | |
| 7 | 8 | "time" | |
| @@ -10,6 +11,8 @@ import ( | |||
| 10 | 11 | ||
| 11 | 12 | "github.com/bnema/vev/internal/domain" | |
| 12 | 13 | "github.com/bnema/vev/internal/ports" | |
| 14 | + | "github.com/bnema/vev/internal/usecase/layout" | |
| 15 | + | "github.com/bnema/vev/pkg/vt" | |
| 13 | 16 | ) | |
| 14 | 17 | ||
| 15 | 18 | type closeTrackingTransport struct { | |
| @@ -296,6 +299,60 @@ func TestResumeRebasesFullOutputWindowBeforeFirstPaint(t *testing.T) { | |||
| 296 | 299 | require.Equal(t, first.NewStateNum, second.BaseStateNum) | |
| 297 | 300 | } | |
| 298 | 301 | ||
| 302 | + | func TestParkingReleasesPaneCapturesBeforeHeadlessCloseAndResume(t *testing.T) { | |
| 303 | + | pty, release := newBlockingPTY(t) | |
| 304 | + | defer release() | |
| 305 | + | d := newTestDaemon(t, newFactorySeq(t, pty), stubClock{}) | |
| 306 | + | oldTransport := &closeTrackingTransport{} | |
| 307 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTransport) | |
| 308 | + | require.NoError(t, err) | |
| 309 | + | ||
| 310 | + | tb := sess.tabs[0] | |
| 311 | + | survivor := tb.panes["pane-1"] | |
| 312 | + | closed := newPane("pane-2", nil, domain.Size{Cols: 40, Rows: 23}) | |
| 313 | + | tb.mu.Lock() | |
| 314 | + | tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2")}}, Focus: "pane-1"} | |
| 315 | + | tb.panes[closed.id] = closed | |
| 316 | + | d.applyLayoutLocked(tb) | |
| 317 | + | tb.mu.Unlock() | |
| 318 | + | survivor.screen.Write([]byte("survivor")) | |
| 319 | + | closed.screen.Write([]byte("closed")) | |
| 320 | + | d.paint(sess, ac, true) | |
| 321 | + | ||
| 322 | + | ac.sendMu.Lock() | |
| 323 | + | require.Contains(t, ac.captureFrames, closed, "fixture must render and capture the pane before parking") | |
| 324 | + | ac.sendMu.Unlock() | |
| 325 | + | token := ac.resumeToken | |
| 326 | + | require.True(t, sess.detachIfCurrent(ac)) | |
| 327 | + | require.True(t, d.parkAttachment(sess, ac)) | |
| 328 | + | ||
| 329 | + | // Headless close cannot find the parked attachment through sess.client. | |
| 330 | + | // Its capture must already have been released before the attachment parked. | |
| 331 | + | require.NoError(t, d.closePane(sess, tb, closed.id, nil, false)) | |
| 332 | + | ac.sendMu.Lock() | |
| 333 | + | require.NotContains(t, ac.captureFrames, closed, "parked attachment must not retain a pane closed while headless") | |
| 334 | + | ac.sendMu.Unlock() | |
| 335 | + | ||
| 336 | + | newTransport := &closeTrackingTransport{} | |
| 337 | + | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, sess.name, token), newTransport) | |
| 338 | + | require.NoError(t, err) | |
| 339 | + | require.Same(t, sess, resumedSess) | |
| 340 | + | require.Same(t, ac, resumedAC) | |
| 341 | + | require.True(t, resumedSess.renderCoordinator().markAttachmentReady(resumedAC)) | |
| 342 | + | d.firstPaint(resumedSess, resumedAC, resumedAC.size) | |
| 343 | + | ||
| 344 | + | sends := newTransport.Sends() | |
| 345 | + | require.Len(t, sends, 1) | |
| 346 | + | output, err := ports.UnmarshalOutput(sends[0].Payload) | |
| 347 | + | require.NoError(t, err) | |
| 348 | + | require.Zero(t, output.BaseStateNum, "resume must start with a complete frame") | |
| 349 | + | terminal := vt.NewScreen(resumedAC.size.Cols, resumedAC.size.Rows) | |
| 350 | + | terminal.Write(output.Data) | |
| 351 | + | contents := strings.Join(frameRows(terminal.Frame), "\n") | |
| 352 | + | require.Contains(t, contents, "survivor", "resume first paint must contain current headless content") | |
| 353 | + | require.NotContains(t, contents, "closed", "resume first paint must not contain closed pane content") | |
| 354 | + | } | |
| 355 | + | ||
| 299 | 356 | func TestExplicitDetachDoesNotPark(t *testing.T) { | |
| 300 | 357 | pty, release := newBlockingPTY(t) | |
| 301 | 358 | defer release() | |
| @@ -2,6 +2,7 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | + | "strings" | |||
| 5 | "sync" | 6 | "sync" | ||
| 6 | "testing" | 7 | "testing" | ||
| 7 | "time" | 8 | "time" | ||
| @@ -10,6 +11,8 @@ import ( | |||||
| 10 | 11 | ||||
| 11 | "github.com/bnema/vev/internal/domain" | 12 | "github.com/bnema/vev/internal/domain" | ||
| 12 | "github.com/bnema/vev/internal/ports" | 13 | "github.com/bnema/vev/internal/ports" | ||
| 14 | + | "github.com/bnema/vev/internal/usecase/layout" | |||
| 15 | + | "github.com/bnema/vev/pkg/vt" | |||
| 13 | ) | 16 | ) | ||
| 14 | 17 | ||||
| 15 | type closeTrackingTransport struct { | 18 | type closeTrackingTransport struct { | ||
| @@ -296,6 +299,60 @@ func TestResumeRebasesFullOutputWindowBeforeFirstPaint(t *testing.T) { | |||||
| 296 | require.Equal(t, first.NewStateNum, second.BaseStateNum) | 299 | require.Equal(t, first.NewStateNum, second.BaseStateNum) | ||
| 297 | } | 300 | } | ||
| 298 | 301 | ||||
| 302 | + | func TestParkingReleasesPaneCapturesBeforeHeadlessCloseAndResume(t *testing.T) { | |||
| 303 | + | pty, release := newBlockingPTY(t) | |||
| 304 | + | defer release() | |||
| 305 | + | d := newTestDaemon(t, newFactorySeq(t, pty), stubClock{}) | |||
| 306 | + | oldTransport := &closeTrackingTransport{} | |||
| 307 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), oldTransport) | |||
| 308 | + | require.NoError(t, err) | |||
| 309 | + | ||||
| 310 | + | tb := sess.tabs[0] | |||
| 311 | + | survivor := tb.panes["pane-1"] | |||
| 312 | + | closed := newPane("pane-2", nil, domain.Size{Cols: 40, Rows: 23}) | |||
| 313 | + | tb.mu.Lock() | |||
| 314 | + | tb.tree = &layout.Tree{Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{layout.NewLeaf("pane-1"), layout.NewLeaf("pane-2")}}, Focus: "pane-1"} | |||
| 315 | + | tb.panes[closed.id] = closed | |||
| 316 | + | d.applyLayoutLocked(tb) | |||
| 317 | + | tb.mu.Unlock() | |||
| 318 | + | survivor.screen.Write([]byte("survivor")) | |||
| 319 | + | closed.screen.Write([]byte("closed")) | |||
| 320 | + | d.paint(sess, ac, true) | |||
| 321 | + | ||||
| 322 | + | ac.sendMu.Lock() | |||
| 323 | + | require.Contains(t, ac.captureFrames, closed, "fixture must render and capture the pane before parking") | |||
| 324 | + | ac.sendMu.Unlock() | |||
| 325 | + | token := ac.resumeToken | |||
| 326 | + | require.True(t, sess.detachIfCurrent(ac)) | |||
| 327 | + | require.True(t, d.parkAttachment(sess, ac)) | |||
| 328 | + | ||||
| 329 | + | // Headless close cannot find the parked attachment through sess.client. | |||
| 330 | + | // Its capture must already have been released before the attachment parked. | |||
| 331 | + | require.NoError(t, d.closePane(sess, tb, closed.id, nil, false)) | |||
| 332 | + | ac.sendMu.Lock() | |||
| 333 | + | require.NotContains(t, ac.captureFrames, closed, "parked attachment must not retain a pane closed while headless") | |||
| 334 | + | ac.sendMu.Unlock() | |||
| 335 | + | ||||
| 336 | + | newTransport := &closeTrackingTransport{} | |||
| 337 | + | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, sess.name, token), newTransport) | |||
| 338 | + | require.NoError(t, err) | |||
| 339 | + | require.Same(t, sess, resumedSess) | |||
| 340 | + | require.Same(t, ac, resumedAC) | |||
| 341 | + | require.True(t, resumedSess.renderCoordinator().markAttachmentReady(resumedAC)) | |||
| 342 | + | d.firstPaint(resumedSess, resumedAC, resumedAC.size) | |||
| 343 | + | ||||
| 344 | + | sends := newTransport.Sends() | |||
| 345 | + | require.Len(t, sends, 1) | |||
| 346 | + | output, err := ports.UnmarshalOutput(sends[0].Payload) | |||
| 347 | + | require.NoError(t, err) | |||
| 348 | + | require.Zero(t, output.BaseStateNum, "resume must start with a complete frame") | |||
| 349 | + | terminal := vt.NewScreen(resumedAC.size.Cols, resumedAC.size.Rows) | |||
| 350 | + | terminal.Write(output.Data) | |||
| 351 | + | contents := strings.Join(frameRows(terminal.Frame), "\n") | |||
| 352 | + | require.Contains(t, contents, "survivor", "resume first paint must contain current headless content") | |||
| 353 | + | require.NotContains(t, contents, "closed", "resume first paint must not contain closed pane content") | |||
| 354 | + | } | |||
| 355 | + | ||||
| 299 | func TestExplicitDetachDoesNotPark(t *testing.T) { | 356 | func TestExplicitDetachDoesNotPark(t *testing.T) { | ||
| 300 | pty, release := newBlockingPTY(t) | 357 | pty, release := newBlockingPTY(t) | ||
| 301 | defer release() | 358 | defer release() | ||