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

Listening.close() does not close the server #338

Closed
timonv opened this issue Feb 24, 2015 · 24 comments
Closed

Listening.close() does not close the server #338

timonv opened this issue Feb 24, 2015 · 24 comments
Labels
A-server Area: server. C-bug Category: bug. Something is wrong. This is bad!

Comments

@timonv
Copy link

timonv commented Feb 24, 2015

I'm trying to run a hyper server, but after closing the guard, I can't drop it and it hangs. Here's some example code where it happens (sorry for the muda, was trying a ton of things)

fn request_temp_code() -> TempCode {
    Command::new("xdg-open").arg(format!("https://slack.com/oauth/authorize?client_id={}", SLACK_CLIENT_ID)).output().unwrap();
    let server = Server::http(Ipv4Addr(127, 0, 0, 1), 9999);

    let (tx, rx) = channel();
    let mtx = Mutex::new(tx);

    let mut guard = server.listen(move |req: Request, res: Response| {
        match req.uri {
            AbsolutePath(path) => {
                // Trying to manually drop to see if this is the problem
                let tempcode = extract_temp_code(&path).unwrap();
                let gmtx = mtx.lock().unwrap();
                gmtx.send(tempcode).unwrap();
                drop(gmtx);
            },
            _ => ()
        }

        let mut res = res.start().unwrap();
        res.write_all(b"Thanks! Please return to Lax").unwrap();
        res.end().unwrap();
    }).unwrap();

    let tempcode = rx.recv().unwrap();
    guard.close().unwrap();
    println!("{}", "Auth code received!");
    println!("{}", "Server closed!");
    // Hangs here
    tempcode
} 

I haven't tried this with earlier versions of hyper (I think, not tracking numbers), however one of the last commits did involve changes related to the thread spawning into the acceptor. Have I found a possible bug?

@timonv timonv changed the title Hyper server seems to block (again?) Hyper server seems to block (again?) after expected non blocking call Feb 24, 2015
@timonv timonv changed the title Hyper server seems to block (again?) after expected non blocking call Hyper server seems to block after expected non blocking call Feb 24, 2015
@timonv
Copy link
Author

timonv commented Feb 24, 2015

I also tried unparking the thread, to no avail. If I try to manually drop the guard after closing it, it hangs. I added some statements to check if multiple requests happen, nothing.

@reem
Copy link
Contributor

reem commented Feb 24, 2015

Hmmm... It's likely that the guard returned by server.listen is incorrectly blocking for too long. I have to take another look at the code because this should work fine.

In order to facilitate fixing this, can you provide a standalone example that reproduces?

@timonv
Copy link
Author

timonv commented Feb 25, 2015

Sure! Let's see if I can do that in under ten minutes :-)

@timonv
Copy link
Author

timonv commented Feb 25, 2015

@reem
Copy link
Contributor

reem commented Feb 25, 2015

From the code in your example I think it's very likely that the problem was some deadlock in your code mixing channels and mutexes, since there is no reason hyper would treat that function differently than any other.

Closing, unless you can reproduce (since I can't :/).

@reem reem closed this as completed Feb 25, 2015
@timonv
Copy link
Author

timonv commented Feb 25, 2015

You can't? The example I gave doesn't work and reproduces the problem. :(

@timonv
Copy link
Author

timonv commented Feb 25, 2015

@reem Not sure if I need to mention for closed issues, but doing it anyway. Why would it deadlock? The mutex guard should be dropped right after the handler finishes it's shizzle. Is there anything else I can do to make this work? Maybe if Handler supported Copy I could just use an Arc.

I'd like to have this out of the way as its just the auth part of the project anyway :')

Edit: If its faster, I'm also on IRC.

@reem
Copy link
Contributor

reem commented Feb 25, 2015

@timonv I looked at the example more closely and the issue is actually clear, I just didn't spot it before. The guard is created in get_message, so get_message is going to block until the entire server is done, have get_message also return the guard or create it in main to make things work.

@timonv
Copy link
Author

timonv commented Feb 26, 2015

@reem So I can't just create a server, get the token and ditch the server again? I can have the guard in the main loop sure, but that seems like an ugly and unneeded solution, as it would essentially keep the entire server open for the duration of the app. If it blocks untill the entire server is done, then you'd kinda expect it to stop over time.

@reem
Copy link
Contributor

reem commented Feb 26, 2015

You can close the server by calling .close() on the guard. You should just do this after you receive the message, if you want to close the server right then.

@reem
Copy link
Contributor

reem commented Feb 26, 2015

However, know that hyper runs servers with a thread pool by default, so you should take care to set the number of threads to 1.

@timonv
Copy link
Author

timonv commented Feb 26, 2015

@reem I was already closing the guard. I tried moving to a single thread for gigs yesterday but it didn't work. https://github.com/timonv/hyper_blocking_example/blob/master/src/main.rs

The weird thing is, if you print some giberish at the end of the handler, it gets printed. What could Thread::scoped's join guard be waiting for? No trouble dropping the mutex guard either.

@reem
Copy link
Contributor

reem commented Feb 26, 2015

I will investigate this further, reopening.

@reem reem reopened this Feb 26, 2015
@ryansname
Copy link

@reem I seem to be facing the same issue in a similar use case: very simplified version. I've narrowed it down to a problem when the Listening struct is dropped.
Of note also is that the handler is still active after the call to close().
Is there anything I can help with to track down this problem?

@seanmonstar
Copy link
Member

At the moment, std::net::TcpListener has lost the ability to closed, which is what used to power this functionality. In order fix this, we would likely need some sort of channel that would send(Close), and the ListenerPool would need to recv on this channel to determine if it should continue listening. It would also need to close the listening port.

@seanmonstar seanmonstar added the A-server Area: server. label Apr 27, 2015
@seanmonstar seanmonstar changed the title Hyper server seems to block after expected non blocking call Listening.close() does not close the server Apr 27, 2015
@seanmonstar seanmonstar added the C-bug Category: bug. Something is wrong. This is bad! label Apr 27, 2015
@LukasKalbertodt
Copy link

I just want to remark that I'd like to use close. A solution would be appreciated! :)

@JohanLorenzo
Copy link
Contributor

I encountered this issue too. For reference, here's a simpler piece of code that shows Listening.close() doesn't work:

extern crate hyper;

use hyper::server::{ Server, Request, Response };
use std::thread;

fn hello(_: Request, res: Response) {
    res.send(b"Hello World!").unwrap();
}

fn main() {
    let mut listening = Server::http("127.0.0.1:8000").unwrap().handle(hello).unwrap();
    println!("SERVER STARTED");
    // Check with your OS that the port is open. For instance:
    // `watch -n 0.1 "lsof -Pan -i tcp  | grep 8000"`
    thread::sleep_ms(5000);
    listening.close();
    println!("SERVER STOPPED");
    // See the port is still open
    thread::sleep_ms(5000);
}

I wonder if we shouldn't update the documentation, until a fix gets merged.

@seanmonstar
Copy link
Member

It now does do something on master.

@t-botz
Copy link

t-botz commented Dec 1, 2016

I'm sorry but I don't understand why this bug has been closed. The problem still exists right?

I am doing intergration test and spin up a new hyper server for each test. I use a pool of port number but the thing is that Hyper never release the port after being closed meaning that I can't run all my tests...

@lilith
Copy link

lilith commented Dec 22, 2016

I have the same use case as @thibaultdelor, and am seeing the same problem. Using hyper 0.9.12 and rustc 1.14.0-nightly (5665bdf3e 2016-11-02)

@jmatraszek
Copy link

jmatraszek commented Feb 21, 2017

I am still having this issue (using iron 0.5.1, hyper 0.10.4, rustc 1.15.0). @seanmonstar should this issue be really closed?

BTW. Tried sleeping, as suggested here: #968 (comment)
My code: https://github.com/jmatraszek/haxonite/blob/6_serve_watch/src/main.rs#L132

@jgh-
Copy link

jgh- commented Apr 10, 2017

Yeah I'm seeing this on iron 0.5.1, hyper 0.10.7, and rustc 1.16.0 still. Calling close at least seems to let the drop happen without a deadlock, but the server is obviously not shut down.

from server/mod.rs (on 0.10.x branch):

impl Listening {
    /// Warning: This function doesn't work. The server remains listening after you called
    /// it. See https://github.com/hyperium/hyper/issues/338 for more details.
    ///
    /// Stop the server from listening to its socket address.
    pub fn close(&mut self) -> ::Result<()> {
        let _ = self._guard.take();
        debug!("closing server");
        Ok(())
    }
}

_guard is Option<JoinHandle<()>>, from the documentation on JoinHandle:

An owned permission to join on a thread (block on its termination).

A JoinHandle detaches the child thread when it is dropped.

So not only is this not closing the connection, it's also detaching the thread that was started.

@n3phtys
Copy link

n3phtys commented Nov 25, 2017

Definitely still happening with rustc 1.23.0-nightly and hyper 0.10.13. This issue should be reopened.

@seanmonstar
Copy link
Member

The 0.10 branch is no longer seeing development. A way to shutdown servers that works is in 0.11. Fixing this is non trivial, and so likely won't be fixed. If someone would like to submit a change that fixes this in 0.10, a new release could be made.

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

No branches or pull requests