Skip to content

False positive AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE #3363

@iloveeclipse

Description

@iloveeclipse

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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions