Use locking to solve data race. (#141)

* Use locking to solve data race.

* add more doc

* Update doc on WithHotReload

Co-authored-by: ktr <ktr@syfm.me>

* fix tests

Co-authored-by: ktr <ktr@syfm.me>

.gitignore

@@ -16,3 +16,4 @@
 coverage.txt
 fuzz.out
 _tools
+.idea

example/go.mod

@@ -2,6 +2,8 @@ module github.com/ktr0731/go-fuzzyfinder/example
 
 go 1.17
 
+replace github.com/ktr0731/go-fuzzyfinder => ../
+
 require (
 	github.com/ktr0731/go-fuzzyfinder v0.5.1
 	github.com/mattn/go-isatty v0.0.14

example/go.sum

@@ -6,8 +6,6 @@ github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
 github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
-github.com/ktr0731/go-fuzzyfinder v0.5.1 h1:rDcWxmGi6ux4NURekn9iAXpbYBp8Kj4cznrz162S9og=
-github.com/ktr0731/go-fuzzyfinder v0.5.1/go.mod h1:gud27uRG2vF+oD58eGhYZj7Pc9enRX0qecwp09w/jno=
 github.com/lucasb-eyer/go-colorful v1.0.3 h1:QIbQXiugsb+q10B+MI+7DI1oQLdmnep86tWFlaaUAac=
 github.com/lucasb-eyer/go-colorful v1.0.3/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
 github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=

example/hotreload/main.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"sync"
+	"time"
 
 	fuzzyfinder "github.com/ktr0731/go-fuzzyfinder"
 	isatty "github.com/mattn/go-isatty"
@@ -19,10 +21,14 @@ func main() {
 		return
 	}
 	var slice []string
+	var mut sync.RWMutex
 	go func(slice *[]string) {
 		s := bufio.NewScanner(os.Stdin)
 		for s.Scan() {
+			mut.Lock()
 			*slice = append(*slice, s.Text())
+			mut.Unlock()
+			time.Sleep(50 * time.Millisecond) // to give a feeling of how it looks like in the terminal
 		}
 	}(&slice)
 
@@ -31,7 +37,7 @@ func main() {
 		func(i int) string {
 			return slice[i]
 		},
-		fuzzyfinder.WithHotReload(),
+		fuzzyfinder.WithHotReloadLock(mut.RLocker()),
 	)
 	if err != nil {
 		log.Fatal(err)

fuzzyfinder.go

@@ -100,8 +100,10 @@ func (f *finder) initFinder(items []string, matched []matching.Matched, opt opt)
 
 	if !isInTesting() {
 		f.drawTimer = time.AfterFunc(0, func() {
+			f.stateMu.Lock()
 			f._draw()
 			f._drawPreview()
+			f.stateMu.Unlock()
 			f.term.Show()
 		})
 		f.drawTimer.Stop()
@@ -615,8 +617,10 @@ func (f *finder) find(slice interface{}, itemFunc func(i int) string, opts []Opt
 
 	inited := make(chan struct{})
 	if opt.hotReload && rv.Kind() == reflect.Ptr {
+		opt.hotReloadLock.Lock()
 		rvv := reflect.Indirect(rv)
 		items, matched = makeItems(rvv.Len())
+		opt.hotReloadLock.Unlock()
 
 		go func() {
 			<-inited
@@ -627,11 +631,13 @@ func (f *finder) find(slice interface{}, itemFunc func(i int) string, opts []Opt
 				case <-ctx.Done():
 					return
 				case <-time.After(30 * time.Millisecond):
+					opt.hotReloadLock.Lock()
 					curr := rvv.Len()
 					if prev != curr {
 						items, matched = makeItems(curr)
 						f.updateItems(items, matched)
 					}
+					opt.hotReloadLock.Unlock()
 					prev = curr
 				}
 			}

fuzzyfinder_test.go

@@ -260,6 +260,40 @@ func TestFind_hotReload(t *testing.T) {
 	})
 }
 
+func TestFind_hotReloadLock(t *testing.T) {
+	f, term := fuzzyfinder.NewWithMockedTerminal()
+	events := append(runes("adrena"), keys(input{tcell.KeyEsc, rune(tcell.KeyEsc), tcell.ModNone})...)
+	term.SetEventsV2(events...)
+
+	var mu sync.RWMutex
+	assertWithGolden(t, func(t *testing.T) string {
+		_, err := f.Find(
+			&tracks,
+			func(i int) string {
+				return tracks[i].Name
+			},
+			fuzzyfinder.WithPreviewWindow(func(i, width, height int) string {
+				// Hack, wait until updateItems is called.
+				time.Sleep(50 * time.Millisecond)
+				mu.RLock()
+				defer mu.RUnlock()
+				if i == -1 {
+					return "not found"
+				}
+				return "Name: " + tracks[i].Name + "\nArtist: " + tracks[i].Artist
+			}),
+			fuzzyfinder.WithMode(fuzzyfinder.ModeCaseSensitive),
+			fuzzyfinder.WithHotReloadLock(mu.RLocker()),
+		)
+		if !errors.Is(err, fuzzyfinder.ErrAbort) {
+			t.Fatalf("Find must return ErrAbort, but got '%s'", err)
+		}
+
+		res := term.GetResult()
+		return res
+	})
+}
+
 func TestFind_enter(t *testing.T) {
 	cases := map[string]struct {
 		events   []tcell.Event

option.go

@@ -1,13 +1,16 @@
 package fuzzyfinder
 
+import "sync"
+
 type opt struct {
-	mode         mode
-	previewFunc  func(i, width, height int) string
-	multi        bool
-	hotReload    bool
-	promptString string
-	header       string
-	beginAtTop   bool
+	mode          mode
+	previewFunc   func(i, width, height int) string
+	multi         bool
+	hotReload     bool
+	hotReloadLock sync.Locker
+	promptString  string
+	header        string
+	beginAtTop    bool
 }
 
 type mode int
@@ -25,6 +28,7 @@ const (
 
 var defaultOption = opt{
 	promptString: "> ",
+	hotReloadLock: &sync.Mutex{}, // this won't resolve the race condition but avoid nil panic
 }
 
 // Option represents available fuzzy-finding options.
@@ -53,12 +57,26 @@ func WithPreviewWindow(f func(i, width, height int) string) Option {
 
 // WithHotReload reloads the passed slice automatically when some entries are appended.
 // The caller must pass a pointer of the slice instead of the slice itself.
+//
+// Deprecated: use WithHotReloadLock instead.
 func WithHotReload() Option {
 	return func(o *opt) {
 		o.hotReload = true
 	}
 }
 
+// WithHotReloadLock reloads the passed slice automatically when some entries are appended.
+// The caller must pass a pointer of the slice instead of the slice itself.
+// The caller must pass a RLock which is used to synchronize access to the slice.
+// The caller MUST NOT lock in the itemFunc passed to Find / FindMulti because it will be locked by the fuzzyfinder.
+// If used together with WithPreviewWindow, the caller MUST use the RLock only in the previewFunc passed to WithPreviewWindow. 
+func WithHotReloadLock(lock sync.Locker) Option {
+	return func(o *opt) {
+		o.hotReload = true
+		o.hotReloadLock = lock
+	}
+}
+
 type cursorPosition int
 
 const (

testdata/fixtures/testfind_hotreloadlock.golden

@@ -0,0 +1,11 @@
+                              ┌────────────────────────────┐
+                              │ Name: adrenaline!!!        │
+                              │ Artist: TrySail            │
+                              │                            │
+                              │                            │
+                              │                            │
+                              │                            │
+> adrenaline!!!               │                            │
+  1/9                         │                            │
+> adrena█                     └────────────────────────────┘
+