Open howardjohn opened 1 month ago
That is surprising. The Connection
is design to stay alive until all shared references to it are gone. Those are the SendRequest
, and any ResponseFuture
, SendStream
, and RecvStream
. Perhaps something about the way the types are stored inside the Upgraded
makes it drop the reference and no longer be counted for liveness?
cc @nox, who worked on this previously.
Ah, I think I got things a little mixed up.
In https://github.com/hyperium/hyper/pull/3647, I sent a test and a fix.
Upgraded
which kept the connection alive as expected.SendRequest
s dropped, even if there were SendStream
, RecvStream
, etc left (via Upgraded
). So it is the cause of the issue describedThat being said, I do still see the issue in our application despite dropping everything, I am just unable to reproduce it in a unit test.
Ok I think I figured it out and put up two PRs:
Version Hyper 1.3
Platform
Description
We are seeing a few issues around using H2 CONNECT. Our application is using Tokio, Rustls, and Hyper to communicate between a client and server both using the same stack. We have multiple streams on one TCP connection sometimes (though the issue occurs regardless of multiplexing).
Our first issue that popped up was leaking connections. This was debugging and an (attempted) fix was opened in https://github.com/hyperium/hyper/pull/3647. More details there. This was like not detected before because:
With that fix, everything was working fine. However, a later refactor in our broke things again; we started dropping the
SendRequest
before we were complete with IO operations onUpgraded
(before, we dropped theSendRequest
after). This causes the stream/connection to be closed unexpectedly. This can be reproduced easily by moving thedrop(client)
in https://github.com/hyperium/hyper/pull/3647 to before the write/read operations. This is easy to workaround, but its not documented and @seanmonstar suggested it was not expected on Discord.