lizining1231 commented on code in PR #3677:
URL: https://github.com/apache/dubbo-go/pull/3677#discussion_r3803600323
##########
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:
谢谢你的评审!理论上确实可能永久阻塞
你说得对,`context.WithTimeout` 到期后只是关闭 `Done()` channel,不会强制 kill 掉 goroutine。如果
`benchmarkFunc` 内部是一个裸的 `conn.Read()` 没有设 deadline,有可能永久阻塞
但是我核实了一下,从实际代码来看
```go
func(ctx context.Context) (time.Duration, error) {
err := caller.Call(ctx)
}
```
`caller.Call(ctx)` 只有两种实现:
| 实现 | 调用 | 底层实现 |
|:---|:---|:---|
|
[dubbo_client.go#L83](https://github.com/apache/dubbo-go/blob/develop/tools/benchmark/client/clients/dubbo_client.go#L83)
| `c.client.UnaryCall(ctx, req)` | HTTP/2 transport,读响应时通过 `select { case
<-ctx.Done() }` 监听 context
取消([h2_bundle.go#L9926](https://github.com/golang/go/blob/go1.26.5/src/net/http/h2_bundle.go#L9926))
|
|
[grpc_client.go#L70](https://github.com/apache/dubbo-go/blob/develop/tools/benchmark/client/clients/grpc_client.go#L70)
| `c.client.UnaryCall(ctx, req)` | gRPC transport 通过 `select { case
<-ctx.Done() }` 取消
stream([transport.go#L232](https://github.com/grpc/grpc-go/blob/v1.82.1/internal/transport/transport.go#L232)),同时将
deadline 编码为 `grpc-timeout` header
通知服务端([http2_client.go#L607-L614](https://github.com/grpc/grpc-go/blob/v1.82.1/internal/transport/http2_client.go#L607-L614))
|
因此从ctx 层面看,30s 超时关闭;从transport 层面看,benchmarkFunc 也是30s超时关闭
所以实际上不会永久阻塞
--
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]