70ae2be perf(copy): release pane lock after snapshot capture · vev

M internal/usecase/daemon/copymode.go +10 -48


@@ -50,27 +50,15 @@ func copyTargetPane(rt overlayRuntime) pane {
50	50
51	51		func (d Daemon) copyWheel(sess session, ac *attachedClient, delta int) {
52	52		rt := ac.overlays
53		-	p := copyTargetPane(rt)
54		-	if p == nil {
55		-	return
56		-	}
57		-	p.mu.Lock()
58	53		rt.copyMu.Lock()
59		-	if rt.copyMode == nil \|\| rt.copyPane != p {
	54	+	if rt.copyMode == nil \|\| rt.copyPane == nil \|\| rt.copySnapshot == nil {
60	55		rt.copyMu.Unlock()
61		-	p.mu.Unlock()
62		-	return
63		-	}
64		-	if rt.copySnapshot == nil {
65		-	rt.copyMu.Unlock()
66		-	p.mu.Unlock()
67	56		return
68	57		}
69	58		document := rt.copySnapshot
70	59		if delta > 0 && rt.copyMode.AtBottom(*document) {
71	60		rt.clearCopyModeLocked()
72	61		rt.copyMu.Unlock()
73		-	p.mu.Unlock()
74	62		d.paint(sess, ac, true)
75	63		return
76	64		}

@@ -80,7 +68,6 @@ func (d Daemon) copyWheel(sess session, ac *attachedClient, delta int) {
80	68		rt.clearCopyModeLocked()
81	69		}
82	70		rt.copyMu.Unlock()
83		-	p.mu.Unlock()
84	71
85	72		d.paint(sess, ac, true)
86	73		}

@@ -98,8 +85,11 @@ func (d Daemon) enterCopyMode(sess session, ac *attachedClient) {
98	85		if p == nil {
99	86		return
100	87		}
	88	+	// Capture the live pane under pane.mu; all subsequent copy interaction uses
	89	+	// the immutable document while holding only copyMu.
101	90		p.mu.Lock()
102	91		document := scopy.NewSnapshot(p.scrollback, p.screen.Frame)
	92	+	p.mu.Unlock()
103	93		rt.copyMu.Lock()
104	94		rt.copyMode = scopy.NewMode(document)
105	95		rt.copySnapshot = &document

@@ -110,7 +100,6 @@ func (d Daemon) enterCopyMode(sess session, ac *attachedClient) {
110	100		rt.copyDragging = false
111	101		rt.normalMousePressValid = false
112	102		rt.copyMu.Unlock()
113		-	p.mu.Unlock()
114	103		if floatingSource {
115	104		tb.mu.Lock()
116	105		stillVisible := tb.floating.state == floatingVisible && tb.floating.pane == p

@@ -127,33 +116,20 @@ func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {
127	116		if ev.Button != mouse.Left {
128	117		return
129	118		}
130		-	p := copyTargetPane(rt)
131		-	if p == nil {
	119	+	rt.copyMu.Lock()
	120	+	if rt.copyMode == nil \|\| rt.copyPane == nil \|\| rt.copySnapshot == nil {
	121	+	rt.copyMu.Unlock()
132	122		return
133	123		}
134		-	p.mu.Lock()
135		-	if ev.Row >= p.screen.Frame.Height {
	124	+	document := rt.copySnapshot
	125	+	if ev.Row >= document.Height {
136	126		if ev.Type == mouse.Press {
137		-	rt.copyMu.Lock()
138	127		rt.copyPressRowValid = false
139	128		rt.copyDragging = false
140		-	rt.copyMu.Unlock()
141	129		}
142		-	p.mu.Unlock()
143		-	return
144		-	}
145		-	rt.copyMu.Lock()
146		-	if rt.copyMode == nil \|\| rt.copyPane != p {
147		-	rt.copyMu.Unlock()
148		-	p.mu.Unlock()
149		-	return
150		-	}
151		-	if rt.copySnapshot == nil {
152	130		rt.copyMu.Unlock()
153		-	p.mu.Unlock()
154	131		return
155	132		}
156		-	document := rt.copySnapshot
157	133		absRow := rt.copyMode.ViewportTop + ev.Row
158	134		changed := false
159	135		switch ev.Type {

@@ -177,7 +153,6 @@ func (d Daemon) copyMouse(sess session, ac *attachedClient, ev mouse.Event) {
177	153		// Button release intentionally has no visual effect.
178	154		}
179	155		rt.copyMu.Unlock()
180		-	p.mu.Unlock()
181	156
182	157		if changed {
183	158		d.paint(sess, ac, true)

@@ -190,17 +165,11 @@ func (d Daemon) handleCopyInput(ac attachedClient, data []byte) {
190	165		if sess == nil {
191	166		return
192	167		}
193		-	p := copyTargetPane(rt)
194		-	if p == nil {
195		-	return
196		-	}
197		-	p.mu.Lock()
198	168		rt.copyMu.Lock()
199		-	if rt.copyMode == nil \|\| rt.copyPane != p {
	169	+	if rt.copyMode == nil \|\| rt.copyPane == nil \|\| rt.copySnapshot == nil {
200	170		rt.copyPending = nil
201	171		d.stopCopyPendingTimerLocked(ac)
202	172		rt.copyMu.Unlock()
203		-	p.mu.Unlock()
204	173		return
205	174		}
206	175		if len(rt.copyPending) > 0 {

@@ -211,16 +180,10 @@ func (d Daemon) handleCopyInput(ac attachedClient, data []byte) {
211	180		data = combined
212	181		rt.copyPending = nil
213	182		}
214		-	if rt.copySnapshot == nil {
215		-	rt.copyMu.Unlock()
216		-	p.mu.Unlock()
217		-	return
218		-	}
219	183		document := rt.copySnapshot
220	184		if rt.copySearch != nil {
221	185		changed, closeSearch, accepted := d.routeCopySearchInputLocked(rt, *document, data)
222	186		rt.copyMu.Unlock()
223		-	p.mu.Unlock()
224	187		if changed \|\| closeSearch \|\| accepted {
225	188		d.paint(sess, ac, true)
226	189		}

@@ -292,7 +255,6 @@ func (d Daemon) handleCopyInput(ac attachedClient, data []byte) {
292	255		rt.clearCopyModeLocked()
293	256		}
294	257		rt.copyMu.Unlock()
295		-	p.mu.Unlock()
296	258
297	259		if copyOut && text != "" {
298	260		chunks := scopy.OSC52(text)

 func (d *Daemon) copyWheel(sess *session, ac *attachedClient, delta int) {
 	rt := ac.overlays
-	p := copyTargetPane(rt)
-	if p == nil {
-		return
-	}
-	p.mu.Lock()
 	rt.copyMu.Lock()
-	if rt.copyMode == nil || rt.copyPane != p {
+	if rt.copyMode == nil || rt.copyPane == nil || rt.copySnapshot == nil {
 		rt.copyMu.Unlock()
-		p.mu.Unlock()
-		return
-	}
-	if rt.copySnapshot == nil {
-		rt.copyMu.Unlock()
-		p.mu.Unlock()
 		return
 	}
 	document := rt.copySnapshot
 	if delta > 0 && rt.copyMode.AtBottom(*document) {
 		rt.clearCopyModeLocked()
 		rt.copyMu.Unlock()
-		p.mu.Unlock()
 		d.paint(sess, ac, true)
 		return
 	}
 		rt.clearCopyModeLocked()
 	}
 	rt.copyMu.Unlock()
-	p.mu.Unlock()
 	d.paint(sess, ac, true)
 }
 	if p == nil {
 		return
 	}
+	// Capture the live pane under pane.mu; all subsequent copy interaction uses
+	// the immutable document while holding only copyMu.
 	p.mu.Lock()
 	document := scopy.NewSnapshot(p.scrollback, p.screen.Frame)
+	p.mu.Unlock()
 	rt.copyMu.Lock()
 	rt.copyMode = scopy.NewMode(document)
 	rt.copySnapshot = &document
 	rt.copyDragging = false
 	rt.normalMousePressValid = false
 	rt.copyMu.Unlock()
-	p.mu.Unlock()
 	if floatingSource {
 		tb.mu.Lock()
 		stillVisible := tb.floating.state == floatingVisible && tb.floating.pane == p
 	if ev.Button != mouse.Left {
 		return
 	}
-	p := copyTargetPane(rt)
+	rt.copyMu.Lock()
-	if p == nil {
+	if rt.copyMode == nil || rt.copyPane == nil || rt.copySnapshot == nil {
+		rt.copyMu.Unlock()
 		return
 	}
-	p.mu.Lock()
+	document := rt.copySnapshot
-	if ev.Row >= p.screen.Frame.Height {
+	if ev.Row >= document.Height {
 		if ev.Type == mouse.Press {
-			rt.copyMu.Lock()
 			rt.copyPressRowValid = false
 			rt.copyDragging = false
-			rt.copyMu.Unlock()
 		}
-		p.mu.Unlock()
-		return
-	}
-	rt.copyMu.Lock()
-	if rt.copyMode == nil || rt.copyPane != p {
-		rt.copyMu.Unlock()
-		p.mu.Unlock()
-		return
-	}
-	if rt.copySnapshot == nil {
 		rt.copyMu.Unlock()
-		p.mu.Unlock()
 		return
 	}
-	document := rt.copySnapshot
 	absRow := rt.copyMode.ViewportTop + ev.Row
 	changed := false
 	switch ev.Type {
 		// Button release intentionally has no visual effect.
 	}
 	rt.copyMu.Unlock()
-	p.mu.Unlock()
 	if changed {
 		d.paint(sess, ac, true)
 	if sess == nil {
 		return
 	}
-	p := copyTargetPane(rt)
-	if p == nil {
-		return
-	}
-	p.mu.Lock()
 	rt.copyMu.Lock()
-	if rt.copyMode == nil || rt.copyPane != p {
+	if rt.copyMode == nil || rt.copyPane == nil || rt.copySnapshot == nil {
 		rt.copyPending = nil
 		d.stopCopyPendingTimerLocked(ac)
 		rt.copyMu.Unlock()
-		p.mu.Unlock()
 		return
 	}
 	if len(rt.copyPending) > 0 {
 		data = combined
 		rt.copyPending = nil
 	}
-	if rt.copySnapshot == nil {
-		rt.copyMu.Unlock()
-		p.mu.Unlock()
-		return
-	}
 	document := rt.copySnapshot
 	if rt.copySearch != nil {
 		changed, closeSearch, accepted := d.routeCopySearchInputLocked(rt, *document, data)
 		rt.copyMu.Unlock()
-		p.mu.Unlock()
 		if changed || closeSearch || accepted {
 			d.paint(sess, ac, true)
 		}
 		rt.clearCopyModeLocked()
 	}
 	rt.copyMu.Unlock()
-	p.mu.Unlock()
 	if copyOut && text != "" {
 		chunks := scopy.OSC52(text)

M internal/usecase/daemon/copymode_test.go +22 -0


@@ -599,6 +599,28 @@ func TestCopyModeEmptyYankDoesNotClearClipboard(t *testing.T) {
599	599		}
600	600		}
601	601
	602	+	func TestHandleCopyInputUsesImmutableSnapshotWithoutPaneLock(t *testing.T) {
	603	+	p, _ := newBlockingPTY(t)
	604	+	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
	605	+	pane := sess.activeTab().focusedPane()
	606	+	pane.screen.Write([]byte("live"))
	607	+	d.enterCopyMode(sess, ac)
	608	+	awaitFrame(t, sends, ports.MsgOutput)
	609	+
	610	+	pane.mu.Lock()
	611	+	defer pane.mu.Unlock()
	612	+	done := make(chan struct{})
	613	+	go func() {
	614	+	d.handleCopyInput(ac, []byte("x"))
	615	+	close(done)
	616	+	}()
	617	+	select {
	618	+	case <-done:
	619	+	case <-time.After(time.Second):
	620	+	t.Fatal("immutable copy input waited for the live pane lock")
	621	+	}
	622	+	}
	623	+
602	624		func TestCopyModeEnterExitConcurrentWithPaintRace(t *testing.T) {
603	625		p, _ := newBlockingPTY(t)
604	626		d, sess, ac, sends := newManualSessionWithPTYs(t, p)


@@ -599,6 +599,28 @@ func TestCopyModeEmptyYankDoesNotClearClipboard(t *testing.T) {
599	}	599		}
600	}	600		}
601		601
		602	+	func TestHandleCopyInputUsesImmutableSnapshotWithoutPaneLock(t *testing.T) {
		603	+	p, _ := newBlockingPTY(t)
		604	+	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
		605	+	pane := sess.activeTab().focusedPane()
		606	+	pane.screen.Write([]byte("live"))
		607	+	d.enterCopyMode(sess, ac)
		608	+	awaitFrame(t, sends, ports.MsgOutput)
		609	+
		610	+	pane.mu.Lock()
		611	+	defer pane.mu.Unlock()
		612	+	done := make(chan struct{})
		613	+	go func() {
		614	+	d.handleCopyInput(ac, []byte("x"))
		615	+	close(done)
		616	+	}()
		617	+	select {
		618	+	case <-done:
		619	+	case <-time.After(time.Second):
		620	+	t.Fatal("immutable copy input waited for the live pane lock")
		621	+	}
		622	+	}
		623	+
602	func TestCopyModeEnterExitConcurrentWithPaintRace(t *testing.T) {	624		func TestCopyModeEnterExitConcurrentWithPaintRace(t *testing.T) {
603	p, _ := newBlockingPTY(t)	625		p, _ := newBlockingPTY(t)
604	d, sess, ac, sends := newManualSessionWithPTYs(t, p)	626		d, sess, ac, sends := newManualSessionWithPTYs(t, p)

M internal/usecase/daemon/input.go +2 -1


@@ -270,8 +270,10 @@ func (d Daemon) handleTerminalMouse(sess session, ac attachedClient, p pane,
270	270		return
271	271		}
272	272
	273	+	// The immutable snapshot is the only live-pane access needed for drag entry.
273	274		p.mu.Lock()
274	275		document := scopy.NewSnapshot(p.scrollback, p.screen.Frame)
	276	+	p.mu.Unlock()
275	277		rt.copyMu.Lock()
276	278		mode := scopy.NewMode(document)
277	279		mode.StartSelectionAt(document, pressTop+pressRow)

@@ -284,7 +286,6 @@ func (d Daemon) handleTerminalMouse(sess session, ac attachedClient, p pane,
284	286		rt.copyDragging = true
285	287		rt.normalMousePressValid = false
286	288		rt.copyMu.Unlock()
287		-	p.mu.Unlock()
288	289		d.paint(sess, ac, true)
289	290		case mouse.Release:
290	291		rt.copyMu.Lock()

 				return
 			}
+			// The immutable snapshot is the only live-pane access needed for drag entry.
 			p.mu.Lock()
 			document := scopy.NewSnapshot(p.scrollback, p.screen.Frame)
+			p.mu.Unlock()
 			rt.copyMu.Lock()
 			mode := scopy.NewMode(document)
 			mode.StartSelectionAt(document, pressTop+pressRow)
 			rt.copyDragging = true
 			rt.normalMousePressValid = false
 			rt.copyMu.Unlock()
-			p.mu.Unlock()
 			d.paint(sess, ac, true)
 		case mouse.Release:
 			rt.copyMu.Lock()