-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(context): make set and get header thread safe #4101
base: master
Are you sure you want to change the base?
fix(context): make set and get header thread safe #4101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
b5e8b10
to
4857f7c
Compare
I do not recommend the gin framework to add concurrency safety for headers. It should be up to the user to decide whether to add locks. The main reasons are:
|
4ef3dd9
to
da66140
Compare
@flc1125 Since concurrent read writes to the headers map will lead to application crash, I believe this protection should be present in the library itself.
Yes, this could be an issue so have created a separate mutex for header map A similar change has been made to get and set context: #700 |
da66140
to
cd86308
Compare
Thank you very much for accepting the feedback I provided. However, I still insist that gin does not need to address the header competition issue. For this, I sincerely apologize. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4101 +/- ##
==========================================
- Coverage 99.21% 98.99% -0.22%
==========================================
Files 42 44 +2
Lines 3182 3397 +215
==========================================
+ Hits 3157 3363 +206
- Misses 17 24 +7
- Partials 8 10 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
add reader lock
fix lint
cd86308
to
46b1c57
Compare
fixes #4100