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

Hang on the first connection after dropping/re-creating the server #244

Open
jfaust opened this issue Jun 29, 2023 · 5 comments
Open

Hang on the first connection after dropping/re-creating the server #244

jfaust opened this issue Jun 29, 2023 · 5 comments

Comments

@jfaust
Copy link

jfaust commented Jun 29, 2023

I'm trying to use tiny-http as an oauth redirect server on localhost in a desktop app. Since I don't want the server running constantly, during login I:

  1. Create an http server
  2. Wait for the right request, and respond with a 302 redirect
  3. Destroy the http server

This works great the first time. The second time, the browser hangs. If I refresh the page, it works.

I managed to boil it down to this example:

fn main() {
    loop {
        let server = tiny_http::Server::http("127.0.0.1:9975").unwrap();
        println!("Now listening on port 9975");

        for mut rq in server.incoming_requests() {
            let response = tiny_http::Response::empty(302).with_header(
                tiny_http::Header::from_bytes(&b"Location"[..], &b"https://www.google.com/"[..])
                    .unwrap(),
            );
            let _ = rq.respond(response);
            break;
        }

        drop(server);

        std::thread::sleep(std::time::Duration::from_secs(1));
    }
}

To reproduce:

  1. Run this snippet
  2. Go to http://localhost:9975 in your browser. It will redirect to Google.
  3. Go to http://localhost:9975 in your browser again. It hangs.
  4. Go to http://localhost:9975 in your browser again. It works this time.

Interestingly, using curl instead of the browser does not reproduce the issue.

Also, if I change the port each time the problem goes away. However, if I loop between a fixed number of ports, as soon as a port is being recycled, the problem happens again.

This is on Windows. I haven't checked other platforms yet.

@jfaust
Copy link
Author

jfaust commented Jun 29, 2023

What seems to be happening is that after the first request (and subsequent dropping of the server), there's still a thread left reading from the socket:

 	[Inline Frame] hello-world.exe!std::sys::windows::net::Socket::recv_with_flags() Line 222	Rust
 	[Inline Frame] hello-world.exe!std::sys::windows::net::Socket::read() Line 244	Rust
 	[Inline Frame] hello-world.exe!std::sys_common::net::UdpSocket::recv() Line 676	Rust
 	hello-world.exe!std::net::udp::UdpSocket::recv() Line 695	Rust
 	hello-world.exe!tiny_http::connection::impl$2::read(enum2$<tiny_http::connection::Connection> * self, ref_mut$<slice2$<u8>>) Line 59	Rust
 	hello-world.exe!tiny_http::util::refined_tcp_stream::impl$3::read(enum2$<tiny_http::util::refined_tcp_stream::Stream> * self, ref_mut$<slice2$<u8>>) Line 84	Rust
 	hello-world.exe!tiny_http::util::refined_tcp_stream::impl$7::read(tiny_http::util::refined_tcp_stream::RefinedTcpStream * self, ref_mut$<slice2$<u8>>) Line 176	Rust
 	hello-world.exe!std::io::Read::read_buf::closure$0<tiny_http::util::refined_tcp_stream::RefinedTcpStream>(std::io::Read::read_buf::closure_env$0<tiny_http::util::refined_tcp_stream::RefinedTcpStream>) Line 821	Rust
 	hello-world.exe!std::io::default_read_buf<std::io::Read::read_buf::closure_env$0<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>(std::io::Read::read_buf::closure_env$0<tiny_http::util::refined_tcp_stream::RefinedTcpStream> read, std::io::readbuf::BorrowedCursor) Line 474	Rust
 	hello-world.exe!std::io::Read::read_buf<tiny_http::util::refined_tcp_stream::RefinedTcpStream>(tiny_http::util::refined_tcp_stream::RefinedTcpStream * self, std::io::readbuf::BorrowedCursor) Line 822	Rust
 	hello-world.exe!std::io::impls::impl$0::read_buf<tiny_http::util::refined_tcp_stream::RefinedTcpStream>(tiny_http::util::refined_tcp_stream::RefinedTcpStream * * self, std::io::readbuf::BorrowedCursor) Line 26	Rust
 	hello-world.exe!std::io::buffered::bufreader::buffer::Buffer::fill_buf<ref_mut$<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>(tiny_http::util::refined_tcp_stream::RefinedTcpStream * self) Line 114	Rust
 	hello-world.exe!std::io::buffered::bufreader::impl$4::fill_buf<tiny_http::util::refined_tcp_stream::RefinedTcpStream>(std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream> * self) Line 376	Rust
 	hello-world.exe!std::io::buffered::bufreader::impl$3::read<tiny_http::util::refined_tcp_stream::RefinedTcpStream>(std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream> * self, ref_mut$<slice2$<u8>>) Line 270	Rust
 	hello-world.exe!tiny_http::util::sequential::impl$4::read<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>(tiny_http::util::sequential::SequentialReader<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>> * self, ref_mut$<slice2$<u8>>) Line 121	Rust
 	hello-world.exe!std::io::impls::impl$0::read<tiny_http::util::sequential::SequentialReader<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>>(tiny_http::util::sequential::SequentialReader<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>> * * self, ref_mut$<slice2$<u8>>) Line 20	Rust
 	hello-world.exe!std::io::impl$20::next<ref_mut$<tiny_http::util::sequential::SequentialReader<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>>>(std::io::Bytes<ref_mut$<tiny_http::util::sequential::SequentialReader<std::io::buffered::bufreader::BufReader<tiny_http::util::refined_tcp_stream::RefinedTcpStream>>>> * self) Line 2718	Rust
 	hello-world.exe!tiny_http::client::ClientConnection::read_next_line() Line 85	Rust
 	hello-world.exe!tiny_http::client::ClientConnection::read() Line 110	Rust
