fix: synchronize daemon copy mode
- commit
- fb67a13ee6c9be0c44b6b058706508f74e00f6b6
- author
- brice · · 2026-07-02 16:07 UTC
- parent
- e47efb0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +56 -3
M internal/usecase/daemon/daemon.go +23 -3
| @@ -147,10 +147,17 @@ type attachedClient struct { | |||
| 147 | 147 | rend *renderer.Renderer | |
| 148 | 148 | size domain.Size | |
| 149 | 149 | keys *keys.Router | |
| 150 | + | copyMu sync.Mutex | |
| 150 | 151 | copyMode *scopy.Mode | |
| 151 | 152 | sendMu sync.Mutex | |
| 152 | 153 | } | |
| 153 | 154 | ||
| 155 | + | func (ac *attachedClient) copyModeActive() bool { | |
| 156 | + | ac.copyMu.Lock() | |
| 157 | + | defer ac.copyMu.Unlock() | |
| 158 | + | return ac.copyMode != nil | |
| 159 | + | } | |
| 160 | + | ||
| 154 | 161 | // send serialises a frame onto the client's transport. | |
| 155 | 162 | func (ac *attachedClient) send(f ports.Frame) error { | |
| 156 | 163 | ac.sendMu.Lock() | |
| @@ -680,7 +687,7 @@ func (s *session) activeWindow() *window { | |||
| 680 | 687 | } | |
| 681 | 688 | ||
| 682 | 689 | func (d *Daemon) handleInput(sess *session, ac *attachedClient, data []byte) { | |
| 683 | - | if ac.copyMode != nil { | |
| 690 | + | if ac.copyModeActive() { | |
| 684 | 691 | d.handleCopyInput(sess, ac, data) | |
| 685 | 692 | return | |
| 686 | 693 | } | |
| @@ -698,17 +705,25 @@ func (d *Daemon) enterCopyMode(sess *session, ac *attachedClient) { | |||
| 698 | 705 | win.mu.Lock() | |
| 699 | 706 | snap := scopy.NewSnapshot(win.scrollback, win.screen.Frame) | |
| 700 | 707 | win.mu.Unlock() | |
| 708 | + | ac.copyMu.Lock() | |
| 701 | 709 | ac.copyMode = scopy.NewMode(snap) | |
| 710 | + | ac.copyMu.Unlock() | |
| 702 | 711 | d.paint(sess, ac, true) | |
| 703 | 712 | } | |
| 704 | 713 | ||
| 705 | 714 | func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) { | |
| 706 | 715 | win := sess.activeWindow() | |
| 707 | - | if win == nil || ac.copyMode == nil { | |
| 716 | + | if win == nil { | |
| 708 | 717 | return | |
| 709 | 718 | } | |
| 710 | 719 | ||
| 711 | 720 | win.mu.Lock() | |
| 721 | + | ac.copyMu.Lock() | |
| 722 | + | if ac.copyMode == nil { | |
| 723 | + | ac.copyMu.Unlock() | |
| 724 | + | win.mu.Unlock() | |
| 725 | + | return | |
| 726 | + | } | |
| 712 | 727 | snap := scopy.NewSnapshot(win.scrollback, win.screen.Frame) | |
| 713 | 728 | changed := false | |
| 714 | 729 | copyOut := false | |
| @@ -749,6 +764,10 @@ func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) | |||
| 749 | 764 | if copyOut { | |
| 750 | 765 | text = ac.copyMode.SelectedText(snap) | |
| 751 | 766 | } | |
| 767 | + | if exit { | |
| 768 | + | ac.copyMode = nil | |
| 769 | + | } | |
| 770 | + | ac.copyMu.Unlock() | |
| 752 | 771 | win.mu.Unlock() | |
| 753 | 772 | ||
| 754 | 773 | if copyOut { | |
| @@ -760,7 +779,6 @@ func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) | |||
| 760 | 779 | } | |
| 761 | 780 | } | |
| 762 | 781 | if exit { | |
| 763 | - | ac.copyMode = nil | |
| 764 | 782 | d.paint(sess, ac, true) | |
| 765 | 783 | return | |
| 766 | 784 | } | |
| @@ -1001,6 +1019,7 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 1001 | 1019 | ||
| 1002 | 1020 | ac.sendMu.Lock() | |
| 1003 | 1021 | win.mu.Lock() | |
| 1022 | + | ac.copyMu.Lock() | |
| 1004 | 1023 | if reset || ac.copyMode != nil { | |
| 1005 | 1024 | ac.rend.Reset() | |
| 1006 | 1025 | } | |
| @@ -1008,6 +1027,7 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 1008 | 1027 | if ac.copyMode != nil { | |
| 1009 | 1028 | frame, damage = composeCopyClientFrame(ac.copyMode, win) | |
| 1010 | 1029 | } | |
| 1030 | + | ac.copyMu.Unlock() | |
| 1011 | 1031 | data, err := ac.rend.Draw(frame, damage) | |
| 1012 | 1032 | win.screen.ClearDamage() | |
| 1013 | 1033 | win.mu.Unlock() | |
| @@ -147,10 +147,17 @@ type attachedClient struct { | |||||
| 147 | rend *renderer.Renderer | 147 | rend *renderer.Renderer | ||
| 148 | size domain.Size | 148 | size domain.Size | ||
| 149 | keys *keys.Router | 149 | keys *keys.Router | ||
| 150 | + | copyMu sync.Mutex | |||
| 150 | copyMode *scopy.Mode | 151 | copyMode *scopy.Mode | ||
| 151 | sendMu sync.Mutex | 152 | sendMu sync.Mutex | ||
| 152 | } | 153 | } | ||
| 153 | 154 | ||||
| 155 | + | func (ac *attachedClient) copyModeActive() bool { | |||
| 156 | + | ac.copyMu.Lock() | |||
| 157 | + | defer ac.copyMu.Unlock() | |||
| 158 | + | return ac.copyMode != nil | |||
| 159 | + | } | |||
| 160 | + | ||||
| 154 | // send serialises a frame onto the client's transport. | 161 | // send serialises a frame onto the client's transport. | ||
| 155 | func (ac *attachedClient) send(f ports.Frame) error { | 162 | func (ac *attachedClient) send(f ports.Frame) error { | ||
| 156 | ac.sendMu.Lock() | 163 | ac.sendMu.Lock() | ||
| @@ -680,7 +687,7 @@ func (s *session) activeWindow() *window { | |||||
| 680 | } | 687 | } | ||
| 681 | 688 | ||||
| 682 | func (d *Daemon) handleInput(sess *session, ac *attachedClient, data []byte) { | 689 | func (d *Daemon) handleInput(sess *session, ac *attachedClient, data []byte) { | ||
| 683 | - | if ac.copyMode != nil { | 690 | + | if ac.copyModeActive() { |
| 684 | d.handleCopyInput(sess, ac, data) | 691 | d.handleCopyInput(sess, ac, data) | ||
| 685 | return | 692 | return | ||
| 686 | } | 693 | } | ||
| @@ -698,17 +705,25 @@ func (d *Daemon) enterCopyMode(sess *session, ac *attachedClient) { | |||||
| 698 | win.mu.Lock() | 705 | win.mu.Lock() | ||
| 699 | snap := scopy.NewSnapshot(win.scrollback, win.screen.Frame) | 706 | snap := scopy.NewSnapshot(win.scrollback, win.screen.Frame) | ||
| 700 | win.mu.Unlock() | 707 | win.mu.Unlock() | ||
| 708 | + | ac.copyMu.Lock() | |||
| 701 | ac.copyMode = scopy.NewMode(snap) | 709 | ac.copyMode = scopy.NewMode(snap) | ||
| 710 | + | ac.copyMu.Unlock() | |||
| 702 | d.paint(sess, ac, true) | 711 | d.paint(sess, ac, true) | ||
| 703 | } | 712 | } | ||
| 704 | 713 | ||||
| 705 | func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) { | 714 | func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) { | ||
| 706 | win := sess.activeWindow() | 715 | win := sess.activeWindow() | ||
| 707 | - | if win == nil || ac.copyMode == nil { | 716 | + | if win == nil { |
| 708 | return | 717 | return | ||
| 709 | } | 718 | } | ||
| 710 | 719 | ||||
| 711 | win.mu.Lock() | 720 | win.mu.Lock() | ||
| 721 | + | ac.copyMu.Lock() | |||
| 722 | + | if ac.copyMode == nil { | |||
| 723 | + | ac.copyMu.Unlock() | |||
| 724 | + | win.mu.Unlock() | |||
| 725 | + | return | |||
| 726 | + | } | |||
| 712 | snap := scopy.NewSnapshot(win.scrollback, win.screen.Frame) | 727 | snap := scopy.NewSnapshot(win.scrollback, win.screen.Frame) | ||
| 713 | changed := false | 728 | changed := false | ||
| 714 | copyOut := false | 729 | copyOut := false | ||
| @@ -749,6 +764,10 @@ func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) | |||||
| 749 | if copyOut { | 764 | if copyOut { | ||
| 750 | text = ac.copyMode.SelectedText(snap) | 765 | text = ac.copyMode.SelectedText(snap) | ||
| 751 | } | 766 | } | ||
| 767 | + | if exit { | |||
| 768 | + | ac.copyMode = nil | |||
| 769 | + | } | |||
| 770 | + | ac.copyMu.Unlock() | |||
| 752 | win.mu.Unlock() | 771 | win.mu.Unlock() | ||
| 753 | 772 | ||||
| 754 | if copyOut { | 773 | if copyOut { | ||
| @@ -760,7 +779,6 @@ func (d *Daemon) handleCopyInput(sess *session, ac *attachedClient, data []byte) | |||||
| 760 | } | 779 | } | ||
| 761 | } | 780 | } | ||
| 762 | if exit { | 781 | if exit { | ||
| 763 | - | ac.copyMode = nil | |||
| 764 | d.paint(sess, ac, true) | 782 | d.paint(sess, ac, true) | ||
| 765 | return | 783 | return | ||
| 766 | } | 784 | } | ||
| @@ -1001,6 +1019,7 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 1001 | 1019 | ||||
| 1002 | ac.sendMu.Lock() | 1020 | ac.sendMu.Lock() | ||
| 1003 | win.mu.Lock() | 1021 | win.mu.Lock() | ||
| 1022 | + | ac.copyMu.Lock() | |||
| 1004 | if reset || ac.copyMode != nil { | 1023 | if reset || ac.copyMode != nil { | ||
| 1005 | ac.rend.Reset() | 1024 | ac.rend.Reset() | ||
| 1006 | } | 1025 | } | ||
| @@ -1008,6 +1027,7 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 1008 | if ac.copyMode != nil { | 1027 | if ac.copyMode != nil { | ||
| 1009 | frame, damage = composeCopyClientFrame(ac.copyMode, win) | 1028 | frame, damage = composeCopyClientFrame(ac.copyMode, win) | ||
| 1010 | } | 1029 | } | ||
| 1030 | + | ac.copyMu.Unlock() | |||
| 1011 | data, err := ac.rend.Draw(frame, damage) | 1031 | data, err := ac.rend.Draw(frame, damage) | ||
| 1012 | win.screen.ClearDamage() | 1032 | win.screen.ClearDamage() | ||
| 1013 | win.mu.Unlock() | 1033 | win.mu.Unlock() | ||
M internal/usecase/daemon/daemon_test.go +33 -0
| @@ -340,6 +340,39 @@ func TestCopyModeEscapeRestoresLiveFullRepaint(t *testing.T) { | |||
| 340 | 340 | } | |
| 341 | 341 | } | |
| 342 | 342 | ||
| 343 | + | func TestCopyModeEnterExitConcurrentWithPaintRace(t *testing.T) { | |
| 344 | + | p, _ := newBlockingPTY(t) | |
| 345 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| 346 | + | sess.windows[0].scrollback = scopy.NewScrollback(4) | |
| 347 | + | sess.windows[0].screen.Write([]byte("live")) | |
| 348 | + | ||
| 349 | + | done := make(chan struct{}) | |
| 350 | + | var drain sync.WaitGroup | |
| 351 | + | drain.Go(func() { | |
| 352 | + | for { | |
| 353 | + | select { | |
| 354 | + | case <-sends: | |
| 355 | + | case <-done: | |
| 356 | + | return | |
| 357 | + | } | |
| 358 | + | } | |
| 359 | + | }) | |
| 360 | + | ||
| 361 | + | var wg sync.WaitGroup | |
| 362 | + | for range 100 { | |
| 363 | + | wg.Go(func() { | |
| 364 | + | d.enterCopyMode(sess, ac) | |
| 365 | + | d.handleInput(sess, ac, []byte("q")) | |
| 366 | + | }) | |
| 367 | + | wg.Go(func() { | |
| 368 | + | d.paint(sess, ac, true) | |
| 369 | + | }) | |
| 370 | + | } | |
| 371 | + | wg.Wait() | |
| 372 | + | close(done) | |
| 373 | + | drain.Wait() | |
| 374 | + | } | |
| 375 | + | ||
| 343 | 376 | func windowCount(sess *session) int { | |
| 344 | 377 | sess.mu.Lock() | |
| 345 | 378 | defer sess.mu.Unlock() | |
| @@ -340,6 +340,39 @@ func TestCopyModeEscapeRestoresLiveFullRepaint(t *testing.T) { | |||||
| 340 | } | 340 | } | ||
| 341 | } | 341 | } | ||
| 342 | 342 | ||||
| 343 | + | func TestCopyModeEnterExitConcurrentWithPaintRace(t *testing.T) { | |||
| 344 | + | p, _ := newBlockingPTY(t) | |||
| 345 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |||
| 346 | + | sess.windows[0].scrollback = scopy.NewScrollback(4) | |||
| 347 | + | sess.windows[0].screen.Write([]byte("live")) | |||
| 348 | + | ||||
| 349 | + | done := make(chan struct{}) | |||
| 350 | + | var drain sync.WaitGroup | |||
| 351 | + | drain.Go(func() { | |||
| 352 | + | for { | |||
| 353 | + | select { | |||
| 354 | + | case <-sends: | |||
| 355 | + | case <-done: | |||
| 356 | + | return | |||
| 357 | + | } | |||
| 358 | + | } | |||
| 359 | + | }) | |||
| 360 | + | ||||
| 361 | + | var wg sync.WaitGroup | |||
| 362 | + | for range 100 { | |||
| 363 | + | wg.Go(func() { | |||
| 364 | + | d.enterCopyMode(sess, ac) | |||
| 365 | + | d.handleInput(sess, ac, []byte("q")) | |||
| 366 | + | }) | |||
| 367 | + | wg.Go(func() { | |||
| 368 | + | d.paint(sess, ac, true) | |||
| 369 | + | }) | |||
| 370 | + | } | |||
| 371 | + | wg.Wait() | |||
| 372 | + | close(done) | |||
| 373 | + | drain.Wait() | |||
| 374 | + | } | |||
| 375 | + | ||||
| 343 | func windowCount(sess *session) int { | 376 | func windowCount(sess *session) int { | ||
| 344 | sess.mu.Lock() | 377 | sess.mu.Lock() | ||
| 345 | defer sess.mu.Unlock() | 378 | defer sess.mu.Unlock() | ||