Skip to content

Commit 38e10d5

Browse files
BagToadCopilot
authored and
williammartin
committed
fix(huh prompter): use synchronized accessors to eliminate data race
Replace Value() pointer bindings with syncAccessor in MultiSelectWithSearch. huh's OptionsFunc runs in a goroutine while the main event loop writes field values, causing a data race on shared variables. syncAccessor implements huh's Accessor interface with a shared mutex, ensuring all reads and writes are synchronized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 95a59f4 commit 38e10d5

2 files changed

Lines changed: 56 additions & 20 deletions

File tree

‎internal/prompter/huh_prompter.go‎

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package prompter
33
import (
44
"fmt"
55
"slices"
6+
"sync"
67

78
"charm.land/huh/v2"
89
"github.com/cli/cli/v2/internal/ghinstance"
@@ -101,12 +102,35 @@ type searchOptionsBinding struct {
101102
Selected *[]string
102103
}
103104

104-
func (p *huhPrompter) buildMultiSelectWithSearchForm(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) (*huh.Form, *[]string) {
105-
selectedValues := make([]string, len(defaultValues))
106-
copy(selectedValues, defaultValues)
105+
// syncAccessor is a thread-safe huh.Accessor implementation.
106+
// huh calls OptionsFunc from a goroutine while the main event loop
107+
// writes field values via Set(). This accessor synchronizes both
108+
// paths through the same mutex.
109+
type syncAccessor[T any] struct {
110+
mu *sync.Mutex
111+
value T
112+
}
113+
114+
func (a *syncAccessor[T]) Get() T {
115+
a.mu.Lock()
116+
defer a.mu.Unlock()
117+
return a.value
118+
}
119+
120+
func (a *syncAccessor[T]) Set(value T) {
121+
a.mu.Lock()
122+
defer a.mu.Unlock()
123+
a.value = value
124+
}
125+
126+
func (p *huhPrompter) buildMultiSelectWithSearchForm(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) (*huh.Form, *syncAccessor[[]string]) {
127+
var mu sync.Mutex
128+
129+
queryAccessor := &syncAccessor[string]{mu: &mu}
130+
selectAccessor := &syncAccessor[[]string]{mu: &mu, value: slices.Clone(defaultValues)}
107131

108132
optionKeyLabels := make(map[string]string)
109-
for _, k := range selectedValues {
133+
for _, k := range defaultValues {
110134
optionKeyLabels[k] = k
111135
}
112136

@@ -117,12 +141,25 @@ func (p *huhPrompter) buildMultiSelectWithSearchForm(prompt, searchPrompt string
117141
var cachedSearchQuery string
118142
var cachedSearchResult MultiSelectSearchResult
119143

120-
buildOptions := func(query string) []huh.Option[string] {
121-
if !searchCacheValid || query != cachedSearchQuery {
122-
cachedSearchResult = searchFunc(query)
144+
buildOptions := func() []huh.Option[string] {
145+
mu.Lock()
146+
query := queryAccessor.value
147+
needsFetch := !searchCacheValid || query != cachedSearchQuery
148+
mu.Unlock()
149+
150+
if needsFetch {
151+
result := searchFunc(query)
152+
mu.Lock()
153+
cachedSearchResult = result
123154
cachedSearchQuery = query
124155
searchCacheValid = true
156+
mu.Unlock()
125157
}
158+
159+
mu.Lock()
160+
defer mu.Unlock()
161+
162+
selectedValues := selectAccessor.value
126163
result := cachedSearchResult
127164

128165
if result.Err != nil {
@@ -181,37 +218,36 @@ func (p *huhPrompter) buildMultiSelectWithSearchForm(prompt, searchPrompt string
181218
return formOptions
182219
}
183220

184-
var searchQuery string
185221
binding := &searchOptionsBinding{
186-
Query: &searchQuery,
187-
Selected: &selectedValues,
222+
Query: &queryAccessor.value,
223+
Selected: &selectAccessor.value,
188224
}
189225

190226
form := p.newForm(
191227
huh.NewGroup(
192228
huh.NewInput().
193229
Title(searchPrompt).
194-
Value(&searchQuery),
230+
Accessor(queryAccessor),
195231
huh.NewMultiSelect[string]().
196232
Title(prompt).
197-
Options(buildOptions("")...).
233+
Options(buildOptions()...).
198234
OptionsFunc(func() []huh.Option[string] {
199-
return buildOptions(searchQuery)
235+
return buildOptions()
200236
}, binding).
201-
Value(&selectedValues).
237+
Accessor(selectAccessor).
202238
Limit(0),
203239
),
204240
)
205-
return form, &selectedValues
241+
return form, selectAccessor
206242
}
207243

208244
func (p *huhPrompter) MultiSelectWithSearch(prompt, searchPrompt string, defaultValues, persistentValues []string, searchFunc func(string) MultiSelectSearchResult) ([]string, error) {
209-
form, result := p.buildMultiSelectWithSearchForm(prompt, searchPrompt, defaultValues, persistentValues, searchFunc)
245+
form, accessor := p.buildMultiSelectWithSearchForm(prompt, searchPrompt, defaultValues, persistentValues, searchFunc)
210246
err := form.Run()
211247
if err != nil {
212248
return nil, err
213249
}
214-
return *result, nil
250+
return accessor.Get(), nil
215251
}
216252

217253
func (p *huhPrompter) buildInputForm(prompt, defaultValue string) (*huh.Form, *string) {

‎internal/prompter/huh_prompter_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ func TestHuhPrompterMultiSelectWithSearch(t *testing.T) {
450450
"Select", "Search", tt.defaults, tt.persistent, staticSearchFunc,
451451
)
452452
runForm(t, f, tt.ix)
453-
assert.Equal(t, tt.wantResult, *result)
453+
assert.Equal(t, tt.wantResult, result.Get())
454454
})
455455
}
456456
}
@@ -482,7 +482,7 @@ func TestHuhPrompterMultiSelectWithSearchPersistence(t *testing.T) {
482482
tab(), waitForOptions(),
483483
enter(), // submit — result-a should persist
484484
))
485-
assert.Equal(t, []string{"result-a"}, *result)
485+
assert.Equal(t, []string{"result-a"}, result.Get())
486486
})
487487
t.Run("empty search results shows no-results placeholder", func(t *testing.T) {
488488
emptySearchFunc := func(query string) MultiSelectSearchResult {
@@ -495,7 +495,7 @@ func TestHuhPrompterMultiSelectWithSearchPersistence(t *testing.T) {
495495
// With no results, the "No results" placeholder is shown but nothing
496496
// is selected, so submitting returns empty.
497497
runForm(t, f, newInteraction(tab(), waitForOptions(), toggle(), enter()))
498-
assert.Equal(t, []string{""}, *result)
498+
assert.Equal(t, []string{""}, result.Get())
499499
})
500500
}
501501

0 commit comments

Comments
 (0)