hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.07k stars 1.55k forks source link

Add test for HTTP2 CONNECT termination #3655

Closed howardjohn closed 1 month ago

howardjohn commented 1 month ago

For https://github.com/hyperium/hyper/issues/3652.

In https://github.com/hyperium/hyper/pull/3647, I thought I tracked down the root cause of https://github.com/hyperium/hyper/issues/3652. However, with the help of @seanmonstar in https://github.com/hyperium/hyper/issues/3652#issuecomment-2085359895 I realized that this was too forcefully terminating the connection even if things other than SendRequest were still alive. I also realized the original test case I wrote was just wrong and failed to properly reproduce the issue.

This adds a test that properly reproduces the issue. On my machine, it fails about 5% of the time:

1876 runs so far, 100 failures (94.94% pass rate). 95.197349ms avg, 1.097347435s max, 5.398457ms min

With further investigation, I believe this bug actually originates in h2 itself. https://github.com/hyperium/h2/pull/772. With that PR, this test is 100% reliable

64010 runs so far, 0 failures (100.00% pass rate). 44.484057ms avg, 121.454709ms max, 1.872657ms min
seanmonstar commented 1 month ago

Thank you! I was waiting to release h2 with the fix, so we don't add a flaky test to hyper. The h2 v0.4.5 is out now :)

howardjohn commented 1 month ago

Ah makes sense. Thanks!