-
Notifications
You must be signed in to change notification settings - Fork 629
Description
AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE
is falsely reported for an absolutely valid (nonsense) code (extracted from a bigger JUnit test), referring to https://wiki.sei.cmu.edu/confluence/display/java/VNA02-J.+Ensure+that+compound+operations+on+shared+variables+are+atomic :
public class FalsePositiveTest {
private volatile boolean hasCalled = false;
public void test1() {
assertEquals(false, hasCalled);
// do something that changes the flag
hasCalled = false; // Operation on the "hasCalled" shared variable in "FalsePositiveTest" class is not atomic
}
public void test2() {
assertEquals(false, hasCalled);
// do something that changes the flag
hasCalled = false; // Operation on the "hasCalled" shared variable in "FalsePositiveTest" class is not atomic
}
public void snapshot() {
FalsePositiveTest p = new FalsePositiveTest();
p.hasCalled = hasCalled; // Operation on the "hasCalled" shared variable in "FalsePositiveTest" class is not atomic
System.out.println(p);
}
public static void assertEquals(Object expected, Object actual) {
}
}
The warning seem to assume the operations on the variable inside each method may somehow see wrong value?
But this assumption here is not correct, the opposite is the case, the test expects the shared variable to change!
Even if it would be not a test, the assumption that the shared field value should not change if used multiple times in the same method is wrong. It could be a problem but it also could be perfectly expected state like here.
Neither test1()
nor test2()
need to see same hasCalled
value at any time inside the method (the opposite is the case), and the read of the field is guaranteed to be atomic. Sadly enough, removing the volatile
would mute the spotbugs warning but break the test that actually modifies the field in parallel which is why the field is marked as volatile!
I'm not sure if this is matching to https://wiki.sei.cmu.edu/confluence/display/java/VNA02-J.+Ensure+that+compound+operations+on+shared+variables+are+atomic explanation either, as there is only about compound operations on a shared variable like ++
or flag=!flag
, not about using shared variable multiple times in the method body.