Copilot commented on code in PR #3677:
URL: https://github.com/apache/dubbo-go/pull/3677#discussion_r3802645446
##########
tools/benchmark/client/engine/engine.go:
##########
@@ -114,8 +114,8 @@ func (e *Engine) worker(benchmarkFunc BenchmarkFunc) {
func (e *Engine) Stop() {
e.stopOnce.Do(func() {
close(e.stopChan)
- e.cancel()
e.wg.Wait()
+ e.cancel()
logger.Info("[INFO] Benchmark completed")
Review Comment:
Waiting for `e.wg.Wait()` before calling `e.cancel()` can cause `Stop()` to
block indefinitely if any in-flight work only terminates on context
cancellation (or if `benchmarkFunc`/downstream code ignores `stopChan` and
doesn’t reliably honor the per-request timeout in some edge cases). Consider
implementing a bounded drain: wait for the waitgroup up to a configurable/grace
timeout, then call `e.cancel()` to force shutdown (and optionally log that the
grace period elapsed). This preserves the goal of avoiding spurious
cancellations while preventing teardown hangs.
--
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]
