Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions internal/state/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import (
// syncFields projects synchronization wrappers onto their values. In
// particular, atomic.Pointer[T].v must be viewed as *T so state relocates its
// target instead of saving an untyped address. Pool.New and Cond.L retain their
// object graphs; other sync primitives are empty records. sync.Map uses its own
// entry codec. The source data graph is quiescent.
// object graphs; Once retains done, with a fresh mutex on load. Other sync
// primitives are empty records. sync.Map uses its own entry codec. The source
// data graph is quiescent, including any Once.Do call.
func syncFields(obj reflect.Value) ([]reflect.Value, bool) {
typ := obj.Type()
pkg, name := typ.PkgPath(), typ.Name()
Expand All @@ -20,6 +21,8 @@ func syncFields(obj reflect.Value) ([]reflect.Value, bool) {
return []reflect.Value{obj.FieldByName("New")}, true
case reflect.TypeFor[sync.Cond]():
return []reflect.Value{obj.FieldByName("L")}, true
case reflect.TypeFor[sync.Once]():
return []reflect.Value{obj.FieldByName("done")}, true
Comment on lines +24 to +25

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Once/Pool/Cond field access is not version-gated

The sync.Once case uses obj.FieldByName("done") with no field-presence or version guard, and the Once path returns before the runtime.Version() != "go1.26.6" assertion at line 42 (that check only guards the sync/atomic paths). The pre-existing Pool.New/Cond.L arms share this gap. If a future Go release renames or restructures these fields, FieldByName returns an invalid reflect.Value and encode/decode panics at first snapshot of such a type rather than failing loudly at startup. Since the codebase already hard-pins go1.26.6 for the atomic paths, consider extending an equivalent explicit version/field-presence assertion to the Once (and Pool/Cond) cases so a Go upgrade fails early with a clear message. Robustness/portability only — not exploitable and not a correctness defect at the pinned version.

}
if pkg == "sync" && typ != reflect.TypeFor[sync.Map]() || pkg == "internal/sync" && name == "Mutex" {
return nil, true
Expand Down
102 changes: 92 additions & 10 deletions internal/state/sync_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package state

import (
"bytes"
"context"
"reflect"
"runtime"
"sync"
Expand Down Expand Up @@ -212,17 +214,97 @@ func TestSyncPoolZero(t *testing.T) {
}

func TestSyncOnceZero(t *testing.T) {
src, dst := new(sync.Once), new(sync.Once)
src.Do(func() {})
dst.Do(func() {})
roundtrip(t, src, dst)
calls := 0
dst.Do(func() { calls++ })
dst.Do(func() { calls++ })
src.Do(func() { t.Fatal("snapshot reset the source Once") })
if calls != 1 {
t.Fatal("restored Once did not start fresh")
for _, status := range []string{"zero", "done", "panic"} {
t.Run(status, func(t *testing.T) {
for _, initialized := range []bool{false, true} {
src, dst := new(sync.Once), new(sync.Once)
switch status {
case "done":
src.Do(func() {})
case "panic":
func() {
defer func() {
if got := recover(); got != "once panic" {
t.Fatalf("unexpected panic: %v", got)
}
}()
src.Do(func() { panic("once panic") })
}()
}
if initialized {
dst.Do(func() {})
}
// A reused destination must lose its old mutex state as well as done.
mutex := reflectValueRWAddr(reflect.ValueOf(dst).Elem().FieldByName("m")).Interface().(*sync.Mutex)
mutex.Lock()
roundtrip(t, src, dst)
if !mutex.TryLock() {
t.Fatal("restored Once retained the destination mutex state")
}
mutex.Unlock()
var calls atomic.Int32
var workers sync.WaitGroup
for range 8 {
workers.Go(func() { dst.Do(func() { calls.Add(1) }) })
}
workers.Wait()
want := int32(0)
if status == "zero" {
want = 1
}
if calls.Load() != want {
t.Fatalf("destination initialized=%v: got %d calls, want %d", initialized, calls.Load(), want)
}
sourceCalls := int32(0)
src.Do(func() { sourceCalls++ })
if sourceCalls != want {
t.Fatal("snapshot changed the source Once")
}
}
})
}
t.Run("writeback", func(t *testing.T) {
type root struct {
Once sync.Once
Alias *sync.Once
Value int
}
host := new(root)
host.Alias = &host.Once
var guest root
var source, destination State
ctx := context.Background()
mem := make([]byte, 1<<20)
for range 2 {
var input bytes.Buffer
if _, _, err := source.SaveTo(ctx, &input, host); err != nil {
t.Fatal(err)
}
if _, err := destination.Load(ctx, input.Bytes(), &guest); err != nil {
t.Fatal(err)
}
if guest.Alias != &guest.Once {
t.Fatal("Once lost its alias in the guest")
}
before := host.Value
guest.Alias.Do(func() { guest.Value++ })
guest.Once.Do(func() { guest.Value++ })
if guest.Value != 1 || host.Value != before {
t.Fatal("guest repeated initialization or modified the host")
}
n, _, err := destination.Save(ctx, mem, &guest)
if err != nil {
t.Fatal(err)
}
if _, err := source.Load(ctx, mem[:n], host); err != nil {
t.Fatal(err)
}
if host.Alias != &host.Once || host.Value != 1 {
t.Fatal("writeback lost Once identity or initialized data")
}
host.Alias.Do(func() { t.Fatal("writeback lost Once completion") })
}
})
}

func TestSyncWaitGroupZero(t *testing.T) {
Expand Down
Loading