In a
previous post
I discussed a new check that Mutability Detector should make to provide
a more accurate result. The implementation of that checker is now
complete, and will be included in an upcoming 0.7 release.
As with any other checks the tool does, the analysis is
unsound,
that is, it doesn't guarantee correctness. It makes as best a guess as
it can, and I hope that empirical study of its use would show that it's
good enough to rely on, but it is possible that the tool can be fooled
into emitting false positives or false negatives. I thought it would be
useful to talk about the checker does and doesn't do.
The checker analyses
every constructor of your class, and checks for the following problems.
Assigning 'this' to a field
The tool will render your class mutable if it does any of the following:
- assigns 'this' to static field
- SomeOtherClass.staticField = this;
- assigns 'this' to instance of other object
- someOtherObject.instanceField = this;
- assigns 'this' to field of this instance
- assigns 'this' to static field of this class
- ThisClass.staticField = this;
Some of these will be a false positive, e.g. with #2, if you construct an object in the constructor, and assign '
this' to one of its fields, before it's published, it'll be safe. Also assigning '
this'
to an instance field is safe as well (or at least, I can't think why it
is unsafe), but it's weird enough that I don't want to lose too much
sleep trying to be correct about it.
Passing 'this' as a parameter
The tool will render your class mutable if it does any of the following:
- passes 'this' as parameter to another method
- new SomeOtherClass(this);
- SomeOtherClass.staticMethod(this);
- someOtherObject.instanceMethod(this);
- constructs an inner class (which has an implicit reference to 'this')
A false positive in this case could be, with #1, passing '
this'
to somewhere that only saves the reference, without trying to read any
values from it, or with #2, the instance of the inner class might not
escape, meaning the '
this' reference hasn't escaped.
Unexpected false positives
The
last examples of false positives are ones that can occur with a correct
implementation. However, there's one false positive that I didn't
expect to create. While the tool is trying to check for 'this' being passed to other methods, it also accidentally raises an issue when instance fields are passed as well.
Here's an example from
java.util.Vector:
public Vector(Collection c) {
elementData = c.toArray();
elementCount = elementData.length;
if (elementData.getClass() != Object[].class)
elementData = Arrays.copyOf(elementData, elementCount, Object[].class);
}
}
The issue being the call to
Arrays.copyOf() with two instance fields. This piece of code will cause
Vector
to be reported as mutable (among many other reasons), and quite nicely
demonstrates the issues involved. Here, the method is single threaded,
but it hypothetically could change at any point to publish the
references it receives to multiple threads. Without further (expensive)
analysis into what
Arrays.copyOf() does, I can't conclude whether
Vector is immutable or not. I'm choosing to err on the side of caution. There is the scope for a potential
whitelist of safe methods, but that's not on the cards just yet.
Another interesting point is that one field is a primitive
int, the other is an array. Since the value of the
int
will be copied to the method, it's probably safe anyway, no matter
whether threads are involved or not. However, for the array, the
reference to the field will be passed, not the contents. If the array is
not completely populated and made visible when it's passed, the
receiving method could observe it to change, breaking immutability. This
leads on to the wider question of any mutable fields escaping at any
point in the object's life cycle, not just in the constructor. For now,
I'm embracing the happy accident that the checker will warn you of some
potentially dodgy code, and will deliberately leave that particular
false positive active.
I don't expect any of these false positives to really
cause an issue If you disagree, get in touch, email me, leave a comment,
or
raise a bug (that may require sign in).
I expect to release a new version within the next week, stay tuned for more news!