dns: release lock before extra-records channel send

A blocked send held the write lock and leaked the watcher at shutdown.
This commit is contained in:
Kristoffer Dalby
2026-06-05 15:19:27 +00:00
committed by Kristoffer Dalby
parent e413919810
commit 9f0c74e73a
2 changed files with 57 additions and 2 deletions
+12 -2
View File
@@ -158,11 +158,12 @@ func (e *ExtraRecordsMan) updateRecords() {
}
e.mu.Lock()
defer e.mu.Unlock()
// If there has not been any change, ignore the update.
if oldHash, ok := e.hashes[e.path]; ok {
if newHash == oldHash {
e.mu.Unlock()
return
}
}
@@ -171,10 +172,19 @@ func (e *ExtraRecordsMan) updateRecords() {
e.records = set.SetOf(records)
e.hashes[e.path] = newHash
toSend := e.records.Slice()
log.Trace().Caller().Interface("records", e.records).Msgf("extra records updated from path, count old: %d, new: %d", oldCount, e.records.Len())
e.updateCh <- e.records.Slice()
// Release the lock before the (potentially blocking) send so a slow or
// absent consumer cannot stall Records() readers, and abort the send on
// shutdown instead of leaking this goroutine on the closed-down channel.
e.mu.Unlock()
select {
case e.updateCh <- toSend:
case <-e.closeCh:
}
}
// readExtraRecordsFromPath reads a JSON file of [tailcfg.DNSRecord]
@@ -0,0 +1,45 @@
package dns
import (
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/require"
)
// TestUpdateRecordsDoesNotBlockShutdown ensures updateRecords does not park
// forever on the update channel during shutdown. The send must release the
// write lock first and abort on closeCh, otherwise the Run goroutine leaks and
// holds the lock indefinitely when no consumer is draining the channel.
func TestUpdateRecordsDoesNotBlockShutdown(t *testing.T) {
path := filepath.Join(t.TempDir(), "extra.json")
require.NoError(t, os.WriteFile(path,
[]byte(`[{"name":"a.example.com","type":"A","value":"100.64.0.1"}]`), 0o600))
er, err := NewExtraRecordsManager(path)
require.NoError(t, err)
defer er.watcher.Close()
// Change the file so updateRecords passes the unchanged-hash guard and
// reaches the send with no consumer draining UpdateCh.
require.NoError(t, os.WriteFile(path,
[]byte(`[{"name":"b.example.com","type":"A","value":"100.64.0.2"}]`), 0o600))
done := make(chan struct{})
go func() {
er.updateRecords()
close(done)
}()
er.Close()
select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("updateRecords parked on a blocking send and did not return after Close")
}
}