Skip to content

Commit

Permalink
agentlog: fix race condition in tests
Browse files Browse the repository at this point in the history
Output of the race condition detector was:

=== FAIL: agentlog TestPsiEveIntegratedStartTwice (0.17s)
==================
WARNING: DATA RACE
Write at 0x0000029a6120 by goroutine 80:
  github.com/lf-edge/eve/pkg/pillar/agentlog_test.emulateMemoryPressureStats()
      /pillar/agentlog/agentlog_test.go:301 +0xd1
  github.com/lf-edge/eve/pkg/pillar/agentlog_test.preparePSIEnvironment()
      /pillar/agentlog/agentlog_test.go:455 +0x85
  github.com/lf-edge/eve/pkg/pillar/agentlog_test.TestPsiEveIntegratedStartTwice()
      /pillar/agentlog/agentlog_test.go:616 +0x44
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /usr/lib/go/src/testing/testing.go:1629 +0x47

Previous read at 0x0000029a6120 by goroutine 78:
  github.com/lf-edge/eve/pkg/pillar/agentlog.collectMemoryPSI()
      /pillar/agentlog/memprofile.go:151 +0x116
  github.com/lf-edge/eve/pkg/pillar/agentlog.MemoryPSICollector()
      /pillar/agentlog/memprofile.go:248 +0x810
  github.com/lf-edge/eve/pkg/pillar/agentlog.ListenDebug.func6.1()
      /pillar/agentlog/http-debug.go:474 +0x7e

Goroutine 80 (running) created at:
  testing.(*T).Run()
      /usr/lib/go/src/testing/testing.go:1629 +0x805
  testing.runTests.func1()
      /usr/lib/go/src/testing/testing.go:2036 +0x8d
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:1576 +0x216
  testing.runTests()
      /usr/lib/go/src/testing/testing.go:2034 +0x87c
  testing.(*M).Run()
      /usr/lib/go/src/testing/testing.go:1906 +0xb44
  main.main()
      _testmain.go:102 +0x2fc

Goroutine 78 (running) created at:
  github.com/lf-edge/eve/pkg/pillar/agentlog.ListenDebug.func6()
      /pillar/agentlog/http-debug.go:473 +0x2ce
  net/http.HandlerFunc.ServeHTTP()
      /usr/lib/go/src/net/http/server.go:2122 +0x4d
  net/http.(*ServeMux).ServeHTTP()
      /usr/lib/go/src/net/http/server.go:2500 +0xc5
  net/http.serverHandler.ServeHTTP()
      /usr/lib/go/src/net/http/server.go:2936 +0x682
  net/http.(*conn).serve()
      /usr/lib/go/src/net/http/server.go:1995 +0xbd4
  net/http.(*Server).Serve.func3()
      /usr/lib/go/src/net/http/server.go:3089 +0x58
==================
    testing.go:1446: race detected during execution of test

Signed-off-by: Christoph Ostarek <[email protected]>
  • Loading branch information
christoph-zededa committed Dec 9, 2024
1 parent 5330573 commit cabff85
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 9 deletions.
12 changes: 6 additions & 6 deletions pkg/pillar/agentlog/agentlog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,8 @@ func emulateMemoryPressureStats() (cancel context.CancelFunc, err error) {
}

fakePSIFileName := fakePSIFileHandler.Name()
originalPressureMemoryFile := agentlog.PressureMemoryFile
agentlog.PressureMemoryFile = fakePSIFileName
originalPressureMemoryFile := agentlog.PressureMemoryFile()
agentlog.UpdatePressureMemoryFile(fakePSIFileName)

// Start a ticker to write a new line to the file every 0.5 seconds
ticker := time.NewTicker(500 * time.Millisecond)
Expand Down Expand Up @@ -342,7 +342,7 @@ func emulateMemoryPressureStats() (cancel context.CancelFunc, err error) {
agentlog.PsiMutex.Lock()
fakePSIFileHandler.Close()
os.Remove(fakePSIFileName)
agentlog.PressureMemoryFile = originalPressureMemoryFile
agentlog.UpdatePressureMemoryFile(originalPressureMemoryFile)
agentlog.PsiMutex.Unlock()
// We destroy this producer, so release the mutex
psiProducerMutex.Unlock()
Expand Down Expand Up @@ -371,8 +371,8 @@ func createFakePSIStatsFile() (cleanupFunc context.CancelFunc, err error) {
}

fakePSIFileName := fakePSIFileHandler.Name()
originalPressureMemoryFile := agentlog.PressureMemoryFile
agentlog.PressureMemoryFile = fakePSIFileName
originalPressureMemoryFile := agentlog.PressureMemoryFile()
agentlog.UpdatePressureMemoryFile(fakePSIFileName)

// Write some content to the file
agentlog.PsiMutex.Lock()
Expand All @@ -389,7 +389,7 @@ func createFakePSIStatsFile() (cleanupFunc context.CancelFunc, err error) {
agentlog.PsiMutex.Lock()
fakePSIFileHandler.Close()
os.Remove(fakePSIFileName)
agentlog.PressureMemoryFile = originalPressureMemoryFile
agentlog.UpdatePressureMemoryFile(originalPressureMemoryFile)
agentlog.PsiMutex.Unlock()
// We destroy this producer, so release the mutex
psiProducerMutex.Unlock()
Expand Down
23 changes: 20 additions & 3 deletions pkg/pillar/agentlog/memprofile.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,26 @@ const (
var (
// PressureMemoryFile is the memory pressure file. It is a variable, not a constant, to allow
// changing it in tests to mock the file.
PressureMemoryFile = "/proc/pressure/memory"
pressureMemoryFile = "/proc/pressure/memory"
pressureMemoryFileLock sync.RWMutex
)

// PressureMemoryFile returns the path to the memory pressure file in /proc
func PressureMemoryFile() string {
pressureMemoryFileLock.RLock()
path := pressureMemoryFile
pressureMemoryFileLock.RUnlock()

return path
}

// UpdatePressureMemoryFile sets the path to the memory pressure file (mostly used for tests)
func UpdatePressureMemoryFile(newpath string) {
pressureMemoryFileLock.Lock()
pressureMemoryFile = newpath
pressureMemoryFileLock.Unlock()
}

// MemAllocationSite is the return value of GetMemProfile
type MemAllocationSite struct {
InUseBytes int64
Expand Down Expand Up @@ -134,7 +151,7 @@ func GetMemAllocationSites(reportZeroInUse bool) (int, []MemAllocationSite) {
var PsiMutex sync.Mutex

func isPSISupported() bool {
_, err := os.Stat(PressureMemoryFile)
_, err := os.Stat(PressureMemoryFile())
if err != nil {
fmt.Println("PSI is not supported. Be sure to enable CONFIG_PSI=y in your kernel configuration.")
return false
Expand All @@ -148,7 +165,7 @@ func collectMemoryPSI() (*PressureStallInfo, error) {
if !isPSISupported() {
return nil, fmt.Errorf("PSI is not supported")
}
procFile, err := os.Open(PressureMemoryFile)
procFile, err := os.Open(PressureMemoryFile())
if err != nil {
return nil, fmt.Errorf("open: %v", err)
}
Expand Down

0 comments on commit cabff85

Please sign in to comment.