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

Hyper does not appear to drop chained future on disconnect #1360

Closed
cetra3 opened this issue Oct 23, 2017 · 6 comments
Closed

Hyper does not appear to drop chained future on disconnect #1360

cetra3 opened this issue Oct 23, 2017 · 6 comments
Labels
A-client Area: client. C-bug Category: bug. Something is wrong. This is bad!

Comments

@cetra3
Copy link

cetra3 commented Oct 23, 2017

This is a weird one, and I'm not sure where the problem lies exactly but I am seeing that connections are hanging around when doing a hyper proxy-like configuration, so it might be a bug in tokio-core too.

What I'm seeing though is when I'm trying to stream a proxy request (like in this example), the TCP connection drops off from the client -> proxy, but from the proxy -> server, if the client prematurely disconnects, then this second connection stays open.

This was seen by proxying Partial Content requests, especially with a video or other media, and skipping around.

What I'd like to know is when a tcp connection is cut off, is this meant to propagate down the future chain? I would've thought it would cause the whole thing to disconnect, but I'm also not sure when hyper actually disconnects.

@seanmonstar
Copy link
Member

I'm not certain, but this sounds similar to #1353. I can confirm what that issue reports, that if the Body from a client response is dropped before reading to the end, the connection just gets leaked into the reactor. There's work in #1362 that may fix this.

@cetra3
Copy link
Author

cetra3 commented Oct 27, 2017

It does look very similar. I'll give that PR a shot to see if it's resolved

@cetra3
Copy link
Author

cetra3 commented Oct 30, 2017

I've tested the latest master (commit af8d11b2bf4ebe1f69fc1ad1251ae58afd14d6ca), and this breaks stuff on normal connections, causing sockets to stay in TIME_WAIT.

In other words, the opposite is now happening: It's leaking connections on a successful attempt.

My code to test this is as follows:

Cargo.toml:

[package]
name = "leak_test"
version = "0.1.0"

[dependencies]
hyper = {git = "https://github.com/hyperium/hyper"}
shio = "*"

[patch.crates-io]
hyper = {git = "https://github.com/hyperium/hyper"}

src/main.rs:

extern crate hyper;
extern crate shio;

use shio::prelude::*;
use hyper::Client;

fn proxy(ctx: Context) -> BoxFuture<Response, hyper::Error> {
    Client::configure().no_proto().build(ctx.handle())
        .get("http://www.google.com".parse().unwrap())
        .map(|res| Response::build().body(res.body()))
        .into_box()
}

fn main() {
    Shio::new(proxy).run(":7878").unwrap();
}

Then run the following in a terminal:

# Connect 100 times
ab -n 100 http://localhost:7878
# Wait a bit for sockets to close
sleep 20
# Check how many active sockets there are
lsof -i -n -P | grep leak_test

You will see 100 open sockets that don't go away:

leak_test 18478 cetra   53u  IPv4 0xa5f869b873e90b7      0t0  TCP 192.168.1.143:50030->216.58.220.100:80 (ESTABLISHED)
leak_test 18478 cetra   54u  IPv4 0xa5f869b873e50b7      0t0  TCP 192.168.1.143:50032->216.58.220.100:80 (ESTABLISHED)
leak_test 18478 cetra   55u  IPv4 0xa5f869b871779bf      0t0  TCP 192.168.1.143:50034->216.58.220.100:80 (ESTABLISHED)
leak_test 18478 cetra   56u  IPv4 0xa5f869b872809bf      0t0  TCP 192.168.1.143:50036->216.58.220.100:80 (ESTABLISHED)
leak_test 18478 cetra   57u  IPv4 0xa5f869b872aaddf      0t0  TCP 192.168.1.143:50038->216.58.220.100:80 (ESTABLISHED)
leak_test 18478 cetra   58u  IPv4 0xa5f869b744e37af      0t0  TCP 192.168.1.143:50040->216.58.220.100:80 (ESTABLISHED)
...

@seanmonstar
Copy link
Member

I've pushed a fix for when the Client has been dropped before the response finishes, along with a test for this case, to master.

@seanmonstar seanmonstar added A-client Area: client. C-bug Category: bug. Something is wrong. This is bad! labels Oct 30, 2017
@seanmonstar
Copy link
Member

This is in v0.11.7 (I believe requires no_proto still).

@cetra3
Copy link
Author

cetra3 commented Nov 14, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: client. C-bug Category: bug. Something is wrong. This is bad!
Projects
None yet
Development

No branches or pull requests

2 participants