From 02cb1ec5806951da5655233b4b14fcd45edf5327 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Tue, 13 Oct 2026 13:44:31 +0200
Subject: [PATCH] state: GetSnapshot copies the state's maps (closes #261)

The dashboard and /api/v1/status range over the maps in the snapshot
GetSnapshot returns after the read lock is released, while the watcher
writes the same maps under the lock. Go aborts the process when a write
lands during such a range. GetSnapshot now returns copies of the four
maps.

The entries in the maps are still shared with the state. That is safe
because the watcher replaces an entry with a Set call and never changes
one in place; the comment on GetSnapshot says so.

TestGetSnapshot_WhileWritten ranges over snapshots while another
goroutine writes the state and fails under -race without the copies.
TestGetSnapshot_ReturnsCopy now checks that each map is not the state's
own.

Model: opus-5-5
---
 TODO.md                      |  2 ++
 internal/state/state.go      | 14 ++++++--
 internal/state/state_test.go | 69 +++++++++++++++++++++++++++++++-----
 3 files changed, 75 insertions(+), 10 deletions(-)

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)
+		}
 	}
 }