You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Qianlin1455 (via GitHub)" <gi...@apache.org> on 2026/10/08 11:02:39 UTC
[PR] fix(grpc): synchronize closing event test handler [dubbo-go]
Qianlin1455 opened a new pull request, #3755:
URL: https://github.com/apache/dubbo-go/pull/3755
Related to #3614
## Background
`TestGrpcHealthWatchEmitsClosingEvent` is one of the data races tracked in #3614.
The test starts the gRPC health-watch loop in a background goroutine. When a closing event is emitted, `testClosingEventHandler.HandleClosingEvent` appends the event to the shared `events` slice, while the test goroutine reads the same slice concurrently.
Running the test with the race detector reports concurrent reads and writes to `handler.events`.
## Root cause
`testClosingEventHandler.events` was accessed from multiple goroutines without synchronization.
The health-watch goroutine writes to the slice in `HandleClosingEvent`, while the test reads:
- `len(handler.events)`
- `handler.events[0]`
There was no synchronization establishing a happens-before relationship between these accesses.
## Changes
- Protect `testClosingEventHandler.events` with `sync.RWMutex`.
- Add `eventsSnapshot` to provide synchronized reads without exposing the internal slice.
- Update affected tests to use the snapshot instead of directly accessing `events`.
- Remove `TestGrpcHealthWatchEmitsClosingEvent` from `RACE_SKIP_TESTS`.
## Testing
```text
go test -race ./protocol/grpc -run "^TestGrpcHealthWatchEmitsClosingEvent$" -count=20
go test -race ./protocol/grpc -count=1
```
### Checklist
- [x] I confirm the target branch is `develop`
- [x] I have run `make fmt` to format my code
- [x] I have run `make test` to run local tests
- [ ] I have added tests that prove my fix is effective or that my feature works
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(grpc): synchronize closing event test handler [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3755:
URL: https://github.com/apache/dubbo-go/pull/3755#issuecomment-6059072848
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3755?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.56%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`32ec790`](https://app.codecov.io/gh/apache/dubbo-go/commit/32ec790f49fae3b30edfec74458a7d2e2aa2c596?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 985 commits behind head on develop.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3755 +/- ##
============================================
+ Coverage 46.76% 57.56% +10.79%
============================================
Files 295 487 +192
Lines 17172 40001 +22829
============================================
+ Hits 8031 23025 +14994
- Misses 8287 15193 +6906
- Partials 854 1783 +929
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go/pull/3755?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(grpc): synchronize closing event test handler [dubbo-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3755:
URL: https://github.com/apache/dubbo-go/pull/3755#discussion_r4219250543
##########
protocol/grpc/active_notify_test.go:
##########
@@ -130,10 +145,13 @@ func TestGrpcHealthWatchEmitsClosingEvent(t *testing.T) {
healthServer.SetServingStatus(serviceKey, grpc_health_v1.HealthCheckResponse_NOT_SERVING)
require.Eventually(t, func() bool {
- return len(handler.events) == 1
+ return len(handler.eventsSnapshot()) == 1
Review Comment:
In `require.Eventually`, `eventsSnapshot()` is called repeatedly and allocates/copies the slice each poll. This is test code, but it can still slow down/flake under `-race`. Consider adding a cheap synchronized accessor (e.g., `eventsLen()` under `RLock`), or replace the polling of snapshots with a signaling mechanism (e.g., send the event into a buffered channel in `HandleClosingEvent` and wait on it) to avoid repeated allocations and reduce timing sensitivity.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(grpc): synchronize closing event test handler [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl merged PR #3755:
URL: https://github.com/apache/dubbo-go/pull/3755
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(grpc): synchronize closing event test handler [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3755:
URL: https://github.com/apache/dubbo-go/pull/3755#issuecomment-6058453738
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3755) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3755&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3755&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3755&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3755&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3755&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3755)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org