state: GetSnapshot copies the state's maps (closes #261) #272

Merged
clawbot merged 1 commits from fix/261-snapshot-map-copies into next 2026-10-13 13:44:32 +02:00
3 changed files with 75 additions and 10 deletions
+2
View File
@@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
# Completed Steps
- 2026-10-13: a check saving the state while the dashboard or `/api/v1/status`
is read cannot crash the process (closes #261).
- 2026-10-06: canonical files re-fetched from `sneak/prompts` at `dd4027b`; lint
and test are phases of the `Dockerfile`, every scripted build uncached (closes
#257).
+12 -2
View File
@@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"log/slog"
"maps"
"os"
"path/filepath"
"slices"
@@ -270,12 +271,21 @@ func (s *State) Save() error {
return nil
}
// GetSnapshot returns a copy of the current snapshot.
// GetSnapshot returns a copy of the current snapshot, with copies of
// its maps, so that a caller can range over them while the state is
// written. The entries in the maps are not copied: an entry saved in the
// state must be replaced, never changed in place.
func (s *State) GetSnapshot() Snapshot {
s.mu.RLock()
defer s.mu.RUnlock()
return *s.snapshot
snap := *s.snapshot
snap.Domains = maps.Clone(snap.Domains)
snap.Hostnames = maps.Clone(snap.Hostnames)
snap.Ports = maps.Clone(snap.Ports)
snap.Certificates = maps.Clone(snap.Certificates)
return snap
}
// SetDomainState updates the state for a domain.
+61 -8
View File
@@ -995,7 +995,8 @@ func TestPortStateUnmarshalJSON_BothFormats(t *testing.T) {
}
}
// TestGetSnapshot_ReturnsCopy verifies that GetSnapshot returns a value copy.
// TestGetSnapshot_ReturnsCopy verifies that the maps in the snapshot
// GetSnapshot returns are not the state's own.
func TestGetSnapshot_ReturnsCopy(t *testing.T) {
t.Parallel()
@@ -1005,15 +1006,67 @@ func TestGetSnapshot_ReturnsCopy(t *testing.T) {
snap := s.GetSnapshot()
// Mutate the returned snapshot's map.
snap.Domains["mutated.example.com"] = &state.DomainState{
Nameservers: []string{"ns1.mutated.com."},
const added = "added.example.com"
snap.Domains[added] = &state.DomainState{}
snap.Hostnames[added] = &state.HostnameState{}
snap.Ports[added] = &state.PortState{}
snap.Certificates[added] = &state.CertificateState{}
if _, ok := s.GetDomainState(added); ok {
t.Error("the snapshot's Domains map is the state's own")
}
// The original domain data should still be accessible.
_, ok := s.GetDomainState("example.com")
if !ok {
t.Error("original domain should still exist")
if _, ok := s.GetHostnameState(added); ok {
t.Error("the snapshot's Hostnames map is the state's own")
}
if _, ok := s.GetPortState(added); ok {
t.Error("the snapshot's Ports map is the state's own")
}
if _, ok := s.GetCertificateState(added); ok {
t.Error("the snapshot's Certificates map is the state's own")
}
}
// TestGetSnapshot_WhileWritten ranges over snapshots while another
// goroutine writes the state, as the dashboard does while a check runs.
// Under -race it fails when a snapshot's maps are the state's own.
func TestGetSnapshot_WhileWritten(t *testing.T) {
t.Parallel()
s := state.NewForTestWithDataDir(t.TempDir())
const writes = 1000
written := make(chan struct{})
go func() {
defer close(written)
for range writes {
s.SetDomainState("example.com", &state.DomainState{})
s.SetHostnameState(testHostname, &state.HostnameState{})
s.SetPortState(testIPv4+":443", &state.PortState{})
s.SetCertificateState(
testIPv4+":443:"+testHostname, &state.CertificateState{},
)
}
}()
for {
select {
case <-written:
return
default:
}
// json.Marshal ranges over each of the snapshot's maps.
_, err := json.Marshal(s.GetSnapshot())
if err != nil {
t.Fatal(err)
}
}
}