NonAtomicVolatileUpdate
This update of a volatile variable is non-atomic

Category
Severity
WARNING
Maturity

The problem

The volatile modifier ensures that updates to a variable are propagated predictably to other threads. A read of a volatile variable always returns the most recent write by any thread.

However, this does not mean that all updates to a volatile variable are atomic. For example, if you increment or decrement a volatile variable, you are actually doing (1) a read of the variable, (2) an increment or decrement of a local copy, and (3) a write back to the variable. Each step is atomic individually, but the whole sequence is not, and it will cause a race condition if two threads try to increment or decrement a volatile variable at the same time. The same is true for compound assignment, e.g. foo += bar.

If you intended for this update to be atomic, you should wrap all update operations on this variable in a synchronized block. If the variable is an integer, you could use an AtomicInteger instead of a volatile int.

Suppression

Suppress false positives by adding the suppression annotation @SuppressWarnings("NonAtomicVolatileUpdate") to the enclosing element.


Positive examples

NonAtomicVolatileUpdatePositiveCases.java

/*
* Copyright 2014 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


package com.google.errorprone.bugpatterns.testdata;

/** Positive test cases for {@code NonAtomicVolatileUpdate} checker. */
public class NonAtomicVolatileUpdatePositiveCases {

private static class VolatileContainer {
public volatile int volatileInt = 0;
}

private volatile int myVolatileInt = 0;
private VolatileContainer container = new VolatileContainer();

public void increment() {
// BUG: Diagnostic contains:
myVolatileInt++;
// BUG: Diagnostic contains:
++myVolatileInt;
// BUG: Diagnostic contains:
myVolatileInt += 1;
// BUG: Diagnostic contains:
myVolatileInt = myVolatileInt + 1;
// BUG: Diagnostic contains:
myVolatileInt = 1 + myVolatileInt;

// BUG: Diagnostic contains:
if (myVolatileInt++ == 0) {
System.out.println("argh");
}

// BUG: Diagnostic contains:
container.volatileInt++;
// BUG: Diagnostic contains:
++container.volatileInt;
// BUG: Diagnostic contains:
container.volatileInt += 1;
// BUG: Diagnostic contains:
container.volatileInt = container.volatileInt + 1;
// BUG: Diagnostic contains:
container.volatileInt = 1 + container.volatileInt;
}

public void decrement() {
// BUG: Diagnostic contains:
myVolatileInt--;
// BUG: Diagnostic contains:
--myVolatileInt;
// BUG: Diagnostic contains:
myVolatileInt -= 1;
// BUG: Diagnostic contains:
myVolatileInt = myVolatileInt - 1;

// BUG: Diagnostic contains:
container.volatileInt--;
// BUG: Diagnostic contains:
--container.volatileInt;
// BUG: Diagnostic contains:
container.volatileInt -= 1;
// BUG: Diagnostic contains:
container.volatileInt = container.volatileInt - 1;
}

private volatile String myVolatileString = "";

public void stringUpdate() {
// BUG: Diagnostic contains:
myVolatileString += "update";
// BUG: Diagnostic contains:
myVolatileString = myVolatileString + "update";
}
}

Negative examples

NonAtomicVolatileUpdateNegativeCases.java

/*
* Copyright 2014 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


package com.google.errorprone.bugpatterns.testdata;

/** Positive test cases for {@code NonAtomicVolatileUpdate} checker. */
public class NonAtomicVolatileUpdateNegativeCases {

private volatile int myVolatileInt = 0;
private int myInt = 0;
private volatile String myVolatileString = "";
private String myString = "";

public void incrementNonVolatile() {
myInt++;
++myInt;
myInt += 1;
myInt = myInt + 1;
myInt = 1 + myInt;

myInt = myVolatileInt + 1;
myVolatileInt = myInt + 1;

myString += "update";
myString = myString + "update";
}

public void decrementNonVolatile() {
myInt--;
--myInt;
myInt -= 1;
myInt = myInt - 1;
}

public synchronized void synchronizedIncrement() {
myVolatileInt++;
++myVolatileInt;
myVolatileInt += 1;
myVolatileInt = myVolatileInt + 1;
myVolatileInt = 1 + myVolatileInt;

myVolatileString += "update";
myVolatileString = myVolatileString + "update";
}

public void synchronizedBlock() {
synchronized (this) {
myVolatileInt++;
++myVolatileInt;
myVolatileInt += 1;
myVolatileInt = myVolatileInt + 1;
myVolatileInt = 1 + myVolatileInt;

myVolatileString += "update";
myVolatileString = myVolatileString + "update";
}
}
}