| To avoid deadlocks, class initializers should not reference subtypes of the |
| current class. |
| |
| For example: |
| |
| ```java |
| class Foo { |
| public static final Bar INSTANCE = new Bar(); |
| public static class Bar extends Foo {} |
| } |
| ``` |
| |
| There is a circular reference between the class initializers for `Foo` and |
| `Bar`: `Foo` depends on `Bar` in the initializer for a `static` field, and |
| initializing `Bar` requires initializing its supertype `Foo`. If one thread |
| starts initializing `Foo` and another thread simultaneously starts initializing |
| `Bar`, it will result in a deadlock. |
| |
| ## Suggested fixes |
| |
| The best solution is to refactor to break the cycle, by defining the constant |
| field in a separate class from the supertype of the field. |
| |
| For example, using this approach to fix the sample above would result in |
| something like: |
| |
| ```java |
| class Foos { |
| public static final Bar INSTANCE = new Bar(); |
| public static class Foo {} |
| public static class Bar extends Foo {} |
| } |
| ``` |
| |
| That refactoring may be too invasive (say the code is part of an API, and there |
| are many references to the current structure). |
| |
| If the subclass is never referenced outside the current file (i.e. `Bar` is |
| never used outside of `Foo`, it is only referenced via `Foo.INSTANCE`), making |
| `Bar` `private` makes deadlocks less likely (see caveats below in the discussion |
| about `private` classes): |
| |
| ```java |
| class Foo { |
| public static final Foo INSTANCE = new Bar(); |
| private static class Bar extends Foo {} |
| } |
| ``` |
| |
| If the subclass *is* referenced outside the current field, deadlocks can be |
| avoided by ensuring that the subclass has only private constructors (or `static` |
| factory methods), so that the only way to initialize the subclass is to first |
| initialize the containing class: |
| |
| ```java |
| class Foo { |
| public static final Foo INSTANCE = new Bar(); |
| private static class Bar extends Foo { |
| private Bar() {} |
| } |
| } |
| ``` |
| |
| If the subclass needs to be directly created by code outside the current file, a |
| static factory can be added as a member of the outer class, for example: |
| |
| ```java |
| class Foo { |
| public static final Foo INSTANCE = new Bar(); |
| |
| private static class Bar extends Foo { |
| private Bar() {} |
| } |
| |
| public static Bar createBar() { |
| return new Bar(); |
| } |
| } |
| ``` |
| |
| -------------------------------------------------------------------------------- |
| |
| ## AutoValue |
| |
| AutoValue implementation classes are necessarily non-`private`, since they are |
| generated into separate files. However examples like the following can't |
| deadlock as long as the only reference to the `AutoValue_Base` class is inside |
| `Base`, since there is no way for a thread to cause `AutoValue_Base` to be |
| initialized without first having initialized `Base`: |
| |
| ```java |
| @AutoValue |
| abstract class Base { |
| abstract String bar(); |
| |
| static final Object DEFAULT = new AutoValue_Base("bar"); |
| |
| static Base of(String bar) { |
| return new AutoValue_Base(bar); |
| } |
| } |
| ``` |
| |
| There is a separate Error Prone check, https://errorprone.info/bugpattern/AutoValueSubclassLeaked, to |
| prevent `AutoValue_` classes from being accessed outside the file containing the |
| corresponding `@AutoValue` base class. |
| |
| ## Private member classes are *often* safe |
| |
| The check ignores references that cross from a `private` inner class (or any |
| class inside it) to its immediately enclosing class, since these are more often |
| safe. For example, the following is safe in normal usage: |
| |
| ```java |
| public class A { |
| private static Object benignCycle = new B.C(); |
| private static class B { |
| public static class C extends A { } |
| } |
| } |
| ``` |
| |
| There is a cycle `A` -> `A.B.C` -> `A`, but the public APIs do not make it |
| possible to access `A.B.C` in a way that causes initialization until after A is |
| initialized. |
| |
| That cycle can still cause problems if users access the private class by using |
| reflection. Fortunately, in practice, most reflective initialization of classes |
| seem to be performed in a warmup phase during which multithreaded usage is less |
| likely. As a result, the danger from reflection seems to come more from cases in |
| which |
| [one class needs to be initialized before another even in a single-threaded environment](https://github.com/google/guava/pull/8634). |
| (And some of the deadlocks detected by ClassInitializationDeadlock will |
| additionally be cases in which one class needs to be initialized before |
| another.) |
| |
| Reflection aside, cycles involving `private` classes sometimes still *can* allow |
| users to trigger deadlocks through the public API. So, while |
| ClassInitializationDeadlock ignores `private` classes as a heuristic that is |
| good enough for most real-world examples of deadlocks that have been observed, |
| `private` alone does not actually guarantee safety. |
| |
| Here is an example of a cycle involving a `private` class that can trigger a |
| deadlock. In it, `A.C` can trigger initialization of `B`, despite `B` being |
| private. |
| |
| ```java |
| public class A { |
| private static Object bad_cycle = new B(); |
| private static class B extends A { } |
| public static class C extends B { } |
| } |
| ``` |