Skip to content

Commit 7c807df

Browse files
mcuelenaereclaude
andcommitted
fix(mdns): hold lock across SetOptions field update + restart
Cursorbot caught a real race I missed: SetOptions was acquiring the lock, updating fields, releasing the lock, then calling Restart() which re-acquires the same lock inside start(true). Between the unlock and the re-lock, a concurrent Stop() (e.g. SIGINT during a networkStateChanged) could shut the server down and set m.server = nil, after which the pending Restart() would build a fresh server — silently undoing the Stop(). Extract startLocked() as the body of start() assuming the caller already holds m.lock, and have SetOptions hold the lock continuously across the field update and the restart. Public Start() / Restart() still go through start() which acquires the lock as before. Cursor: #1454 (comment) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 846b975 commit 7c807df

1 file changed

Lines changed: 14 additions & 5 deletions

File tree

internal/mdns/mdns.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,14 @@ func normalizeLocalNames(in []string) []string {
214214
func (m *MDNS) start(allowRestart bool) error {
215215
m.lock.Lock()
216216
defer m.lock.Unlock()
217+
return m.startLocked(allowRestart)
218+
}
217219

220+
// startLocked is the body of start; the caller must already hold m.lock.
221+
// SetOptions uses this so it can update the fields and (re)start the
222+
// server under a single continuous lock acquisition, preventing a
223+
// concurrent Stop() from squeezing in between.
224+
func (m *MDNS) startLocked(allowRestart bool) error {
218225
if m.server != nil {
219226
if !allowRestart {
220227
return fmt.Errorf("mDNS server already running")
@@ -327,16 +334,18 @@ func (m *MDNS) Stop() error {
327334
}
328335

329336
// SetOptions sets the local names, listen options, and service, then
330-
// restarts the mDNS server. All three fields are updated under a
331-
// single lock acquisition so a concurrent restart can't observe a
332-
// partially-applied configuration.
337+
// restarts the mDNS server. Field updates and the restart happen
338+
// under a single lock acquisition so that (a) a concurrent restart
339+
// can't observe a partially-applied configuration and (b) a
340+
// concurrent Stop() can't slip in between the update and the
341+
// restart and be silently undone.
333342
func (m *MDNS) SetOptions(options *MDNSOptions) error {
334343
m.lock.Lock()
344+
defer m.lock.Unlock()
335345
m.localNames = options.LocalNames
336346
if options.ListenOptions != nil {
337347
m.listenOptions = options.ListenOptions
338348
}
339349
m.service = options.Service
340-
m.lock.Unlock()
341-
return m.Restart()
350+
return m.startLocked(true)
342351
}

0 commit comments

Comments
 (0)