Summary
-------
The addition of a new `this-escape` lint warning to the compiler.
Problem
-------
The Java language requires subclass constructors to invoke a superclass constructor before performing any other work, but superclass constructors are allowed to invoke instance methods as part of their initialization. Therefore, if a superclass constructor invokes an instance method that a subclass overrides, then that method will be invoked before the subclass has done any of its own initialization, possibly leading to unexpected behavior.
The following class demonstrates the problem:
import java.util.*;
import java.util.function.*;
/**
* A {@link Set} that only allows elements matching some {@link Predicate}.
*/
public class FilteredSet<E> extends HashSet<E> {
private final Predicate<? super E> filter;
public FilteredSet(Predicate<? super E> filter, Collection<? extends E> elems) {
super(elems);
this.filter = filter;
}
@Override
public boolean add(E elem) {
if (!this.filter.test(elem))
throw new IllegalArgumentException("disallowed element");
return super.add(elem);
}
public static void main(String[] args) {
new FilteredSet<>(s -> true, Arrays.asList("abc", "def")); // NullPointerException
}
}
The above example appears bug-free at first glance, but it actually throws a `NullPointerException` if run. Moreover, how this error happens is due to a tricky interplay of three different classes:
1. The `FilteredSet` constructor invokes the `HashSet(Collection)` superclass constructor;
1. `HashSet(Collection)` constructor invokes `AbstractCollection.addAll()`;
1. `AbstractCollection.addAll()` invokes `add()` for each element;
1. `FilteredSet.add()` invokes `this.filter.test()`, but `this.filter` is still null.
This bug is possible because the `HashSet(Collection)` constructor has a 'this' escape. This term refers to any time a reference to this "escapes" from a superclass constructor's control to some other code before it has been fully initialized.
The example above illustrates a key difficulty with 'this' escape bugs, which is that they typically involve an interplay among multiple source files. Inspecting any one file at a time does not reveal the bug.
Another example of a subtle 'this' escape bug would be a superclass constructor that adds `this` to some `HashSet`, perhaps when registering itself as a listener. If a subclass overrides `hashCode()` to include one or more of its own fields, then the `HashSet` will record the object under the wrong hash value, because at that time those subclass fields won't be initialized yet.
Solution
--------
Update the Java compiler so that it can generate warnings about possible 'this' escapes.
As with various other similar warnings, to enable 'this' escape detection, one would include the `-Xlint:this-escape` flag (or `Xlint:all`) on the command line, and to suppress it locally, one would use `@SuppressWarnings("this-escape")`.
Perfect analysis is not possible, so to the extent the analysis must be imperfect, it is preferable for it to err on the side of generating false positives rather than false negatives. This way, if your code does not generate any warnings, then you can feel confident that there is a high probability that there are no leaks.
This is analogous to how you can be confident that you won't get `ClassCastExceptions` at runtime if your code doesn't generate any `unchecked` warnings at compile time.
Specification
-------------
The 'this' escape warning only looks for "escapes" in which some _subclass_ might still be uninitialized. It doesn't look for escapes in which the class doing the leaking might itself be uninitialized. In other words, the warning assumes any individual constructor knows what it's doing. Instead, it only looks for potential problems that necessarily span multiple classes.
In fact, the boundary is actually larger than that: potential problems must span multiple *modules* (or packages if not using modules). Warnings are only reported for constructors that could be invoked from some subclass defined _in a different module_. So, for example, `private` constructors and constructors in non-`public` classes don't generate warnings. This design choice reflects the "key difficulty" of 'this' escape bugs mentioned above.
Note: Choosing the module boundary here instead of, for example, the source file boundary, is a conservative choice to keep the warning from being too "aggressive" initially. The thought here is that any new lint option runs the risk of turning off developers if it generates too many warnings in their existing code (even if the warnings are valid), making them want to just disable it completely. As we gain experience over time, we can tighten the screws so to speak, perhaps in conjunction with increasing sophistication in the analysis.
The analysis covers constructors, fields with initializers, and non-static initialization blocks.
As with other similar warnings, `@SuppressWarnings("this-escape")` can be used to suppress this warning as needed. This suppression will work at the class, constructor, or field level (for fields with initializers).
Unfortunately, to cover leaks from initialization blocks, there's nowhere to put a `@SuppressWarnings("this-escape")` except on the containing class.
Changes to the `module-info.java` Javadoc for module `jdk.compiler`:
diff --git a/src/jdk.compiler/share/classes/module-info.java b/src/jdk.compiler/share/classes/module-info.java
index 7974f8cd6a3..142a185a050 100644
--- a/src/jdk.compiler/share/classes/module-info.java
+++ b/src/jdk.compiler/share/classes/module-info.java
@@ -183,6 +183,7 @@ import javax.tools.StandardLocation;
* <tr><th scope="row">{@code strictfp} <td>unnecessary use of the {@code strictfp} modifier
* <tr><th scope="row">{@code synchronization} <td>synchronization attempts on instances of value-based classes
* <tr><th scope="row">{@code text-blocks} <td>inconsistent white space characters in text block indentation
+ * <tr><th scope="row">{@code this-escape} <td>superclass constructor leaking {@code this} before subclass initialized
* <tr><th scope="row">{@code try} <td>issues relating to use of {@code try} blocks
* (that is, try-with-resources)
* <tr><th scope="row">{@code unchecked} <td>unchecked operations
Changes to the `javac(1)` manual page in module `jdk.compiler`.
* Add to section "Extra Options" under `Xlint`:
· this‐escape: Warns about constructors leaking this prior to
subclass initialization.
* Add to section "EXAMPLES OF USING ‐XLINT KEYS":
this−escape
Warns about constructors leaking this prior to subclass initial‐
ization. For example, this class:
public class MyClass {
public MyClass() {
System.out.println(this.hashCode());
}
}
generates the following warning:
MyClass.java:3: warning: [this‐escape] possible ’this’ escape
before subclass is fully initialized
System.out.println(this.hashCode());
^
A ’this’ escape warning is generated when a constructor does
something that might result in a subclass method being invoked
before the constructor returns. In such cases the subclass
method would be operating on an incompletely initialized in‐
stance. In the above example, a subclass of MyClass that over‐
rides hashCode() to incorporate its own fields would likely pro‐
duce an incorrect result when invoked as shown.
Warnings are only generated if a subclass could exist that is
outside of the current module (or package, if no module) being
compiled. So, for example, constructors in final and non‐public
classes do not generate warnings.