DATA RACE: Is this an Echo Data Race or is it something I am doing

[wayneforrest]

Issue Description

I am experiencing a DATA RACE and I am not sure what the cause is. Is this an Echo Issue or is it something that I am doing?

I interpret this as the context is being reset, the write operation, while I try to read the request : echo.Contex.Request().

`WARNING: DATA RACE
Write at 0x00c000168a00 by goroutine 15298:
  github.com/labstack/echo/v4.(*context).Reset()
      /Users/user/go/pkg/mod/github.com/labstack/echo/[email protected]/context.go:626 +0x70
  github.com/labstack/echo/v4.(*Echo).ServeHTTP()
      /Users/user/go/pkg/mod/github.com/labstack/echo/[email protected]/echo.go:612 +0x68
  net/http.serverHandler.ServeHTTP()
      /opt/homebrew/Cellar/go/1.19.2/libexec/src/net/http/server.go:2947 +0x4f4
  net/http.initALPNRequest.ServeHTTP()
      /opt/homebrew/Cellar/go/1.19.2/libexec/src/net/http/server.go:3556 +0x290
  net/http.(*initALPNRequest).ServeHTTP()
      <autogenerated>:1 +0x74
  net/http.Handler.ServeHTTP-fm()
      <autogenerated>:1 +0x60
  net/http.(*http2serverConn).runHandler()
      /opt/homebrew/Cellar/go/1.19.2/libexec/src/net/http/h2_bundle.go:5911 +0x98
  net/http.(*http2serverConn).processHeaders.func1()
      /opt/homebrew/Cellar/go/1.19.2/libexec/src/net/http/h2_bundle.go:5641 +0x58

Previous read at 0x00c000168a00 by goroutine 15262:
  github.com/labstack/echo/v4.(*context).Request()
      /Users/user/go/pkg/mod/github.com/labstack/echo/[email protected]/context.go:232 +0x2c
  main.serveFile()
      /Users/user/Documents/Projects/Racing/racer/cmd/server.go:139 +0x174
  main.handleGetRequest.func1()
      /Users/user/Documents/Projects/Racing/racer/cmd/server.go:208 +0x2a0
  github.com/labstack/echo/v4.(*Echo).add.func1()
      /Users/user/go/pkg/mod/github.com/labstack/echo/[email protected]/echo.go:520 +0x70
  github.com/labstack/echo/v4/middleware.echoHandlerFuncWrapper.ServeHTTP()
      /Users/user/go/pkg/mod/github.com/labstack/echo/[email protected]/middleware/timeout.go:164 +0x16c
  github.com/labstack/echo/v4/middleware.(*echoHandlerFuncWrapper).ServeHTTP()
      <autogenerated>:1 +0x8c
  net/http.(*timeoutHandler).ServeHTTP.func1()
      /opt/homebrew/Cellar/go/1.19.2/libexec/src/net/http/server.go:3407 +0xac`

[aldas]

without seeing the code, probably easiest would be just to drop the timeout middleware.

[wayneforrest]

I have just read the WARNING, in timeout.go so moving my timeout middleware to be the first middleware:

[aldas]

I strongly recommend rethinking if you really need timeout middleware at all. If you have upstream proxies like Nginx/Apache etc you do not need timeout mw as these services have it built in.

Are you adding because it might be useful or you have some specific requirement to fullfill? like requests can not be longer than X seconds (not this requirement can not work well with filedownloads because clients with bad connect ala mobile can take considerable more time to download files). Or requirements - even if request ends the transactions must be commited. These are some of reasons I have seen people try to use timeout mw.

[wayneforrest]

Thanks for the advice @aldas. It is a situation where I do not want tasks to take too long, and would rather return an error in those situations - I could deal with this at another level, and not make use of the middleware.

[aldas]

If you are not dealing with transactional data you could use context.WithTimeout() and make sure that you call your DB etc method by their context supporting methods. In this case when your context timeouts these methods would return and there would not need to be mess that comes with timeout middleware.

and http.Server has some ways to deal with problematic clients. See these fields

func main() {
	e := echo.New()

	s := http.Server{
		Addr:              ":8080",
		Handler:           e,
		ReadTimeout:       0,
		ReadHeaderTimeout: 0,
		WriteTimeout:      0,
		IdleTimeout:       0,
		MaxHeaderBytes:    0,
	}
	
	if err := s.ListenAndServe(); err != http.ErrServerClosed {
		log.Fatal(err)
	}
}

[aldas]

closing, when requests need to be limited I think https://github.com/labstack/echo/blob/master/middleware/context_timeout.go limiting context deadline is solution - when deadline is hit, then contextaware methods will return and handler eventually ends.