Merge pull request #754 from peco/fix-selection-copy

prevent deadlock in Selection.Copy

selection/selection.go

@@ -37,11 +37,24 @@ func (s *Set) Add(l line.Line) {
 }
 
 // Copy copies all selected lines from s into dst.
+//
+// Items are collected under the source read lock first, then added to dst
+// after releasing that lock. This avoids deadlocks when dst == s and
+// prevents ABBA deadlocks during concurrent bidirectional copies.
 func (s *Set) Copy(dst *Set) {
-	s.Ascend(func(l line.Line) bool {
-		dst.Add(l)
+	s.mutex.RLock()
+	items := make([]line.Line, 0, s.tree.Len())
+	s.tree.Ascend(func(it btree.Item) bool {
+		if l, ok := it.(line.Line); ok {
+			items = append(items, l)
+		}
 		return true
 	})
+	s.mutex.RUnlock()
+
+	for _, l := range items {
+		dst.Add(l)
+	}
 }
 
 // Remove removes the specified line from the selection.

selection/selection_test.go

@@ -3,6 +3,7 @@ package selection
 import (
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/peco/peco/line"
 	"github.com/stretchr/testify/require"
@@ -110,3 +111,52 @@ func TestRangeStart(t *testing.T) {
 	rs.Reset()
 	require.False(t, rs.Valid())
 }
+
+func TestCopySelf(t *testing.T) {
+	s := New()
+	s.Add(line.NewRaw(0, "Alice", false, false))
+	s.Add(line.NewRaw(1, "Bob", false, false))
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		s.Copy(s)
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		t.Fatal("Copy(self) deadlocked")
+	}
+
+	require.Equal(t, 2, s.Len())
+}
+
+func TestCopyCrossNoDeadlock(t *testing.T) {
+	a := New()
+	b := New()
+	a.Add(line.NewRaw(0, "Alice", false, false))
+	b.Add(line.NewRaw(1, "Bob", false, false))
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		var wg sync.WaitGroup
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			a.Copy(b)
+		}()
+		go func() {
+			defer wg.Done()
+			b.Copy(a)
+		}()
+		wg.Wait()
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		t.Fatal("cross-Copy deadlocked")
+	}
+}