Skip to content
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

Use AtomicInteger instead of unsafe increment on volatile fields #22278

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

stsypanov
Copy link
Contributor

I've found several cases where we increment numeric field declared volatile. Such explicit increment may bring incorrect behaviour, as increment operation itself is not atomic.

@jhoeller jhoeller self-assigned this Jan 21, 2019
@jhoeller jhoeller added this to the 5.1.5 milestone Jan 21, 2019
@jhoeller jhoeller added the type: enhancement A general enhancement label Jan 24, 2019
@jhoeller jhoeller changed the base branch from master to 5.1.x January 24, 2019 14:27
@jhoeller jhoeller changed the base branch from 5.1.x to master January 24, 2019 14:28
@jhoeller jhoeller added type: bug A general bug and removed type: enhancement A general enhancement labels Feb 3, 2019
@jhoeller
Copy link
Contributor

jhoeller commented Feb 4, 2019

Reviewing the three affected places, it turns out that all of those are effectively evaluation counts with no need for absolute correctness attached. As far as I can tell, all of those checks would still work correctly even if an occasional concurrent count update has been lost.

That said, this is nevertheless worth addressing, so I'll merge this into master. There is just no strong need for a backport, so I'll rather keep the existing minimal locking in place and leave the potential runtime side effects of stronger locking up to 5.2 only.

@jhoeller jhoeller modified the milestones: 5.1.5, 5.2 M1 Feb 4, 2019
@jhoeller jhoeller added type: enhancement A general enhancement and removed type: bug A general bug labels Feb 4, 2019
@jhoeller jhoeller merged commit 248d3f8 into spring-projects:master Feb 4, 2019
@stsypanov stsypanov deleted the unsafe-volatile-ops branch February 19, 2019 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants