-
Notifications
You must be signed in to change notification settings - Fork 11
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
Mark variables as volatile for safe concurrent access #28
Conversation
base/src/main/java/com/fasterxml/jackson/jakarta/rs/cfg/MapperConfiguratorBase.java
Show resolved
Hide resolved
Sounds reasonable; access is messy and I don't really remember original reasoning. But agreed that it is safer to mark as The only open thing is that one comment, suggesting PR was intended for 2.17 branch (or needs to updated). Either way is with me, just LMK. |
This is a follow up to #26 and with that PR brining in optimistic locking behaviour, it becomes more important to have these changes. |
@pjfanning I understand but my question was specifically if:
so I'm fine merging it but it seems to be in inconsistent state. I could solve it either way but not sure which of above is the intent. Hope this makes more sense. |
I'd prefer this to be in 2.17.1. If we choose to delay this to 2.18 then I would prefer to revert #26 and delay it to 2.18 too. |
Ok. I suppose I should then merge this & cherry-pick, least amount of work. |
Done. |
If this change is ok I can port it to jackson-jaxrs-providers.
Shared variables that have mutable values are best marked as volatile. The
_defaultAnnotationsToUse
variable is only set in the constructors so it feels best to mark it asfinal
to highlight that the value doesn't change and does not need to bevolatile
.