Panic when out of file descriptors

[nikclayton-dfinity]

https://github.com/tiny-http/tiny-http/blob/master/src/util/refined_tcp_stream.rs#L45 can panic when there are no available file descriptors.

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 24, kind: Other, message: "No file descriptors available" }', /source/tiny_http-0.7.0/src/util/refined_tcp_stream.rs:44:54

Rather than panic an error should be returned, so that the calling code can decide on the best course of action.

[bradfier]

This will be a breaking change, but we can definitely return a Result here rather than blindly panicking.

Some elaboration of why this is done this way below.

[bradfier]

Are you having the issue where a tiny-http::Server is panicking because of the code in RefinedTcpStream? As the latter type isn't exported by the library so I assume you aren't using it directly.

It would be useful if you could capture the above error with a full backtrace, but my suspicion is it's coming from the acceptor thread that dispatches to the worker pool.

Handling this situation is interesting, if the acceptor thread has run out of FDs, there's not a lot we can do with it. We need to use try_clone to split the TcpStream into a reader and a writer, and doing so consumes one additional file descriptor. Having the acceptor thread bubble an error up to the caller doesn't feel useful, as the connection we couldn't handle has already been lost.

[rawler]

FWIW, panic is also the results if the limit on threads is hit. As a consequence of the panic, a Mutex is poisoned, and recovery is pretty much impossible, AFAICT.

Feb 08 00:47:23: thread '' panicked at 'failed to spawn thread: Os { code: 11, kind: WouldBlock, message: "Resource temporarily unavailable" }', /rustc/db9d1b20bba1968c1ec1fc49616d474>
Feb 08 00:47:23: note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
Feb 08 00:47:24: thread '' panicked at 'failed to spawn thread: Os { code: 11, kind: WouldBlock, message: "Resource temporarily unavailable" }', /rustc/db9d1b20bba1968c1ec1fc49616d474>
Feb 08 00:47:29: thread '' panicked at 'called Result::unwrap() on an Err value: PoisonError { .. }', /home/ulrikm/.cargo-docker/registry/src/github.com-1ecc6299db9ec823/tiny_http>
Feb 08 00:47:30: thread '' panicked at 'called Result::unwrap() on an Err value: PoisonError { .. }', /home/ulrikm/.cargo-docker/registry/src/github.com-1ecc6299db9ec823/tiny_http>

[rawler]

Not sure what is the right approach to fix this. For me I typically use tiny_http as an embedded server in applications, where the web-serving is held in a side-thread. I think there should either be some mechanism to automatically recover from these errors, and/or it needs to be very clearly spelled out in the documentation that the application is expected to handle it.

[rawler]

Added tests for the two different scenarios of "Out of file-handles" and "Out of threads" to a draft PR. #219

[mleonhard]

I'm usingtiny-http and rouille as a stand-alone API server. I'm hitting the poison issue on thread spawn failure. I filed #220 and #221.

[rawler]

@bradfier Sorry, I realized first now I didn't exactly answer the question. The problem with how tiny-http works right now IMO, is that the internal Mutex is poisoned, and never recovers. That means that if the service temporarily hits too many concurrent connections, not only are the "overflowing" connections rejected, but the service is left permanently in a defunctional state. I.E. even if all other connections disconnect, the service will no longer accept new connections either.

This is what I tried to cover with the drafted test-cases. They start a server in a subprocess, arbitrarily confined with narrow resource-limits. The tests then make sure to hit those limits, (assert!(clients.iter().any(Result::is_err));), releases all connections, and tries connecting again.

I've clean up the tests a little bit to better illustrate.