lizining1231 opened a new issue, #3712: URL: https://github.com/apache/dubbo-go/issues/3712
### Summary `go test ./... -race` intermittently fails `TestAccessLogFilterGoroutineShutdown` in `filter/accesslog` with `"3" is not greater than "3"`. The test asserts that creating the access-log filter spawns a goroutine by comparing the **process-wide** `runtime.NumGoroutine()` before and after `newFilter()`. When a previous test in the same process has just called `Shutdown()`, the old `processLogs` goroutine is still **asynchronously exiting**, so the newly started goroutine (+1) and the departing one (-1) cancel out and the count does not increase. ### Affected Locations | Package | Test | Test Location | Related Source | | --- | --- | --- | --- | | filter/accesslog | `TestAccessLogFilterGoroutineShutdown` | `https://github.com/apache/dubbo-go/blob/master/filter/accesslog/memory_leak_test.go#L49-L64` (assert at L64) | Start goroutine: `https://github.com/apache/dubbo-go/blob/master/filter/accesslog/filter.go#L95-L109` (`go accessLogFilter.processLogs()` at L105); Async shutdown: `https://github.com/apache/dubbo-go/blob/master/filter/accesslog/filter.go#L415-L419` + `#L422-L432` | ### Reproduction ```bash # deterministic local reproduction (failure message matches CI exactly) go test -race -run 'TestAccessLogFilter(FileHandleManagement|GoroutineShutdown)' ./filter/accesslog/ -count=10 -cpu=1 # -> fails ~5/10 runs with: "3" is not greater than "3" # CI: flaky under the full suite go test ./... -race ``` `TestAccessLogFilterFileHandleManagement` runs first (alphabetical order), calls `Shutdown()`, and leaves the `processLogs` goroutine exiting; the following `TestAccessLogFilterGoroutineShutdown` then samples the count while the old goroutine is still winding down. ### Root Cause - The test measures the **process-wide** goroutine count (`runtime.NumGoroutine()`) across a fixed `100ms` sleep window (`memory_leak_test.go` L53-L64). - `newFilter()` starts `processLogs` via `sync.Once` (`filter.go` L97-L106); `Shutdown()` cancels the context **asynchronously**, so the old goroutine takes time to actually exit. - `resetGlobalState()` (`memory_leak_test.go` L42-L46) resets `accessLogFilter` and the `sync.Once`, but **cannot wait** for the previous goroutine to exit. - Net effect: new goroutine started (+1) while the leftover goroutine exits (-1) → count unchanged → `assert.Greater` fails. The exact number (`3 == 3`) matches CI because both tests run in the same process with identical baseline goroutines. ### Proposed Fix - Replace the global-goroutine-count heuristic with a **deterministic signal** owned by the filter, e.g. an atomic `started` flag or a `started` channel set inside `once.Do` right before `go processLogs()`, and assert on that instead of `runtime.NumGoroutine()`. - Optionally make `resetGlobalState()` drain a `done` channel so the previous goroutine is confirmed exited before the next test samples. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
