Rule S3077 frequently generates a false positive

It has been my experience that S3077 always generates false positives for the projects on which I work. The intent of the rule appears to be to indicate that using “volatile MyObject myRef” does not guarantee that different threads will see updates to the object referenced by “myRef”, which is correct. However, in my experience, use of “volatile” has only ever been used to guarantee that different threads will see updates to myRef, itself, not to the object it references. (On the other hand, I have found that code that uses “volatile” for primitive, mutable values other than boolean is almost always incorrect.)

The workaround, to use AtomicReferences, obfuscates the code.

I presume your question concerns analyzing Java code, is that correct? Please confirm!

(Tip: it’s always good to include relevant language tags in the post)

Ah, sorry. Yes, it’s for java.

Any thoughts on this?

Hi @jrh3,

Could you provide some small code examples of good findings and false-positive about S3077?

My view of this rule is: Usage of volatile is related to a multi-threads context. If a mutable object is shared, “volatile” will not help to prevent concurrent modifications, so it’s good to inform the developer.

Unfortunately, the rule is not able to distinguish if the referenced object is mutable (at risk) or not. And with a mutable object, using AtomicReference does not prevent underlying concurrent modifications.

One possible change for this rule would be to focus only on arrays. Because java arrays are always mutable, and java.util.concurrent.atomic.Atomic*Array provides good helpers to manipulate arrays of immutable objects safely.

Hi Alban,

Just returned from vacation. Suppose I want to set a variable and have other threads retrieve the variable as soon as it has been set. That’s when I would typically use “volatile” to guarantee that other threads reading the variable see the latest value:

private volatile SomeObject object;
makeNewObject(String arg) { object = new SomeObject(arg); }
clearObject() { object = null; }
getObject() { return object; }

As multiple threads may invoke makeNewObject() or clearObject(), “object” has been declared “volatile” so that getObject() always returns the last object that was set. Note: the validity of this code has nothing to do with whether or not SomeObject is mutable; that is an independent issue. When dealing with non-primitive objects, I have never seen a case where a developer declared “object” to be volatile as an (erroneous) means to ensure that updates to the referenced object were visible in other threads; I have only ever seen it used as above - a completely legitimate use.

When sonar reports these, we are forced to either change to AtomicReference or disable sonar for this declaration. I prefer developers to not get in the habit of disabling sonar, but this constant unnecessary warning leads us to do just that.

-Jim