Skip to content

Commit 526c138

Browse files
committed
fix(logs): protect PublicJSON from racing with SetTailingMode
PublicJSON was called on *LogsConfig directly without the LogSource lock, while SetTailingMode writes Config.TailingMode under the lock. Add a PublicJSON method to *LogSource that takes the lock before delegating to Config.PublicJSON, and update the caller in inventorychecks to use it. Found via a race-detector-enabled build in staging (see #54333).
1 parent 92f6b05 commit 526c138

3 files changed

Lines changed: 59 additions & 1 deletion

File tree

comp/metadata/inventorychecks/impl/inventorychecks.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func (ic *inventorychecksImpl) getPayload(withConfigs bool) marshaler.JSONMarsha
222222
logsMetadata[logSource.Name] = []metadata{}
223223
}
224224

225-
parsedJSON, err := logSource.Config.PublicJSON()
225+
parsedJSON, err := logSource.PublicJSON()
226226
if err != nil {
227227
ic.log.Debugf("could not parse log configuration for source metadata %s: %v", logSource.Name, err)
228228
continue

pkg/logs/sources/source.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,15 @@ func (s *LogSource) GetTailingMode() string {
121121
return s.Config.TailingMode
122122
}
123123

124+
// PublicJSON returns the public JSON representation of the source's logs config.
125+
// The lock is held because Config.TailingMode can be mutated at runtime by
126+
// SetTailingMode on another goroutine.
127+
func (s *LogSource) PublicJSON() ([]byte, error) {
128+
s.lock.Lock()
129+
defer s.lock.Unlock()
130+
return s.Config.PublicJSON()
131+
}
132+
124133
// SetTailingMode sets the tailing mode configured for this source. This is mutated after
125134
// creation when a tailing mode is inferred at launch time, so it must go through the lock.
126135
func (s *LogSource) SetTailingMode(mode string) {

pkg/logs/sources/source_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package sources
88
import (
99
"sync"
1010
"testing"
11+
"time"
1112

1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/suite"
@@ -104,3 +105,51 @@ func TestConcurrentTailingModeAndStatusAccess(t *testing.T) {
104105
close(stop)
105106
wg.Wait()
106107
}
108+
109+
// TestConcurrentPublicJSONAndSetTailingMode verifies that PublicJSON (which reads
110+
// Config.TailingMode) does not race with SetTailingMode (which writes it).
111+
// Before the fix, PublicJSON was called on Config directly without the LogSource
112+
// lock, racing with SetTailingMode on another goroutine.
113+
func TestConcurrentPublicJSONAndSetTailingMode(t *testing.T) {
114+
source := NewLogSource("test", &config.LogsConfig{
115+
Type: "file",
116+
Path: "/var/log/test.log",
117+
TailingMode: "end",
118+
})
119+
120+
var wg sync.WaitGroup
121+
stop := make(chan struct{})
122+
123+
// writer: mimics (*Launcher).launchTailers mutating TailingMode concurrently.
124+
wg.Go(func() {
125+
for i := 0; ; i++ {
126+
select {
127+
case <-stop:
128+
return
129+
default:
130+
}
131+
if i%2 == 0 {
132+
source.SetTailingMode("beginning")
133+
} else {
134+
source.SetTailingMode("end")
135+
}
136+
}
137+
})
138+
139+
// reader: mimics inventorychecksImpl.getPayload calling PublicJSON concurrently.
140+
wg.Go(func() {
141+
for {
142+
select {
143+
case <-stop:
144+
return
145+
default:
146+
}
147+
_, _ = source.PublicJSON()
148+
}
149+
})
150+
151+
// Let the race detector observe the overlap.
152+
time.Sleep(50 * time.Millisecond)
153+
close(stop)
154+
wg.Wait()
155+
}

0 commit comments

Comments
 (0)