>	hello-world.exe!tiny_http::client::impl$1::next(tiny_http::client::ClientConnection * self) Line 187	Rust
 	hello-world.exe!tiny_http::impl$3::from_listener::closure$0::closure$0<enum2$<tiny_http::connection::Listener>>(tiny_http::impl$3::from_listener::closure$0::closure_env$0<enum2$<tiny_http::connection::Listener>> *) Line 373	Rust
 	hello-world.exe!alloc::boxed::impl$46::call_mut<tuple$<>,dyn$<core::ops::function::FnMut<tuple$<>,assoc$<Output,tuple$<>>>,core::marker::Send>,alloc::alloc::Global>(alloc::boxed::Box<dyn$<core::ops::function::FnMut<tuple$<>,assoc$<Output,tuple$<>>>,core::marker::Send>,alloc::alloc::Global> * self) Line 1981	Rust
 	hello-world.exe!tiny_http::util::task_pool::impl$2::add_thread::closure$0(tiny_http::util::task_pool::impl$2::add_thread::closure_env$0) Line 124	Rust
 	hello-world.exe!std::sys_common::backtrace::__rust_begin_short_backtrace<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>(tiny_http::util::task_pool::impl$2::add_thread::closure_env$0 f) Line 140	Rust
 	hello-world.exe!std::thread::impl$0::spawn_unchecked_::closure$1::closure$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>(std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>) Line 527	Rust
 	hello-world.exe!core::panic::unwind_safe::impl$23::call_once<tuple$<>,std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>>(core::panic::unwind_safe::AssertUnwindSafe<std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>> self) Line 272	Rust
 	hello-world.exe!std::panicking::try::do_call<core::panic::unwind_safe::AssertUnwindSafe<std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>>,tuple$<>>(unsigned char * data) Line 487	Rust
 	[External Code]	
 	hello-world.exe!std::panicking::try<tuple$<>,core::panic::unwind_safe::AssertUnwindSafe<std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>>>(core::panic::unwind_safe::AssertUnwindSafe<std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>> f) Line 449	Rust
 	hello-world.exe!std::panic::catch_unwind<core::panic::unwind_safe::AssertUnwindSafe<std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>>,tuple$<>>(core::panic::unwind_safe::AssertUnwindSafe<std::thread::impl$0::spawn_unchecked_::closure$1::closure_env$0<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>> f) Line 141	Rust
 	hello-world.exe!std::thread::impl$0::spawn_unchecked_::closure$1<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>(std::thread::impl$0::spawn_unchecked_::closure_env$1<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>) Line 525	Rust
 	hello-world.exe!core::ops::function::FnOnce::call_once<std::thread::impl$0::spawn_unchecked_::closure_env$1<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>>,tuple$<>>(std::thread::impl$0::spawn_unchecked_::closure_env$1<tiny_http::util::task_pool::impl$2::add_thread::closure_env$0,tuple$<>> *) Line 250	Rust
 	[Inline Frame] hello-world.exe!alloc::boxed::impl$45::call_once() Line 1973	Rust
 	[Inline Frame] hello-world.exe!alloc::boxed::impl$45::call_once() Line 1973	Rust
 	hello-world.exe!std::sys::windows::thread::impl$0::new::thread_start() Line 56	Rust

Why is it saying it's in UdpSocket::recv instead of TcpSocket::recv? No idea.

@jfaust
Copy link
Author

jfaust commented Jun 29, 2023

This makes some sense - the connection is being kept alive (since the browser asked it to be via Connection: keep-alive), so the socket remains open, sending requests into a message queue with no one on the other side. When the connection doesn't generate a response, presumably the browser closes its end of the socket, which is why the next request works.

@jfaust
Copy link
Author

jfaust commented Jun 29, 2023

Explicitly setting a Connection: close header in the response works - however, tiny-http prevents this in Response::add_header(). Thoughts on removing that check? Or adding another function to Response, like request_close().

@kolbma
Copy link

kolbma commented Dec 19, 2023

  1. May I ask why you need a server for this? Isn't it possible for the desktop app to provide this logic?
  2. What has the browser to do with your app intentions? In your issue it is the browser which tries to reuse connections, after the server has exited.
  3. Can't you send a Connection: close with your "single-response" request, to make it clear for the client that there will be no need for keep-alive? Or a smaller HTTP/1.0-request?
  4. Does it work with setting response header Keep-Alive: timeout=1, max=1?

You could also use recv() instead of incoming_requests(), simplifies the above code at #244 (comment)

The thing with Connection-header is, that setting wrong values results in different/unspecified client behavior and so there should be an enum and some logic for validation.

@jfaust
Copy link
Author

jfaust commented Dec 19, 2023

I ended up switching to hyper for this use case, which has worked, so this is no longer an issue on my end, but not sure if you want to leave this open or not.

  1. It's a server for the browser hit as part of an oauth login flow - logging in opens the browser, the user logs in there, and then the browser redirects to localhost. We only want to have the server running during the login, hence shutting it down. This is possible via custom URIs, but those have restrictions that make them not as nice as using an actual server.
  2. It's not a common case, but where we hit it was basically if the user logs in, logs out and immediately logs in again - the browser hangs during the redirect, and never successfully redirects to the app.

For (3) and (4) I'm not sure, as I don't have this code path to try anymore.

Agreed that the Connection header is probably the wrong way of doing this - I think if the underlying issue can be fixed (there's a TCP connection being kept alive with no handler for it) that's probably a better option.

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

No branches or pull requests

2 participants