diff --git a/TODO.md b/TODO.md index 2f43241..5c3c05e 100644 --- a/TODO.md +++ b/TODO.md @@ -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). diff --git a/internal/state/state.go b/internal/state/state.go index 39da3cb..d8348bb 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -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. diff --git a/internal/state/state_test.go b/internal/state/state_test.go index e511014..ce65140 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -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) + } } }