gin-contrib / timeout

Timeout middleware for Gin
MIT License
185 stars 38 forks source link

fix: make set/read timeout and write/put buffer atomic #65

Closed youzhixiaomutou closed 9 months ago

youzhixiaomutou commented 11 months ago

When I ran stress testing in prod environment, I found the buffer which is used by current request will be written by the previous request which has timeout. I found that setting tw.timeout = true and put buffer to pool is guarded by sync.mutex: https://github.com/gin-contrib/timeout/blob/c9f1fc9648482a03867dae552b1ef630600bb03c/timeout.go#L82-L86 But reading timeout is unsafed, so that it may cause a data race? I think set/read timeout and write/put buffer should be atomic, otherwise it will cause writing buffer which has already been put back to pool. https://github.com/gin-contrib/timeout/blob/c9f1fc9648482a03867dae552b1ef630600bb03c/writer.go#L29-L61

codecov-commenter commented 11 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.27%. Comparing base (c9f1fc9) to head (86d7f88). Report is 17 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #65 +/- ## ======================================= Coverage 95.27% 95.27% ======================================= Files 4 4 Lines 127 127 ======================================= Hits 121 121 Misses 4 4 Partials 2 2 ``` | [Flag](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | Coverage Δ | | |---|---|---| | [go-](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `95.27% <100.00%> (ø)` | | | [go-1.18](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `95.27% <100.00%> (ø)` | | | [go-1.19](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `92.91% <100.00%> (-2.37%)` | :arrow_down: | | [go-1.20](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `90.55% <100.00%> (-4.73%)` | :arrow_down: | | [go-1.21](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `95.27% <100.00%> (+4.72%)` | :arrow_up: | | [macos-latest](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `95.27% <100.00%> (ø)` | | | [ubuntu-latest](https://app.codecov.io/gh/gin-contrib/timeout/pull/65/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib) | `95.27% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-contrib#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

appleboy commented 9 months ago

@youzhixiaomutou Thanks.