From dceaa618a4ae533352e80327c74b5a1c92adca75 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Mon, 23 Feb 2026 22:51:44 +0200 Subject: task 304: harden snapshot immutability after sub-agent review --- internal/statsengine/snapshot.go | 91 ++++++++++++++++++++++++++--- internal/statsengine/snapshot_test.go | 105 ++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 internal/statsengine/snapshot_test.go (limited to 'internal/statsengine') diff --git a/internal/statsengine/snapshot.go b/internal/statsengine/snapshot.go index 3534731..898b7f1 100644 --- a/internal/statsengine/snapshot.go +++ b/internal/statsengine/snapshot.go @@ -2,6 +2,7 @@ package statsengine import ( "ior/internal/types" + "slices" "time" ) @@ -44,13 +45,13 @@ type Snapshot struct { GapTrend Trend ThroughputTrend Trend - LatencySeriesNs []float64 - GapSeriesNs []float64 - ThroughputSeriesB []float64 + latencySeriesNs []float64 + gapSeriesNs []float64 + throughputSeriesB []float64 - Syscalls []SyscallSnapshot - Files []FileSnapshot - Processes []ProcessSnapshot + syscalls []SyscallSnapshot + files []FileSnapshot + processes []ProcessSnapshot LatencyHistogram HistogramSnapshot GapHistogram HistogramSnapshot @@ -109,5 +110,81 @@ type HistogramBucketSnapshot struct { // HistogramSnapshot is an immutable histogram view at snapshot time. type HistogramSnapshot struct { Total uint64 - Buckets []HistogramBucketSnapshot + buckets []HistogramBucketSnapshot +} + +// NewSnapshot creates a snapshot while defensively copying all slice-backed +// inputs so callers cannot mutate shared snapshot state. +func NewSnapshot( + latencySeriesNs []float64, + gapSeriesNs []float64, + throughputSeriesB []float64, + syscalls []SyscallSnapshot, + files []FileSnapshot, + processes []ProcessSnapshot, + latencyHistogram HistogramSnapshot, + gapHistogram HistogramSnapshot, +) Snapshot { + return Snapshot{ + latencySeriesNs: slices.Clone(latencySeriesNs), + gapSeriesNs: slices.Clone(gapSeriesNs), + throughputSeriesB: slices.Clone(throughputSeriesB), + syscalls: slices.Clone(syscalls), + files: slices.Clone(files), + processes: slices.Clone(processes), + LatencyHistogram: latencyHistogram.Clone(), + GapHistogram: gapHistogram.Clone(), + } +} + +// NewHistogramSnapshot creates an immutable histogram snapshot by copying +// bucket storage. +func NewHistogramSnapshot(total uint64, buckets []HistogramBucketSnapshot) HistogramSnapshot { + return HistogramSnapshot{ + Total: total, + buckets: slices.Clone(buckets), + } +} + +// Clone returns a deep copy of the histogram snapshot. +func (h HistogramSnapshot) Clone() HistogramSnapshot { + return HistogramSnapshot{ + Total: h.Total, + buckets: slices.Clone(h.buckets), + } +} + +// LatencySeriesNs returns a defensive copy of latency sparkline samples. +func (s Snapshot) LatencySeriesNs() []float64 { + return slices.Clone(s.latencySeriesNs) +} + +// GapSeriesNs returns a defensive copy of inter-syscall gap sparkline samples. +func (s Snapshot) GapSeriesNs() []float64 { + return slices.Clone(s.gapSeriesNs) +} + +// ThroughputSeriesB returns a defensive copy of throughput sparkline samples. +func (s Snapshot) ThroughputSeriesB() []float64 { + return slices.Clone(s.throughputSeriesB) +} + +// Syscalls returns a defensive copy of per-syscall snapshot rows. +func (s Snapshot) Syscalls() []SyscallSnapshot { + return slices.Clone(s.syscalls) +} + +// Files returns a defensive copy of per-file snapshot rows. +func (s Snapshot) Files() []FileSnapshot { + return slices.Clone(s.files) +} + +// Processes returns a defensive copy of per-process snapshot rows. +func (s Snapshot) Processes() []ProcessSnapshot { + return slices.Clone(s.processes) +} + +// Buckets returns a defensive copy of histogram buckets. +func (h HistogramSnapshot) Buckets() []HistogramBucketSnapshot { + return slices.Clone(h.buckets) } diff --git a/internal/statsengine/snapshot_test.go b/internal/statsengine/snapshot_test.go new file mode 100644 index 0000000..065eded --- /dev/null +++ b/internal/statsengine/snapshot_test.go @@ -0,0 +1,105 @@ +package statsengine + +import "testing" + +func TestNewSnapshotDefensivelyCopiesSlices(t *testing.T) { + latency := []float64{1, 2, 3} + gap := []float64{4, 5, 6} + throughput := []float64{7, 8, 9} + syscalls := []SyscallSnapshot{{Name: "read", Count: 1}} + files := []FileSnapshot{{Path: "/tmp/a", Accesses: 2}} + processes := []ProcessSnapshot{{PID: 10, Comm: "cmd"}} + latencyBuckets := []HistogramBucketSnapshot{{Label: "[0,1)", Count: 3}} + gapBuckets := []HistogramBucketSnapshot{{Label: "[1,10)", Count: 4}} + + s := NewSnapshot( + latency, + gap, + throughput, + syscalls, + files, + processes, + NewHistogramSnapshot(3, latencyBuckets), + NewHistogramSnapshot(4, gapBuckets), + ) + + latency[0] = 99 + gap[0] = 99 + throughput[0] = 99 + syscalls[0].Name = "write" + files[0].Path = "/tmp/b" + processes[0].Comm = "mutated" + latencyBuckets[0].Count = 99 + gapBuckets[0].Count = 99 + + if got := s.LatencySeriesNs()[0]; got != 1 { + t.Fatalf("latency mutated through input slice: got %v", got) + } + if got := s.GapSeriesNs()[0]; got != 4 { + t.Fatalf("gap mutated through input slice: got %v", got) + } + if got := s.ThroughputSeriesB()[0]; got != 7 { + t.Fatalf("throughput mutated through input slice: got %v", got) + } + if got := s.Syscalls()[0].Name; got != "read" { + t.Fatalf("syscalls mutated through input slice: got %q", got) + } + if got := s.Files()[0].Path; got != "/tmp/a" { + t.Fatalf("files mutated through input slice: got %q", got) + } + if got := s.Processes()[0].Comm; got != "cmd" { + t.Fatalf("processes mutated through input slice: got %q", got) + } + if got := s.LatencyHistogram.Buckets()[0].Count; got != 3 { + t.Fatalf("latency histogram mutated through input slice: got %d", got) + } + if got := s.GapHistogram.Buckets()[0].Count; got != 4 { + t.Fatalf("gap histogram mutated through input slice: got %d", got) + } +} + +func TestSnapshotAccessorsReturnCopies(t *testing.T) { + s := NewSnapshot( + []float64{1}, + []float64{2}, + []float64{3}, + []SyscallSnapshot{{Name: "read"}}, + []FileSnapshot{{Path: "/tmp/a"}}, + []ProcessSnapshot{{Comm: "cmd"}}, + NewHistogramSnapshot(1, []HistogramBucketSnapshot{{Label: "a", Count: 1}}), + NewHistogramSnapshot(1, []HistogramBucketSnapshot{{Label: "b", Count: 1}}), + ) + + lat := s.LatencySeriesNs() + lat[0] = 100 + if got := s.LatencySeriesNs()[0]; got != 1 { + t.Fatalf("latency accessor leaked mutability: got %v", got) + } + + syscalls := s.Syscalls() + syscalls[0].Name = "write" + if got := s.Syscalls()[0].Name; got != "read" { + t.Fatalf("syscalls accessor leaked mutability: got %q", got) + } + + buckets := s.LatencyHistogram.Buckets() + buckets[0].Count = 99 + if got := s.LatencyHistogram.Buckets()[0].Count; got != 1 { + t.Fatalf("bucket accessor leaked mutability: got %d", got) + } +} + +func TestNilAccessorsRemainNil(t *testing.T) { + s := Snapshot{} + if got := s.LatencySeriesNs(); got != nil { + t.Fatalf("expected nil latency series, got %#v", got) + } + if got := s.Syscalls(); got != nil { + t.Fatalf("expected nil syscalls, got %#v", got) + } + + h := HistogramSnapshot{} + if got := h.Buckets(); got != nil { + t.Fatalf("expected nil buckets, got %#v", got) + } +} -- cgit v1.2.3