detached 82284b7

fix(daemon): close tab creation race

commit
82284b7e1a346b25215250b9b757ee53ebac7048
author
brice · · 2026-07-03 12:20 UTC
parent
7281253
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
2 files · +63 -2
Split
UnifiedSplit
M internal/usecase/daemon/session.go +13 -2
@@ -107,17 +107,28 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz
107107
108108 func (d *Daemon) createTab(sess *session, sz domain.Size) error {
109109 tbSize := tabSize(sz)
110- pty, err := d.ptys.Open(d.shell, d.shellArgs, d.childEnv(sess.name), sess.cwd, tbSize)
110+ sess.mu.Lock()
111+ name := sess.name
112+ cwd := sess.cwd
113+ sess.mu.Unlock()
114+ pty, err := d.ptys.Open(d.shell, d.shellArgs, d.childEnv(name), cwd, tbSize)
111115 if err != nil {
112- return fmt.Errorf("daemon: spawning tab for session %q: %w", sess.name, err)
116+ return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err)
113117 }
114118 tb := newTab(pty, tbSize)
119+ d.mu.Lock()
120+ if d.closing || d.sessions[sess.id] != sess || sess.ctx.Err() != nil {
121+ d.mu.Unlock()
122+ _ = pty.Close()
123+ return errors.New("daemon: session closed")
124+ }
115125 tb.ctx, tb.cancel = context.WithCancel(sess.ctx)
116126 sess.mu.Lock()
117127 sess.tabs = append(sess.tabs, tb)
118128 sess.active = len(sess.tabs) - 1
119129 sess.mu.Unlock()
120130 d.startTabGoroutines(sess, tb)
131+ d.mu.Unlock()
121132 return nil
122133 }
123134
@@ -107,17 +107,28 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz
107 107
108 func (d *Daemon) createTab(sess *session, sz domain.Size) error {108 func (d *Daemon) createTab(sess *session, sz domain.Size) error {
109 tbSize := tabSize(sz)109 tbSize := tabSize(sz)
110- pty, err := d.ptys.Open(d.shell, d.shellArgs, d.childEnv(sess.name), sess.cwd, tbSize)110+ sess.mu.Lock()
111+ name := sess.name
112+ cwd := sess.cwd
113+ sess.mu.Unlock()
114+ pty, err := d.ptys.Open(d.shell, d.shellArgs, d.childEnv(name), cwd, tbSize)
111 if err != nil {115 if err != nil {
112- return fmt.Errorf("daemon: spawning tab for session %q: %w", sess.name, err)116+ return fmt.Errorf("daemon: spawning tab for session %q: %w", name, err)
113 }117 }
114 tb := newTab(pty, tbSize)118 tb := newTab(pty, tbSize)
119+ d.mu.Lock()
120+ if d.closing || d.sessions[sess.id] != sess || sess.ctx.Err() != nil {
121+ d.mu.Unlock()
122+ _ = pty.Close()
123+ return errors.New("daemon: session closed")
124+ }
115 tb.ctx, tb.cancel = context.WithCancel(sess.ctx)125 tb.ctx, tb.cancel = context.WithCancel(sess.ctx)
116 sess.mu.Lock()126 sess.mu.Lock()
117 sess.tabs = append(sess.tabs, tb)127 sess.tabs = append(sess.tabs, tb)
118 sess.active = len(sess.tabs) - 1128 sess.active = len(sess.tabs) - 1
119 sess.mu.Unlock()129 sess.mu.Unlock()
120 d.startTabGoroutines(sess, tb)130 d.startTabGoroutines(sess, tb)
131+ d.mu.Unlock()
121 return nil132 return nil
122 }133 }
123 134
M internal/usecase/daemon/session_test.go +50 -0
@@ -179,6 +179,56 @@ func TestKillAllEmptyDaemonSignalsShutdown(t *testing.T) {
179179 }
180180 }
181181
182+func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
183+ p1, release1 := newBlockingPTY(t)
184+ defer release1()
185+ p2 := portsmocks.NewMockPTY(t)
186+ opened := make(chan struct{})
187+ releaseOpen := make(chan struct{})
188+ closed := make(chan struct{})
189+ p2.EXPECT().Close().RunAndReturn(func() error {
190+ close(closed)
191+ return nil
192+ }).Once()
193+ p2.EXPECT().Pid().Return(4242).Maybe()
194+
195+ f := portsmocks.NewMockPTYFactory(t)
196+ f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(p1, nil).Once()
197+ f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
198+ func(string, []string, []string, string, domain.Size) (ports.PTY, error) {
199+ close(opened)
200+ <-releaseOpen
201+ return p2, nil
202+ },
203+ ).Once()
204+
205+ d := newTestDaemon(t, f, stubClock{})
206+ tr, sends, releaseConn := newConn(t, mustHello(ports.IntentNew, "work", domain.Size{Cols: 80, Rows: 24}))
207+ var hg sync.WaitGroup
208+ hg.Go(func() { d.handleConn(tr) })
209+ awaitFrame(t, sends, ports.MsgWelcome)
210+ awaitFrame(t, sends, ports.MsgOutput)
211+ sess := firstSession(d)
212+ require.NotNil(t, sess)
213+
214+ errCh := make(chan error, 1)
215+ go func() { errCh <- d.createTab(sess, domain.Size{Cols: 80, Rows: 24}) }()
216+ <-opened
217+ d.killSession(sess, ports.ReasonSessionKilled)
218+ close(releaseOpen)
219+
220+ require.Error(t, <-errCh)
221+ select {
222+ case <-closed:
223+ case <-time.After(time.Second):
224+ t.Fatal("new PTY was not closed after session died during tab creation")
225+ }
226+ require.Equal(t, 0, sessionCount(d))
227+ releaseConn()
228+ hg.Wait()
229+ d.sessWg.Wait()
230+}
231+
182232 // --- ephemeral numbering ----------------------------------------------------
183233
184234 func TestEphemeralNumberingReuse(t *testing.T) {
@@ -179,6 +179,56 @@ func TestKillAllEmptyDaemonSignalsShutdown(t *testing.T) {
179 }179 }
180 }180 }
181 181
182+func TestCreateTabClosesPTYIfSessionKilledDuringOpen(t *testing.T) {
183+ p1, release1 := newBlockingPTY(t)
184+ defer release1()
185+ p2 := portsmocks.NewMockPTY(t)
186+ opened := make(chan struct{})
187+ releaseOpen := make(chan struct{})
188+ closed := make(chan struct{})
189+ p2.EXPECT().Close().RunAndReturn(func() error {
190+ close(closed)
191+ return nil
192+ }).Once()
193+ p2.EXPECT().Pid().Return(4242).Maybe()
194+
195+ f := portsmocks.NewMockPTYFactory(t)
196+ f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(p1, nil).Once()
197+ f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
198+ func(string, []string, []string, string, domain.Size) (ports.PTY, error) {
199+ close(opened)
200+ <-releaseOpen
201+ return p2, nil
202+ },
203+ ).Once()
204+
205+ d := newTestDaemon(t, f, stubClock{})
206+ tr, sends, releaseConn := newConn(t, mustHello(ports.IntentNew, "work", domain.Size{Cols: 80, Rows: 24}))
207+ var hg sync.WaitGroup
208+ hg.Go(func() { d.handleConn(tr) })
209+ awaitFrame(t, sends, ports.MsgWelcome)
210+ awaitFrame(t, sends, ports.MsgOutput)
211+ sess := firstSession(d)
212+ require.NotNil(t, sess)
213+
214+ errCh := make(chan error, 1)
215+ go func() { errCh <- d.createTab(sess, domain.Size{Cols: 80, Rows: 24}) }()
216+ <-opened
217+ d.killSession(sess, ports.ReasonSessionKilled)
218+ close(releaseOpen)
219+
220+ require.Error(t, <-errCh)
221+ select {
222+ case <-closed:
223+ case <-time.After(time.Second):
224+ t.Fatal("new PTY was not closed after session died during tab creation")
225+ }
226+ require.Equal(t, 0, sessionCount(d))
227+ releaseConn()
228+ hg.Wait()
229+ d.sessWg.Wait()
230+}
231+
182 // --- ephemeral numbering ----------------------------------------------------232 // --- ephemeral numbering ----------------------------------------------------
183 233
184 func TestEphemeralNumberingReuse(t *testing.T) {234 func TestEphemeralNumberingReuse(t *testing.T) {

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close