lizining1231 commented on code in PR #3749:
URL: https://github.com/apache/dubbo-go/pull/3749#discussion_r4091258350
##########
protocol/triple/triple_protocol/duplex_http_call.go:
##########
@@ -125,6 +123,17 @@ func (d *duplexHTTPCall) Write(data []byte) (int, error) {
return bytesWritten, err
}
+// checkBeforeWrite checks cancellation without starting a live request, so
+// buffered writes can keep request headers mutable until their first flush.
+func (d *duplexHTTPCall) checkBeforeWrite() error {
+ if err := d.ctx.Err(); err != nil {
+ d.ensureRequestMade()
+ d.SetError(err)
+ return wrapIfContextError(err)
+ }
+ return nil
+}
+
Review Comment:
为了让 `buffered writer` 在真正写入底层 `transport` 之前,复用 `duplexHTTPCall` 原有的
`context` 取消检查
未缓冲路径里,`Write` 开头会先检查 `d.ctx.Err()`,所以已经取消的 `context` 会立即返回
`CodeCanceled`。但开启 `write buffering` 后,小包会先进入 `streamBufferWriter`
的内存缓冲,不一定马上调用 `duplexHTTPCall.Write`,导致这道检查被绕过。`Send` 可能错误地返回
`nil`,容易让调用方误以为数据已写入。
所以这里拆了一个非导出的 `checkBeforeWrite`:
- `ctx` 未取消时只做检查,不调用 `ensureRequestMade`,保持“第一次 `flush` 才真正发起请求”的语义。
- `ctx` 已取消时复用 `Write` 原本的错误处理行为,设置错误状态,并返回同样的 `context error`。
也就是说,当前通过一个内部接口调用 `checkBeforeWrite()`,让缓冲器在收下数据前完成检查,同时由 `duplexHTTPCall`
维护自身的错误状态。代价是多了一个接口。
不抽函数也有两种做法:
1. 在 `streamBufferWriter` 中识别底层是否为 `*duplexHTTPCall`,再直接访问它的
`ctx`,并重复处理请求启动、错误记录和错误转换。这样缓冲器会依赖这个具体的 `HTTP` 实现,检查逻辑也会分散在两处
2. 创建缓冲器时额外传入 `ctx`,让它自行检查取消状态。这样无需识别 `duplexHTTPCall`。不过只检查 `ctx`
还不够,取消时底层原有的请求启动和错误记录也需要同步处理,否则后续读写行为可能与未缓冲路径不同,这个就比较麻烦
--
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]