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

http2 connections on localhost produce 'connection error: not connected' error on client close #3070

Closed
flumm opened this issue Dec 2, 2022 · 5 comments
Labels
C-bug Category: bug. Something is wrong. This is bad!

Comments

@flumm
Copy link

flumm commented Dec 2, 2022

Version
from Cargo.toml

hyper = { version = "0.14", features = [ "client", "server", "http2", "http1", "tcp" ] }
tokio = { version = "1.0", features = ["rt", "macros", "rt-multi-thread"] }
h2 = "0.3"
http = "0.2"

from cargo tree

h2 v0.3.15
hyper v0.14.23

Platform

Linux zita 5.15.74-1-pve #1 SMP PVE 5.15.74-1 (Mon, 14 Nov 2022 20:17:15 +0100) x86_64 GNU/Linux

Description
Closing a HTTP2 connection from the client side that resides on the same host (localhost/127.0.0.1),
the server sometimes gets an error:
'connection error: not connected'

This never happens (AFAICT) when having the server on a different host than the client

I tried this code (most copied from the hyper/h2 examples)

server:

use hyper::header::UPGRADE;
use hyper::service::{make_service_fn, service_fn};
use hyper::{Body, Request, Response, Server, StatusCode};
use std::convert::Infallible;
use std::net::SocketAddr;

async fn handle(_req: Request<Body>) -> Result<Response<Body>, Infallible> {
    Ok(Response::new(Body::from("Hello World")))
}

async fn start_h2(req: Request<Body>) -> Result<Response<Body>, Infallible> {
    // start the h2 future
    tokio::spawn(async move {
        let conn = match hyper::upgrade::on(req).await {
            Ok(conn) => conn,
            Err(err) => {
                eprintln!("error during upgrade: {err}");
                return;
            }
        };
        let mut http = hyper::server::conn::Http::new();
        http.http2_only(true);
        match http.serve_connection(conn, service_fn(handle)).await {
            Ok(_) => eprintln!("finished gracefully"),
            Err(err) => println!("ERROR: {err}"),
        }
    });

    Ok(Response::builder()
        .status(StatusCode::SWITCHING_PROTOCOLS)
        .header(UPGRADE, "h2c")
        .body(Body::empty())
        .unwrap())
}

#[tokio::main]
async fn main() {
    let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
    let make_service = make_service_fn(|_conn| async { Ok::<_, Infallible>(service_fn(start_h2)) });
    let server = Server::bind(&addr).serve(make_service);
    if let Err(e) = server.await {
        eprintln!("server error: {}", e);
    }
}

client:

use h2::client;

use http::{header::UPGRADE, Method, Request};
use hyper::Body;
use std::error::Error;

#[tokio::main]
pub async fn main() -> Result<(), Box<dyn Error>> {
    let client = hyper::Client::new();

    let req = Request::builder()
        .uri("http://127.0.0.1:3000/")
        .header(UPGRADE, "h2c")
        .body(Body::empty())?;
    let response = client.request(req).await?;
    let upgraded = hyper::upgrade::on(response).await?;

    let (h2, connection) = client::Builder::new()
        .handshake::<_, &[u8]>(upgraded)
        .await?;
    tokio::spawn(async move { connection.await });

    let mut h2 = h2.ready().await?;
    let request = Request::builder()
        .method(Method::GET)
        .uri("/")
        .body(())
        .unwrap();

    let (response, _) = h2.send_request(request, true).unwrap();

    let (head, _) = response.await?.into_parts();

    println!("Received response: {:?}", head);

    Ok(())
}

Letting the server run, and using the client in a loop, i get here ~50%

ERROR: connection error: not connected

How can we gracefully close the connection from the client without getting those errors?

@flumm flumm added the C-bug Category: bug. Something is wrong. This is bad! label Dec 2, 2022
@seanmonstar
Copy link
Member

the server sometimes gets an error: 'connection error: not connected'. This never happens (AFAICT) when having the server on a different host than the client.

Seems likely that the kernel is able to wrap up the shutdown near instantly when it is localhost, and so the error makes sense. It's not something I would expect hyper to silently swallow, so if you don't want it appearing in your reporting, you'll need to filter it yourself.

@seanmonstar seanmonstar closed this as not planned Won't fix, can't repro, duplicate, stale Dec 2, 2022
@flumm
Copy link
Author

flumm commented Dec 5, 2022

if it is not an error, why does hyper throw an error then?
and if it is an actual error, how can we decide if we can discard it or not?

is there no way for hyper to decide if it is a valid error or not?

@seanmonstar
Copy link
Member

seanmonstar commented Dec 5, 2022

if it is not an error, why does hyper throw an error then?

I didn't say it's not an error, I said it makes sense (the error code is ENOTCONN). The connection likely is closed very quickly, so the operating system says it is "not connected". It doesn't mean something catastrophic happened, you need to compare what the error means with the context of how it happened, and then decide if that is a big deal for your application, or something to ignore. hyper can't decide that for you.

@flumm
Copy link
Author

flumm commented Dec 5, 2022

ok i get what you're saying, but it's still not clear to me how we should determine if the error is valid or not since we don't have any additional context from hyper aside from the error.

basically a client connects and it should not be an error when the client disconnects normally. the question is how we can detect such a case (e.g. maybe you can give an example with my code examples from above?)

or asked differently, under what circumstances could such an ENOTCONN error happen (aside from the observed way here)

@Blub
Copy link

Blub commented Dec 5, 2022

the server sometimes gets an error: 'connection error: not connected'. This never happens (AFAICT) when having the server on a different host than the client.

Seems likely that the kernel is able to wrap up the shutdown near instantly when it is localhost, and so the error makes sense. It's not something I would expect hyper to silently swallow, so if you don't want it appearing in your reporting, you'll need to filter it yourself.

It doesn't actually make sense to me. Unless data needs to be sent, a closed socket shouldn't produce an ENOTCONN, or am I missing something? This is the error code for send()-ing to a client that disconnected unexpectedly.
Sounds like a bad race condition somewhere.

Unless the send futures get documented this way? But then every user would have to perform this test, which would duplicate the same piece of code for every user of the library :S
And an example of how to figure out whether it was an actual error would be useful.

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

No branches or pull requests

3 participants