From e54d9d60d3a44bcdc9ab19fd6bf78e83db51a0cb Mon Sep 17 00:00:00 2001 From: Alexey Dolotov Date: Mon, 30 Mar 2026 14:50:32 +0300 Subject: [PATCH 1/3] fix: stabilize flaky CI tests 1. Add sync.Mutex to ScoutConnCollected to eliminate data race between Add()/MarkWrite() in readLoop and learn() iterating results. Introduce Snapshot() for safe read access. 2. Increase bloom filter test size from 500 to 100000 to prevent false negatives from random eviction in the stable bloom filter. 3. Use Require().NoError() in TestHTTPSRequest to prevent nil-pointer panic on resp.Body.Close() when the request fails. Fixes #425 --- antireplay/stable_bloom_filter_test.go | 2 +- mtglib/internal/doppel/scout.go | 14 +++++++------ .../internal/doppel/scout_conn_collected.go | 21 ++++++++++++++++++- mtglib/proxy_test.go | 2 +- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/antireplay/stable_bloom_filter_test.go b/antireplay/stable_bloom_filter_test.go index accb590..8c88ce2 100644 --- a/antireplay/stable_bloom_filter_test.go +++ b/antireplay/stable_bloom_filter_test.go @@ -12,7 +12,7 @@ type StableBloomFilterTestSuite struct { } func (suite *StableBloomFilterTestSuite) TestOp() { - filter := antireplay.NewStableBloomFilter(500, 0.001) + filter := antireplay.NewStableBloomFilter(100000, 0.001) suite.False(filter.SeenBefore([]byte{1, 2, 3})) suite.False(filter.SeenBefore([]byte{4, 5, 6})) diff --git a/mtglib/internal/doppel/scout.go b/mtglib/internal/doppel/scout.go index 4b58e32..d90feba 100644 --- a/mtglib/internal/doppel/scout.go +++ b/mtglib/internal/doppel/scout.go @@ -61,7 +61,9 @@ func (s Scout) learn(ctx context.Context, url string) (ScoutResult, error) { client.CloseIdleConnections() } - if err != nil || len(results.data) == 0 { + data, writeIndex := results.Snapshot() + + if err != nil || len(data) == 0 { return ScoutResult{}, err } @@ -70,14 +72,14 @@ func (s Scout) learn(ctx context.Context, url string) (ScoutResult, error) { // Compute inter-record durations (existing logic). lastTimestamp := time.Time{} - for i, v := range results.data { + for i, v := range data { if v.recordType != tls.TypeApplicationData { continue } if lastTimestamp.IsZero() { if i > 0 { - lastTimestamp = results.data[i-1].timestamp + lastTimestamp = data[i-1].timestamp } else { lastTimestamp = v.timestamp } @@ -90,12 +92,12 @@ func (s Scout) learn(ctx context.Context, url string) (ScoutResult, error) { // Compute cert size: sum of ApplicationData payload between CCS and // the first client Write (which marks the end of server handshake). seenCCS := false - boundary := results.writeIndex + boundary := writeIndex if boundary < 0 { - boundary = len(results.data) + boundary = len(data) } - for i, v := range results.data { + for i, v := range data { if i >= boundary { break } diff --git a/mtglib/internal/doppel/scout_conn_collected.go b/mtglib/internal/doppel/scout_conn_collected.go index 0fe4e4a..cdcfbeb 100644 --- a/mtglib/internal/doppel/scout_conn_collected.go +++ b/mtglib/internal/doppel/scout_conn_collected.go @@ -1,6 +1,9 @@ package doppel -import "time" +import ( + "sync" + "time" +) const ( ScoutConnCollectedPreallocSize = 100 @@ -13,23 +16,39 @@ type ScoutConnResult struct { } type ScoutConnCollected struct { + mu sync.Mutex data []ScoutConnResult writeIndex int // index at which client first wrote post-handshake data; -1 if not set } func (s *ScoutConnCollected) Add(record byte, payloadLen int) { + s.mu.Lock() s.data = append(s.data, ScoutConnResult{ timestamp: time.Now(), recordType: record, payloadLen: payloadLen, }) + s.mu.Unlock() } // MarkWrite records the current data length as the handshake boundary. func (s *ScoutConnCollected) MarkWrite() { + s.mu.Lock() if s.writeIndex < 0 { s.writeIndex = len(s.data) } + s.mu.Unlock() +} + +// Snapshot returns a copy of the collected data and the write index. +func (s *ScoutConnCollected) Snapshot() ([]ScoutConnResult, int) { + s.mu.Lock() + snapshot := make([]ScoutConnResult, len(s.data)) + copy(snapshot, s.data) + writeIndex := s.writeIndex + s.mu.Unlock() + + return snapshot, writeIndex } func NewScoutConnCollected() *ScoutConnCollected { diff --git a/mtglib/proxy_test.go b/mtglib/proxy_test.go index fc05012..278ce4f 100644 --- a/mtglib/proxy_test.go +++ b/mtglib/proxy_test.go @@ -175,7 +175,7 @@ func (suite *ProxyTestSuite) TestHTTPSRequest() { addr := fmt.Sprintf("https://%s/headers", suite.ProxyAddress()) resp, err := client.Get(addr) //nolint: noctx - suite.NoError(err) + suite.Require().NoError(err) defer resp.Body.Close() //nolint: errcheck From 73c6a3aa37315a3e6f66185f1faa8e032e915d88 Mon Sep 17 00:00:00 2001 From: Alexey Dolotov Date: Mon, 30 Mar 2026 15:00:17 +0300 Subject: [PATCH 2/3] fix: tighten ScoutConnCollected encapsulation and add concurrency test - Move error check before Snapshot() to avoid unnecessary allocation - Update existing tests to use Snapshot() instead of direct field access - Add TestConcurrentAddSnapshot to explicitly exercise the mutex --- mtglib/internal/doppel/scout.go | 8 ++- .../doppel/scout_conn_collected_test.go | 52 +++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/mtglib/internal/doppel/scout.go b/mtglib/internal/doppel/scout.go index d90feba..24e06a7 100644 --- a/mtglib/internal/doppel/scout.go +++ b/mtglib/internal/doppel/scout.go @@ -61,10 +61,14 @@ func (s Scout) learn(ctx context.Context, url string) (ScoutResult, error) { client.CloseIdleConnections() } + if err != nil { + return ScoutResult{}, err + } + data, writeIndex := results.Snapshot() - if err != nil || len(data) == 0 { - return ScoutResult{}, err + if len(data) == 0 { + return ScoutResult{}, nil } var result ScoutResult diff --git a/mtglib/internal/doppel/scout_conn_collected_test.go b/mtglib/internal/doppel/scout_conn_collected_test.go index dcf8a41..fad45dd 100644 --- a/mtglib/internal/doppel/scout_conn_collected_test.go +++ b/mtglib/internal/doppel/scout_conn_collected_test.go @@ -1,6 +1,7 @@ package doppel import ( + "sync" "testing" "time" @@ -16,8 +17,10 @@ func (suite *ScoutConnCollectedTestSuite) TestAddSingle() { collected := NewScoutConnCollected() collected.Add(tls.TypeApplicationData, 100) - suite.Len(collected.data, 1) - suite.Equal(byte(tls.TypeApplicationData), collected.data[0].recordType) + data, _ := collected.Snapshot() + + suite.Len(data, 1) + suite.Equal(byte(tls.TypeApplicationData), data[0].recordType) } func (suite *ScoutConnCollectedTestSuite) TestAddTimestampsAreMonotonic() { @@ -31,11 +34,52 @@ func (suite *ScoutConnCollectedTestSuite) TestAddTimestampsAreMonotonic() { time.Sleep(time.Microsecond) collected.Add(tls.TypeApplicationData, 100) - for i := 1; i < len(collected.data); i++ { - suite.True(collected.data[i].timestamp.After(collected.data[i-1].timestamp)) + data, _ := collected.Snapshot() + + for i := 1; i < len(data); i++ { + suite.True(data[i].timestamp.After(data[i-1].timestamp)) } } +func (suite *ScoutConnCollectedTestSuite) TestConcurrentAddSnapshot() { + collected := NewScoutConnCollected() + + var wg sync.WaitGroup + + wg.Add(3) + + go func() { + defer wg.Done() + + for i := 0; i < 1000; i++ { + collected.Add(tls.TypeApplicationData, i) + } + }() + + go func() { + defer wg.Done() + + for i := 0; i < 100; i++ { + collected.MarkWrite() + } + }() + + go func() { + defer wg.Done() + + for i := 0; i < 1000; i++ { + data, _ := collected.Snapshot() + _ = len(data) + } + }() + + wg.Wait() + + data, writeIndex := collected.Snapshot() + suite.Len(data, 1000) + suite.GreaterOrEqual(writeIndex, 0) +} + func TestScoutConnCollected(t *testing.T) { t.Parallel() suite.Run(t, &ScoutConnCollectedTestSuite{}) From eedee631430b5b3b9212a48f03280ca50bae4cc5 Mon Sep 17 00:00:00 2001 From: dolonet Date: Mon, 30 Mar 2026 16:17:51 +0000 Subject: [PATCH 3/3] Address review: use slices.Clone, simplify concurrent test - Replace manual make+copy with slices.Clone in Snapshot() - Remove redundant _ = len(data); Snapshot() call alone is sufficient to exercise the lock under -race --- mtglib/internal/doppel/scout_conn_collected.go | 4 ++-- mtglib/internal/doppel/scout_conn_collected_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mtglib/internal/doppel/scout_conn_collected.go b/mtglib/internal/doppel/scout_conn_collected.go index cdcfbeb..5a4de92 100644 --- a/mtglib/internal/doppel/scout_conn_collected.go +++ b/mtglib/internal/doppel/scout_conn_collected.go @@ -1,6 +1,7 @@ package doppel import ( + "slices" "sync" "time" ) @@ -43,8 +44,7 @@ func (s *ScoutConnCollected) MarkWrite() { // Snapshot returns a copy of the collected data and the write index. func (s *ScoutConnCollected) Snapshot() ([]ScoutConnResult, int) { s.mu.Lock() - snapshot := make([]ScoutConnResult, len(s.data)) - copy(snapshot, s.data) + snapshot := slices.Clone(s.data) writeIndex := s.writeIndex s.mu.Unlock() diff --git a/mtglib/internal/doppel/scout_conn_collected_test.go b/mtglib/internal/doppel/scout_conn_collected_test.go index fad45dd..e49f6bd 100644 --- a/mtglib/internal/doppel/scout_conn_collected_test.go +++ b/mtglib/internal/doppel/scout_conn_collected_test.go @@ -68,8 +68,8 @@ func (suite *ScoutConnCollectedTestSuite) TestConcurrentAddSnapshot() { defer wg.Done() for i := 0; i < 1000; i++ { - data, _ := collected.Snapshot() - _ = len(data) + // call Snapshot concurrently to exercise the lock under -race + collected.Snapshot() //nolint:errcheck } }()