Skip to content

Commit

Permalink
fix(net): set timeouts directly in accept
Browse files Browse the repository at this point in the history
Currently, timeouts are set in `Worker::handle_connection`, _after_ the
stream has been returned by `Listener::accept`. This is not quite right,
as a `Listener` implementation that wraps `HttpListener` might do I/O in
its own `accept` implementation. For that I/O, no timeouts have been set
yet.

This is not a purely theoretical concern either, as `HttpsListener` can
do this, depending on the implementation of `SslServer` it uses. I
suspect, but could not yet confirm, that this behavior is responsible
for issue #950.

As of this commit, the timeouts are set by `HttpListener`, directly
after the connection has been accepted. Please note that `Worker` also
still sets the timeouts. This redundancy will be cleaned up in the
following commits.
  • Loading branch information
hannobraun committed Jan 14, 2017
1 parent 2cc9e7c commit f5d4d65
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
46 changes: 45 additions & 1 deletion src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,22 @@ pub trait NetworkListener: Clone {
fn incoming(&mut self) -> NetworkConnections<Self> {
NetworkConnections(self)
}

/// Sets the read timeout for all streams that are accepted
fn set_read_timeout(&mut self, _: Option<Duration>) {
// This default implementation is only here to prevent the addition of
// these methods from being a breaking change. They should be removed
// when the next breaking release is made.
warn!("Ignoring read timeout");
}

/// Sets the write timeout for all streams that are accepted
fn set_write_timeout(&mut self, _: Option<Duration>) {
// This default implementation is only here to prevent the addition of
// these methods from being a breaking change. They should be removed
// when the next breaking release is made.
warn!("Ignoring write timeout");
}
}

/// An iterator wrapper over a `NetworkAcceptor`.
Expand Down Expand Up @@ -205,13 +221,19 @@ impl NetworkStream + Send {
/// A `NetworkListener` for `HttpStream`s.
pub struct HttpListener {
listener: TcpListener,

read_timeout : Option<Duration>,
write_timeout: Option<Duration>,
}

impl Clone for HttpListener {
#[inline]
fn clone(&self) -> HttpListener {
HttpListener {
listener: self.listener.try_clone().unwrap(),

read_timeout : self.read_timeout.clone(),
write_timeout: self.write_timeout.clone(),
}
}
}
Expand All @@ -220,6 +242,9 @@ impl From<TcpListener> for HttpListener {
fn from(listener: TcpListener) -> HttpListener {
HttpListener {
listener: listener,

read_timeout : None,
write_timeout: None,
}
}
}
Expand All @@ -236,13 +261,24 @@ impl NetworkListener for HttpListener {

#[inline]
fn accept(&mut self) -> ::Result<HttpStream> {
Ok(HttpStream(try!(self.listener.accept()).0))
let stream = HttpStream(try!(self.listener.accept()).0);
try!(stream.set_read_timeout(self.read_timeout));
try!(stream.set_write_timeout(self.write_timeout));
Ok(stream)
}

#[inline]
fn local_addr(&mut self) -> io::Result<SocketAddr> {
self.listener.local_addr()
}

fn set_read_timeout(&mut self, duration: Option<Duration>) {
self.read_timeout = duration;
}

fn set_write_timeout(&mut self, duration: Option<Duration>) {
self.write_timeout = duration;
}
}

#[cfg(windows)]
Expand Down Expand Up @@ -537,6 +573,14 @@ impl<S: SslServer + Clone> NetworkListener for HttpsListener<S> {
fn local_addr(&mut self) -> io::Result<SocketAddr> {
self.listener.local_addr()
}

fn set_read_timeout(&mut self, duration: Option<Duration>) {
self.listener.set_read_timeout(duration)
}

fn set_write_timeout(&mut self, duration: Option<Duration>) {
self.listener.set_write_timeout(duration)
}
}

/// A connector that can protect HTTP streams using SSL.
Expand Down
2 changes: 2 additions & 0 deletions src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,13 @@ impl<L: NetworkListener> Server<L> {

/// Sets the read timeout for all Request reads.
pub fn set_read_timeout(&mut self, dur: Option<Duration>) {
self.listener.set_read_timeout(dur);
self.timeouts.read = dur;
}

/// Sets the write timeout for all Response writes.
pub fn set_write_timeout(&mut self, dur: Option<Duration>) {
self.listener.set_write_timeout(dur);
self.timeouts.write = dur;
}
}
Expand Down

0 comments on commit f5d4d65

Please sign in to comment.