Merge pull request #607 from peco/fix-resume-deadlock

Remove the possibility of Resume() deadlocking

action.go

@@ -369,7 +369,7 @@ func doFinish(ctx context.Context, state *Peco, _ Event) {
 	state.screen.Suspend()
 
 	err = cmd.Run()
-	state.screen.Resume()
+	state.screen.Resume(ctx)
 	state.Hub().SendDraw(ctx, &DrawOptions{DisableCache: true})
 	if err != nil {
 		// bail out, or otherwise the user cannot know what happened

interface.go

@@ -180,7 +180,7 @@ type Screen interface {
 	Flush() error
 	PollEvent(context.Context, *Config) chan Event
 	Print(PrintArgs) int
-	Resume()
+	Resume(context.Context)
 	SetCell(int, int, rune, Attribute, Attribute)
 	SetCursor(int, int)
 	Size() (int, int)

peco_test.go

@@ -244,7 +244,7 @@ func (s *SimScreen) Size() (int, int) {
 	return s.screen.Size()
 }
 
-func (s *SimScreen) Resume()  {}
+func (s *SimScreen) Resume(_ context.Context) {}
 func (s *SimScreen) Suspend() {}
 
 // Sync records a "Sync" event via the interceptor. This satisfies the

screen.go

@@ -301,17 +301,26 @@ func (t *Termbox) Suspend() {
 	}
 }
 
-func (t *Termbox) Resume() {
+func (t *Termbox) Resume(ctx context.Context) {
 	// Resume must be a block operation, because we can't safely proceed
 	// without actually knowing that the screen has been re-initialized.
-	// So we send a channel where we expect a reply back, and wait for that
+	// So we send a channel where we expect a reply back, and wait for that.
+	//
+	// Both selects are guarded by ctx.Done() to avoid deadlock: if the
+	// polling goroutine is not yet waiting on resumeCh, a non-blocking
+	// send would silently drop the message and the subsequent receive
+	// would block forever.
 	ch := make(chan struct{})
 	select {
 	case t.resumeCh <- ch:
-	default:
+	case <-ctx.Done():
+		return
 	}
 
-	<-ch
+	select {
+	case <-ch:
+	case <-ctx.Done():
+	}
 }
 
 // SetCell writes to the terminal

screen_inline.go

@@ -217,4 +217,4 @@ func (s *InlineScreen) SendEvent(_ Event) {}
 func (s *InlineScreen) Suspend() {}
 
 // Resume is a no-op for inline mode.
-func (s *InlineScreen) Resume() {}
+func (s *InlineScreen) Resume(_ context.Context) {}

screen_test.go

@@ -0,0 +1,115 @@
+package peco
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestTermboxResumeNoDeadlock(t *testing.T) {
+	tb := NewTermbox()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	// Simulate the polling goroutine: receive from resumeCh after a short delay,
+	// then close the reply channel (as PollEvent does after re-init).
+	go func() {
+		time.Sleep(50 * time.Millisecond)
+		replyCh := <-tb.resumeCh
+		close(replyCh)
+	}()
+
+	done := make(chan struct{})
+	go func() {
+		tb.Resume(ctx)
+		close(done)
+	}()
+
+	select {
+	case <-done:
+		// Resume completed without deadlock.
+	case <-time.After(2 * time.Second):
+		t.Fatal("Resume() deadlocked")
+	}
+}
+
+func TestTermboxResumeDoesNotDropSend(t *testing.T) {
+	tb := NewTermbox()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	received := make(chan struct{})
+	go func() {
+		replyCh := <-tb.resumeCh
+		close(received)
+		close(replyCh)
+	}()
+
+	tb.Resume(ctx)
+
+	select {
+	case <-received:
+		// The receiver goroutine got the message.
+	default:
+		t.Fatal("receiver did not get the resume message")
+	}
+}
+
+func TestTermboxResumeContextCancelled(t *testing.T) {
+	tb := NewTermbox()
+
+	ctx, cancel := context.WithCancel(context.Background())
+
+	// Cancel immediately so Resume cannot deliver on resumeCh.
+	cancel()
+
+	done := make(chan struct{})
+	go func() {
+		tb.Resume(ctx)
+		close(done)
+	}()
+
+	select {
+	case <-done:
+		// Resume returned promptly after context cancellation.
+	case <-time.After(2 * time.Second):
+		t.Fatal("Resume() did not unblock after context cancellation")
+	}
+}
+
+func TestTermboxResumeContextCancelledWhileWaitingForReply(t *testing.T) {
+	tb := NewTermbox()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// Accept the resume request but never close the reply channel.
+	// This tests that the second select also respects ctx.Done().
+	go func() {
+		<-tb.resumeCh // receive but don't close replyCh
+	}()
+
+	done := make(chan struct{})
+	go func() {
+		tb.Resume(ctx)
+		close(done)
+	}()
+
+	// Give Resume time to pass the first select and block on the second.
+	time.Sleep(50 * time.Millisecond)
+	cancel()
+
+	select {
+	case <-done:
+		// Resume returned after context cancellation during reply wait.
+	case <-time.After(2 * time.Second):
+		t.Fatal("Resume() did not unblock after context cancellation while waiting for reply")
+	}
+
+	// Verify context was indeed cancelled.
+	require.Error(t, ctx.Err())
+}