I would like to suggest a modification to rule squid:S3077 (Non-primitive fields should not be “volatile”)
The rationale for the rule is that the
volatile keyword is not sufficient to guarantee that changes to fields of object referents are seen by other threads. However, if the referent is immutable, there may be no changes to fields.
Consequently, a field that holds a volatile reference to a non-primitive immutable type should not trigger the rule.
private volatile MyImmutableType foo; // Compliant -- reference may change,
// but object referred to may not.
Enum constants should be considered immutable for this purpose. (Although it is technically possible to implement mutable enum constants, I would argue that using mutable enum constants is a design flaw of its own.)
private volatile MyEnumType status; // Compliant -- enum constant
// should be immutable.
See also CERT, CON50-J.:
“[…] when the referent is immutable, declaring the reference volatile suffices to guarantee safe publication of the members of the referent. […] Use of the
volatile keyword can only guarantee safe publication of primitive fields, object references, or fields of immutable object referents.”
Hi @bduderstadt !
You are perfectly right in saying that this rule should not raise issue on immutable types.
And this is exactly why the implementation of the rule has a whitelist of well known immutable types.
The difficulty in your suggestion is to determine if a type is immutable or not, and this is a (very) hard question to solve and we did not take the time to try to answer it (nor do we plan to).
Note that this immutability problem is a question that pops up regularly in a few rules and solving it could help improve a few more rules than just S3077.
Hope that helps.
good to know that the implementation already considers well known immutable types.
What do you think about my suggestion about enums? I’d argue that enum constants are (or at least should be) immutable.
How about adding some clarifying note to the rule description? Perhaps that would help users in deciding how to deal with this issue.
For example “Note: There might be False Positives on immutable enum constants or other immutable types. [Since immutability of a type cannot be decided easily in most cases]”
I just found that there is anyway a change request for the message, so perhaps this can be combined with a note on False Positives.