From c376ce3ef18cc26bbf1f6338cc8518ae329a18d6 Mon Sep 17 00:00:00 2001
From: DryHumour <DryHumour@users.noreply.github.com>
Date: Wed, 30 Apr 2025 10:45:41 -0400
Subject: [PATCH] fix(cursor): fix data race on blinkTag (#784)

---
 cursor/cursor.go      |  4 +++-
 cursor/cursor_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 cursor/cursor_test.go

diff --git a/cursor/cursor.go b/cursor/cursor.go
index d101332..155d56c 100644
--- a/cursor/cursor.go
+++ b/cursor/cursor.go
@@ -173,11 +173,13 @@ func (m *Model) BlinkCmd() tea.Cmd {
 
 	m.blinkTag++
 
+	blinkMsg := BlinkMsg{id: m.id, tag: m.blinkTag}
+
 	return func() tea.Msg {
 		defer cancel()
 		<-ctx.Done()
 		if ctx.Err() == context.DeadlineExceeded {
-			return BlinkMsg{id: m.id, tag: m.blinkTag}
+			return blinkMsg
 		}
 		return blinkCanceled{}
 	}
diff --git a/cursor/cursor_test.go b/cursor/cursor_test.go
new file mode 100644
index 0000000..c526c4a
--- /dev/null
+++ b/cursor/cursor_test.go
@@ -0,0 +1,50 @@
+package cursor
+
+import (
+	"sync"
+	"testing"
+	"time"
+)
+
+// TestBlinkCmdDataRace tests for a race on [Cursor.blinkTag].
+//
+// The original [Model.BlinkCmd] implementation returned a closure over the pointer receiver:
+//
+//	return func() tea.Msg {
+//		defer cancel()
+//		<-ctx.Done()
+//		if ctx.Err() == context.DeadlineExceeded {
+//			return BlinkMsg{id: m.id, tag: m.blinkTag}
+//		}
+//		return blinkCanceled{}
+//	}
+//
+// A race on “m.blinkTag” will occur if:
+//  1. [Model.BlinkCmd] is called e.g. by calling [Model.Focus] from
+//     ["github.com/charmbracelet/bubbletea".Model.Update];
+//  2. ["github.com/charmbracelet/bubbletea".handleCommands] is kept sufficiently busy that it does not recieve and
+//     execute the [Model.BlinkCmd] e.g. by other long running command or commands;
+//  3. at least [Mode.BlinkSpeed] time elapses;
+//  4. [Model.BlinkCmd] is called again;
+//  5. ["github.com/charmbracelet/bubbletea".handleCommands] gets around to receiving and executing the original
+//     closure.
+//
+// Even if this did not formally race, the value of the tag fetched would be semantically incorrect (likely being the
+// current value rather than the value at the time the closure was created).
+func TestBlinkCmdDataRace(t *testing.T) {
+	m := New()
+	cmd := m.BlinkCmd()
+	var wg sync.WaitGroup
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		time.Sleep(m.BlinkSpeed * 3)
+		cmd()
+	}()
+	go func() {
+		defer wg.Done()
+		time.Sleep(m.BlinkSpeed * 2)
+		m.BlinkCmd()
+	}()
+	wg.Wait()
+}