From 7b37838591a7d9c76ce51ac807c9d48f4d690844 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Tue, 20 Oct 2026 02:33:44 +0000 Subject: [PATCH 1/3] fix: synchronize DefaultEntropy reads Signed-off-by: Perfloop Agent --- CHANGELOG.md | 4 +++ ulid.go | 15 +++++++++- ulid_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8da38c6..744d5d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased + +* Synchronize DefaultEntropy reads with monotonic generation. + ## 1.3.1 / 2018-10-02 * Use underlying entropy source for random increments in Monotonic (#32) diff --git a/ulid.go b/ulid.go index e07e1a6..172bfd1 100644 --- a/ulid.go +++ b/ulid.go @@ -134,7 +134,9 @@ func MustNewDefault(t time.Time) ULID { var defaultEntropy = func() io.Reader { rng := rand.New(rand.NewSource(time.Now().UnixNano())) - return &LockedMonotonicReader{MonotonicReader: Monotonic(rng, 0)} + return &defaultEntropyReader{ + LockedMonotonicReader: &LockedMonotonicReader{MonotonicReader: Monotonic(rng, 0)}, + } }() // DefaultEntropy returns a thread-safe per process monotonically increasing @@ -600,6 +602,17 @@ type LockedMonotonicReader struct { MonotonicReader } +type defaultEntropyReader struct { + *LockedMonotonicReader +} + +// Read synchronizes direct reads with monotonic reads on the default source. +func (r *defaultEntropyReader) Read(p []byte) (int, error) { + r.LockedMonotonicReader.mu.Lock() + defer r.LockedMonotonicReader.mu.Unlock() + return r.LockedMonotonicReader.MonotonicReader.Read(p) +} + // MonotonicRead synchronizes calls to the wrapped MonotonicReader. func (r *LockedMonotonicReader) MonotonicRead(ms uint64, p []byte) (err error) { r.mu.Lock() diff --git a/ulid_test.go b/ulid_test.go index afa194b..51652b6 100644 --- a/ulid_test.go +++ b/ulid_test.go @@ -20,6 +20,7 @@ import ( "io" "math" "math/rand" + "reflect" "strings" "testing" "testing/iotest" @@ -688,6 +689,69 @@ func TestMonotonicSafe(t *testing.T) { } } +func TestLockedMonotonicReaderValueImplementsReader(t *testing.T) { + readerType := reflect.TypeOf((*ulid.LockedMonotonicReader)(nil)).Elem() + readerInterface := reflect.TypeOf((*io.Reader)(nil)).Elem() + if !readerType.Implements(readerInterface) { + t.Fatalf("%v does not implement %v", readerType, readerInterface) + } +} + +func TestDefaultEntropyMonotonicReader(t *testing.T) { + reader, ok := ulid.DefaultEntropy().(ulid.MonotonicReader) + if !ok { + t.Fatal("DefaultEntropy does not implement MonotonicReader") + } + + const ms = 123 + prev, err := ulid.New(ms, reader) + if err != nil { + t.Fatal(err) + } + next, err := ulid.New(ms, reader) + if err != nil { + t.Fatal(err) + } + if prev.Compare(next) >= 0 { + t.Fatalf("ULIDs are not increasing: %v >= %v", prev, next) + } +} + +func TestDefaultEntropyConcurrentReadAndMake(t *testing.T) { + const iterations = 4096 + + start := make(chan struct{}) + done := make(chan error, 2) + + go func() { + <-start + entropy := ulid.DefaultEntropy() + var p [10]byte + for i := 0; i < iterations; i++ { + if _, err := io.ReadFull(entropy, p[:]); err != nil { + done <- err + return + } + } + done <- nil + }() + + go func() { + <-start + for i := 0; i < iterations; i++ { + _ = ulid.Make() + } + done <- nil + }() + + close(start) + for i := 0; i < 2; i++ { + if err := <-done; err != nil { + t.Fatal(err) + } + } +} + func TestULID_Bytes(t *testing.T) { tt := time.Unix(1000000, 0) entropy := ulid.Monotonic(rand.New(rand.NewSource(tt.UnixNano())), 0) @@ -711,6 +775,20 @@ func BenchmarkMustNew(b *testing.B) { }) } +func BenchmarkDefaultEntropyRead(b *testing.B) { + entropy := ulid.DefaultEntropy() + var p [10]byte + + b.ReportAllocs() + b.SetBytes(int64(len(p))) + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, err := io.ReadFull(entropy, p[:]); err != nil { + b.Fatal(err) + } + } +} + func benchmarkMakeULID(b *testing.B, f func(uint64, io.Reader)) { b.ReportAllocs() b.SetBytes(int64(len(ulid.ULID{}))) From 18944d7d25c3bb30f1be4721c4508a73ff80a603 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Tue, 20 Oct 2026 12:39:17 +0000 Subject: [PATCH 2/3] docs: explain default entropy reader locking Signed-off-by: Perfloop Agent --- ulid.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ulid.go b/ulid.go index 172bfd1..035903e 100644 --- a/ulid.go +++ b/ulid.go @@ -602,6 +602,12 @@ type LockedMonotonicReader struct { MonotonicReader } +// defaultEntropyReader keeps direct DefaultEntropy reads serialized with +// MonotonicRead using the same mutex. LockedMonotonicReader's promoted Read +// bypasses that lock, so Read belongs on this private adapter. Adding Read to +// LockedMonotonicReader would break its value-form io.Reader compatibility: +// a pointer receiver shadows the promoted Read for values, while a value +// receiver would copy the mutex. type defaultEntropyReader struct { *LockedMonotonicReader } From 4c483c3f6151bfc81cca98a6ef5a28cc99535340 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Thu, 22 Oct 2026 00:23:52 +0000 Subject: [PATCH 3/3] fix: synchronize LockedMonotonicReader reads Signed-off-by: Perfloop Agent --- ulid.go | 24 +++++------------- ulid_test.go | 71 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 24 deletions(-) diff --git a/ulid.go b/ulid.go index 035903e..dc3a154 100644 --- a/ulid.go +++ b/ulid.go @@ -134,9 +134,7 @@ func MustNewDefault(t time.Time) ULID { var defaultEntropy = func() io.Reader { rng := rand.New(rand.NewSource(time.Now().UnixNano())) - return &defaultEntropyReader{ - LockedMonotonicReader: &LockedMonotonicReader{MonotonicReader: Monotonic(rng, 0)}, - } + return &LockedMonotonicReader{MonotonicReader: Monotonic(rng, 0)} }() // DefaultEntropy returns a thread-safe per process monotonically increasing @@ -602,21 +600,11 @@ type LockedMonotonicReader struct { MonotonicReader } -// defaultEntropyReader keeps direct DefaultEntropy reads serialized with -// MonotonicRead using the same mutex. LockedMonotonicReader's promoted Read -// bypasses that lock, so Read belongs on this private adapter. Adding Read to -// LockedMonotonicReader would break its value-form io.Reader compatibility: -// a pointer receiver shadows the promoted Read for values, while a value -// receiver would copy the mutex. -type defaultEntropyReader struct { - *LockedMonotonicReader -} - -// Read synchronizes direct reads with monotonic reads on the default source. -func (r *defaultEntropyReader) Read(p []byte) (int, error) { - r.LockedMonotonicReader.mu.Lock() - defer r.LockedMonotonicReader.mu.Unlock() - return r.LockedMonotonicReader.MonotonicReader.Read(p) +// Read synchronizes calls to the wrapped MonotonicReader. +func (r *LockedMonotonicReader) Read(p []byte) (int, error) { + r.mu.Lock() + defer r.mu.Unlock() + return r.MonotonicReader.Read(p) } // MonotonicRead synchronizes calls to the wrapped MonotonicReader. diff --git a/ulid_test.go b/ulid_test.go index 51652b6..d3d4038 100644 --- a/ulid_test.go +++ b/ulid_test.go @@ -20,7 +20,6 @@ import ( "io" "math" "math/rand" - "reflect" "strings" "testing" "testing/iotest" @@ -689,11 +688,71 @@ func TestMonotonicSafe(t *testing.T) { } } -func TestLockedMonotonicReaderValueImplementsReader(t *testing.T) { - readerType := reflect.TypeOf((*ulid.LockedMonotonicReader)(nil)).Elem() - readerInterface := reflect.TypeOf((*io.Reader)(nil)).Elem() - if !readerType.Implements(readerInterface) { - t.Fatalf("%v does not implement %v", readerType, readerInterface) +func TestLockedMonotonicReaderRead(t *testing.T) { + t.Run("bytes", func(t *testing.T) { + want := []byte("entropy") + reader := &ulid.LockedMonotonicReader{MonotonicReader: ulid.Monotonic(bytes.NewReader(want), 0)} + got := make([]byte, len(want)) + + n, err := reader.Read(got) + if err != nil { + t.Fatalf("read entropy: %v", err) + } + if n != len(want) { + t.Fatalf("read %d bytes, want %d", n, len(want)) + } + if !bytes.Equal(got, want) { + t.Fatalf("read %x, want %x", got, want) + } + }) + + t.Run("error", func(t *testing.T) { + wantErr := io.ErrUnexpectedEOF + reader := &ulid.LockedMonotonicReader{MonotonicReader: ulid.Monotonic(iotest.ErrReader(wantErr), 0)} + var p [10]byte + + n, err := reader.Read(p[:]) + if n != 0 || err != wantErr { + t.Fatalf("read returned (%d, %v), want (0, %v)", n, err, wantErr) + } + }) +} + +func TestLockedMonotonicReaderConcurrentReadAndNew(t *testing.T) { + const iterations = 4096 + + reader := &ulid.LockedMonotonicReader{MonotonicReader: ulid.Monotonic(rand.New(rand.NewSource(1)), 0)} + start := make(chan struct{}) + done := make(chan error, 2) + + go func() { + <-start + var p [10]byte + for i := 0; i < iterations; i++ { + if _, err := io.ReadFull(reader, p[:]); err != nil { + done <- err + return + } + } + done <- nil + }() + + go func() { + <-start + for i := 0; i < iterations; i++ { + if _, err := ulid.New(uint64(i), reader); err != nil { + done <- err + return + } + } + done <- nil + }() + + close(start) + for i := 0; i < 2; i++ { + if err := <-done; err != nil { + t.Fatal(err) + } } }