sonic_data_client: make PollStats stoppable and wire into TestMain#606
Open
sigabrtv1-ui wants to merge 2 commits intosonic-net:masterfrom
Open
sonic_data_client: make PollStats stoppable and wire into TestMain#606sigabrtv1-ui wants to merge 2 commits intosonic-net:masterfrom
sigabrtv1-ui wants to merge 2 commits intosonic-net:masterfrom
Conversation
…tests PollStats is started in init() and never stopped. In tests, the goroutine leaks across test boundaries and races with test teardown on os.File pointers, causing nil pointer panics and DATA RACE failures. Add StopPollStats() and a done channel so tests can stop the goroutine cleanly during TestMain cleanup. Use select with time.After instead of time.Sleep so the goroutine responds to shutdown without a 100ms delay. Fixes DATA RACE observed in CI (master build 20260305.14): panic: runtime error: invalid memory address or nil pointer dereference goroutine in PollStats() racing on *os.File Signed-off-by: sigabrtv1-ui <sig.abrt.v1@gmail.com>
Call StopPollStats() in TestMain for both packages before tests run. This ensures the PollStats goroutine (started in sonic_data_client's init()) exits cleanly before any test has a chance to use gomonkey patches that intercept os.OpenFile or os.File methods. Also fixes coverage: the StopPollStats shutdown path (done channel) was only exercised by gnmi_server tests, not counted toward sonic_data_client coverage. Calling it here closes that gap. Signed-off-by: sigabrtv1-ui <sig.abrt.v1@gmail.com>
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
sigabrtv1-ui
pushed a commit
to sigabrtv1-ui/sonic-gnmi
that referenced
this pull request
Mar 9, 2026
…ZMQ issue) sonic_data_client tests all pass, but ZMQ context cleanup (zmq_ctx_destroy via atexit) blocks indefinitely with Go 1.23.9, causing the test binary to hang until the test timeout kills it. With Go 1.19 the same cleanup completed in ~24s. This is a Go 1.23 CGO runtime regression unrelated to PR sonic-net#611 code. Exclude sonic_data_client from check_gotest_junit for now. Track alongside PR sonic-net#606 (PollStats stoppable) which addresses related ZMQ lifecycle issues. Signed-off-by: Dawei Huang <daweihuang@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PollStats()is started insonic_data_client'sinit()and never stopped. The goroutine leaks across test boundaries in bothgnmi_serverandsonic_data_clienttest packages.When tests use gomonkey to patch
os.OpenFileoros.Filemethods, the still-runningPollStatsgoroutine callslinuxproc.ReadStat("/proc/stat")which goes throughos.OpenFile, hits the mock, and receives a zero-value*os.File{}. CallingStat()on that causes a nil pointer dereference:Also observed as a DATA RACE in CI build
20260305.14.Fix
sonic_data_client/non_db_client.gopollStatsDone chan struct{}+sync.OnceStopPollStats()— safe to call multiple timesPollStats()selects on done channel each iteration and exits when closedselect { case <-time.After(...) }instead oftime.Sleepfor immediate shutdown responsegnmi_server/server_test.goandsonic_data_client/client_test.goStopPollStats()inTestMainbefore running tests in both packagesNotes
StopPollStats()is intentionally test-only — production code never calls itsync.Once)-gcflags=all=-l): once gomonkey patches are effective, this goroutine would otherwise race and crash the test binary