diff --git a/server.go b/server.go index ef73677985..cb87f9ca53 100644 --- a/server.go +++ b/server.go @@ -199,19 +199,26 @@ type Server struct { // Default buffer size is used if not set. WriteBufferSize int - // Maximum duration for reading the full request (including body). - // - // This also limits the maximum duration for idle keep-alive - // connections. + // ReadTimeout is the amount of time allowed to read + // the full request including body. The connection's read + // deadline is reset when the connection opens, or for + // keep-alive connections after the first byte has been read. // // By default request read timeout is unlimited. ReadTimeout time.Duration - // Maximum duration for writing the full response (including body). + // WriteTimeout is the maximum duration before timing out + // writes of the response. It is reset after the request handler + // has returned. // // By default response write timeout is unlimited. WriteTimeout time.Duration + // IdleTimeout is the maximum amount of time to wait for the + // next request when keep-alives are enabled. If IdleTimeout + // is zero, the value of ReadTimeout is used. + IdleTimeout time.Duration + // Maximum number of concurrent client connections allowed per IP. // // By default unlimited number of concurrent connections @@ -226,15 +233,8 @@ type Server struct { // By default unlimited number of requests may be served per connection. MaxRequestsPerConn int - // Maximum keep-alive connection lifetime. - // - // The server closes keep-alive connection after its' lifetime - // expiration. - // - // See also ReadTimeout for limiting the duration of idle keep-alive - // connections. - // - // By default keep-alive connection lifetime is unlimited. + // MaxKeepaliveDuration is a no-op and only left here for backwards compatibility. + // Deprecated: Use IdleTimeout instead. MaxKeepaliveDuration time.Duration // Whether to enable tcp keep-alive connections. @@ -353,7 +353,6 @@ type Server struct { readerPool sync.Pool writerPool sync.Pool hijackConnPool sync.Pool - bytePool sync.Pool // We need to know our listener so we can close it in Shutdown(). ln net.Listener @@ -463,8 +462,6 @@ type RequestCtx struct { userValues userData - lastReadDuration time.Duration - connID uint64 connRequestNum uint64 connTime time.Time @@ -657,10 +654,9 @@ type ctxLogger struct { } func (cl *ctxLogger) Printf(format string, args ...interface{}) { - ctxLoggerLock.Lock() msg := fmt.Sprintf(format, args...) - ctx := cl.ctx - cl.logger.Printf("%.3f %s - %s", time.Since(ctx.Time()).Seconds(), ctx.String(), msg) + ctxLoggerLock.Lock() + cl.logger.Printf("%.3f %s - %s", time.Since(cl.ctx.ConnTime()).Seconds(), cl.ctx.String(), msg) ctxLoggerLock.Unlock() } @@ -1711,10 +1707,6 @@ var ( // ErrConcurrencyLimit may be returned from ServeConn if the number // of concurrently served connections exceeds Server.Concurrency. ErrConcurrencyLimit = errors.New("cannot serve the connection because Server.Concurrency concurrent connections are served") - - // ErrKeepaliveTimeout is returned from ServeConn - // if the connection lifetime exceeds MaxKeepaliveDuration. - ErrKeepaliveTimeout = errors.New("exceeded MaxKeepaliveDuration") ) // ServeConn serves HTTP requests from the given connection. @@ -1799,6 +1791,13 @@ func nextConnID() uint64 { // See Server.MaxRequestBodySize for details. const DefaultMaxRequestBodySize = 4 * 1024 * 1024 +func (s *Server) idleTimeout() time.Duration { + if s.IdleTimeout != 0 { + return s.IdleTimeout + } + return s.ReadTimeout +} + func (s *Server) serveConn(c net.Conn) error { defer atomic.AddInt32(&s.open, -1) @@ -1817,8 +1816,7 @@ func (s *Server) serveConn(c net.Conn) error { } connRequestNum := uint64(0) connID := nextConnID() - currentTime := time.Now() - connTime := currentTime + connTime := time.Now() maxRequestBodySize := s.MaxRequestBodySize if maxRequestBodySize <= 0 { maxRequestBodySize = DefaultMaxRequestBodySize @@ -1835,9 +1833,6 @@ func (s *Server) serveConn(c net.Conn) error { timeoutResponse *Response hijackHandler HijackHandler - lastReadDeadlineTime time.Time - lastWriteDeadlineTime time.Time - connectionClose bool isHTTP11 bool @@ -1845,27 +1840,48 @@ func (s *Server) serveConn(c net.Conn) error { ) for { connRequestNum++ - ctx.time = currentTime - if s.ReadTimeout > 0 || s.MaxKeepaliveDuration > 0 { - lastReadDeadlineTime = s.updateReadDeadline(c, ctx, lastReadDeadlineTime) - if lastReadDeadlineTime.IsZero() { - err = ErrKeepaliveTimeout - break + // If this is a keep-alive connection set the idle timeout. + if connRequestNum > 1 { + if d := s.idleTimeout(); d > 0 { + if err := c.SetReadDeadline(time.Now().Add(d)); err != nil { + panic(fmt.Sprintf("BUG: error in SetReadDeadline(%s): %s", d, err)) + } } } - if !(s.ReduceMemoryUsage || ctx.lastReadDuration > time.Second) || br != nil { + if !s.ReduceMemoryUsage || br != nil { if br == nil { br = acquireReader(ctx) } + + // If this is a keep-alive connection we want to try and read the first bytes + // within the idle time. + if connRequestNum > 1 { + var b []byte + b, err = br.Peek(4) + if len(b) == 0 { + // If reading from a keep-alive connection returns nothing it means + // the connection was closed (either timeout or from the other side). + err = errNothingRead + } + } } else { + // If this is a keep-alive connection acquireByteReader will try to peek + // a couple of bytes already so the idle timeout will already be used. br, err = acquireByteReader(&ctx) } + ctx.Request.isTLS = isTLS ctx.Response.Header.noDefaultContentType = s.NoDefaultContentType if err == nil { + if s.ReadTimeout > 0 { + if err := c.SetReadDeadline(time.Now().Add(s.ReadTimeout)); err != nil { + panic(fmt.Sprintf("BUG: error in SetReadDeadline(%s): %s", s.ReadTimeout, err)) + } + } + if s.DisableHeaderNamesNormalizing { ctx.Request.Header.DisableNormalizing() ctx.Response.Header.DisableNormalizing() @@ -1882,9 +1898,6 @@ func (s *Server) serveConn(c net.Conn) error { } } - currentTime = time.Now() - ctx.lastReadDuration = currentTime.Sub(ctx.time) - if err != nil { if err == io.EOF { err = nil @@ -1941,7 +1954,7 @@ func (s *Server) serveConn(c net.Conn) error { } ctx.connID = connID ctx.connRequestNum = connRequestNum - ctx.time = currentTime + ctx.time = time.Now() s.Handler(ctx) timeoutResponse = ctx.timeoutResponse @@ -1969,8 +1982,10 @@ func (s *Server) serveConn(c net.Conn) error { ctx.SetConnectionClose() } - if s.WriteTimeout > 0 || s.MaxKeepaliveDuration > 0 { - lastWriteDeadlineTime = s.updateWriteDeadline(c, ctx, lastWriteDeadlineTime) + if s.WriteTimeout > 0 { + if err := c.SetWriteDeadline(time.Now().Add(s.WriteTimeout)); err != nil { + panic(fmt.Sprintf("BUG: error in SetWriteDeadline(%s): %s", s.WriteTimeout, err)) + } } connectionClose = connectionClose || ctx.Response.ConnectionClose() @@ -2019,8 +2034,8 @@ func (s *Server) serveConn(c net.Conn) error { hjr = br br = nil - // br may point to ctx.fbr, so do not return ctx into pool. - ctx = s.acquireCtx(c) + // br may point to ctx.fbr, so do not return ctx into pool below. + ctx = nil } if bw != nil { err = bw.Flush() @@ -2038,7 +2053,6 @@ func (s *Server) serveConn(c net.Conn) error { break } - currentTime = time.Now() s.setState(c, StateIdle) if atomic.LoadInt32(&s.stop) == 1 { @@ -2053,13 +2067,15 @@ func (s *Server) serveConn(c net.Conn) error { if bw != nil { releaseWriter(s, bw) } - // in unexpected cases the for loop will break - // before request reset call. in such cases, call it before - // release to fix #548 - if !reqReset { - ctx.Request.Reset() + if ctx != nil { + // in unexpected cases the for loop will break + // before request reset call. in such cases, call it before + // release to fix #548 + if !reqReset { + ctx.Request.Reset() + } + s.releaseCtx(ctx) } - s.releaseCtx(ctx) return err } @@ -2069,59 +2085,6 @@ func (s *Server) setState(nc net.Conn, state ConnState) { } } -func (s *Server) updateReadDeadline(c net.Conn, ctx *RequestCtx, lastDeadlineTime time.Time) time.Time { - readTimeout := s.ReadTimeout - currentTime := ctx.time - if s.MaxKeepaliveDuration > 0 { - connTimeout := s.MaxKeepaliveDuration - currentTime.Sub(ctx.connTime) - if connTimeout <= 0 { - return zeroTime - } - if connTimeout < readTimeout { - readTimeout = connTimeout - } - } - - // Optimization: update read deadline only if more than 25% - // of the last read deadline exceeded. - // See https://github.com/golang/go/issues/15133 for details. - if currentTime.Sub(lastDeadlineTime) > (readTimeout >> 2) { - if err := c.SetReadDeadline(currentTime.Add(readTimeout)); err != nil { - panic(fmt.Sprintf("BUG: error in SetReadDeadline(%s): %s", readTimeout, err)) - } - lastDeadlineTime = currentTime - } - return lastDeadlineTime -} - -func (s *Server) updateWriteDeadline(c net.Conn, ctx *RequestCtx, lastDeadlineTime time.Time) time.Time { - writeTimeout := s.WriteTimeout - if s.MaxKeepaliveDuration > 0 { - connTimeout := s.MaxKeepaliveDuration - time.Since(ctx.connTime) - if connTimeout <= 0 { - // MaxKeepAliveDuration exceeded, but let's try sending response anyway - // in 100ms with 'Connection: close' header. - ctx.SetConnectionClose() - connTimeout = 100 * time.Millisecond - } - if connTimeout < writeTimeout { - writeTimeout = connTimeout - } - } - - // Optimization: update write deadline only if more than 25% - // of the last write deadline exceeded. - // See https://github.com/golang/go/issues/15133 for details. - currentTime := time.Now() - if currentTime.Sub(lastDeadlineTime) > (writeTimeout >> 2) { - if err := c.SetWriteDeadline(currentTime.Add(writeTimeout)); err != nil { - panic(fmt.Sprintf("BUG: error in SetWriteDeadline(%s): %s", writeTimeout, err)) - } - lastDeadlineTime = currentTime - } - return lastDeadlineTime -} - func hijackConnHandler(r io.Reader, c net.Conn, s *Server, h HijackHandler) { hjc := s.acquireHijackConn(r, c) h(hjc) @@ -2209,7 +2172,6 @@ func acquireByteReader(ctxP **RequestCtx) (*bufio.Reader, error) { ctx := *ctxP s := ctx.s c := ctx.c - t := ctx.time s.releaseCtx(ctx) // Make GC happy, so it could garbage collect ctx @@ -2217,16 +2179,10 @@ func acquireByteReader(ctxP **RequestCtx) (*bufio.Reader, error) { ctx = nil *ctxP = nil - v := s.bytePool.Get() - if v == nil { - v = make([]byte, 1) - } - b := v.([]byte) - n, err := c.Read(b) - ch := b[0] - s.bytePool.Put(v) + var b [1]byte + n, err := c.Read(b[:]) + ctx = s.acquireCtx(c) - ctx.time = t *ctxP = ctx if err != nil { // Treat all errors as EOF on unsuccessful read @@ -2238,7 +2194,7 @@ func acquireByteReader(ctxP **RequestCtx) (*bufio.Reader, error) { } ctx.fbr.c = c - ctx.fbr.ch = ch + ctx.fbr.ch = b[0] ctx.fbr.byteRead = false r := acquireReader(ctx) r.Reset(&ctx.fbr) @@ -2310,7 +2266,6 @@ func (ctx *RequestCtx) Init2(conn net.Conn, logger Logger, reduceMemoryUsage boo ctx.s = fakeServer ctx.connRequestNum = 0 ctx.connTime = time.Now() - ctx.time = ctx.connTime keepBodyBuffer := !reduceMemoryUsage ctx.Request.keepBodyBuffer = keepBodyBuffer diff --git a/server_test.go b/server_test.go index 5c87926830..8d7d97f100 100644 --- a/server_test.go +++ b/server_test.go @@ -2283,51 +2283,6 @@ func TestServerTimeoutError(t *testing.T) { } } -func TestServerMaxKeepaliveDuration(t *testing.T) { - s := &Server{ - Handler: func(ctx *RequestCtx) { - time.Sleep(20 * time.Millisecond) - }, - MaxKeepaliveDuration: 10 * time.Millisecond, - } - - rw := &readWriter{} - rw.r.WriteString("GET /aaa HTTP/1.1\r\nHost: aa.com\r\n\r\n") - rw.r.WriteString("GET /bbbb HTTP/1.1\r\nHost: bbb.com\r\n\r\n") - - ch := make(chan error) - go func() { - ch <- s.ServeConn(rw) - }() - - select { - case err := <-ch: - if err != nil { - t.Fatalf("Unexpected error from serveConn: %s", err) - } - case <-time.After(100 * time.Millisecond): - t.Fatalf("timeout") - } - - br := bufio.NewReader(&rw.w) - var resp Response - if err := resp.Read(br); err != nil { - t.Fatalf("Unexpected error when parsing response: %s", err) - } - if !resp.ConnectionClose() { - t.Fatalf("Response must have 'connection: close' header") - } - verifyResponseHeader(t, &resp.Header, 200, 0, string(defaultContentType)) - - data, err := ioutil.ReadAll(br) - if err != nil { - t.Fatalf("Unexpected error when reading remaining data: %s", err) - } - if len(data) != 0 { - t.Fatalf("Unexpected data read after the first response %q. Expecting %q", data, "") - } -} - func TestServerMaxRequestsPerConn(t *testing.T) { s := &Server{ Handler: func(ctx *RequestCtx) {},