Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add backgroundRead for plain http inbound #952

Merged
merged 2 commits into from
Jan 2, 2024

Conversation

hunshcn
Copy link

@hunshcn hunshcn commented Jan 2, 2024

fix #951

@Skyxim

This comment was marked as off-topic.

@hunshcn
Copy link
Author

hunshcn commented Jan 2, 2024

background Read 在 readRequest 结束之后进行,在 no keepalive 的 conn 中相当于检查conn健康性。
在 http/1.1 pipelining http 中,会读到下一个 request 的 byte,所以需要用 peek,不过 pipelining可能会导致这个检查失去作用,这是本身的缺陷。

以上思路和表述均来自标准库 https://github.com/golang/go/blob/go1.21.5/src/net/http/server.go#L682

@wwqgtxx
Copy link
Collaborator

wwqgtxx commented Jan 2, 2024

@hunshcn 然而你的startBackgroundRead精简的过于厉害,BufferedConnPeekRead之间并不是线程安全的,准确说是bufio.Reader本身就不是线程安全的,当for循环到下一次的ReadRequest时会造成严重的并发问题

@hunshcn
Copy link
Author

hunshcn commented Jan 2, 2024

@wwqgtxx 确实是这样,已加锁

@wwqgtxx
Copy link
Collaborator

wwqgtxx commented May 18, 2024

根据golang最新的commit,1.23将会禁止linkname到net/http.registerOnHitEOFnet/http.requestBodyRemains,我们需要在golang1.23 release之前找到替代实现方案实现该PR的功能,@hunshcn 有什么思路么
golang/go@41aab30#diff-041323d2d5c5f3fb41d70527b274a909c7ad27747339bdbfb5b6dc9b0c7e9129R1-R36

@hunshcn
Copy link
Author

hunshcn commented May 18, 2024

根据golang最新的commit,1.23将会禁止linkname到net/http.registerOnHitEOFnet/http.requestBodyRemains,我们需要在golang1.23 release之前找到替代实现方案实现该PR的功能,@hunshcn 有什么思路么 golang/go@41aab30#diff-041323d2d5c5f3fb41d70527b274a909c7ad27747339bdbfb5b6dc9b0c7e9129R1-R36

我们可以在 req.Body 做一次包裹,这样可以自行实现 registerOnHitEOF,并且弄起来也挺简单。

话说是怎么找到这个未发布的 cl 的。

@wwqgtxx
Copy link
Collaborator

wwqgtxx commented May 18, 2024

话说是怎么找到这个未发布的 cl 的。

其实就是顺手翻了翻master上的最新commit

在 req.Body 做一次包裹

看了一下目前的源码,总觉并不太容易实现,有没有兴趣发个PR处理一下

@hunshcn
Copy link
Author

hunshcn commented May 18, 2024

话说是怎么找到这个未发布的 cl 的。

其实就是顺手翻了翻master上的最新commit

在 req.Body 做一次包裹

看了一下目前的源码,总觉并不太容易实现,有没有兴趣发个PR处理一下

在 client.Do 之前对 request.Body 取出来然后做个包裹然后重写 Read,在 Read 获得 EOF 的时候调用 onHitEOF。和标准库里面实现逻辑一致。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants