Investigate: "Referencing subclass from superclass initializer might lead to class loading deadlock"

[sleberknight]

According to IntelliJ, referencing a subclass from a superclass initializer might "lead to class loading deadlock". This is referring to eight classes using Immutables which contain public static final fields in the abstract class which instantiate instances of the (immutable) subclass. For example, in DeleteOptions:

@Value.Immutable
public abstract class DeleteOptions implements ParamAdder {

    public static final DeleteOptions BLANK = ImmutableDeleteOptions.builder().build();
    public static final DeleteOptions RECURSE = ImmutableDeleteOptions.builder().recurse(true).build();

   // ..rest of class
}

IntelliJ gives the following warning:

Referencing subclass ImmutableDeleteOptions from superclass DeleteOptions initializer might lead to class loading deadlock

Obviously this code has been around for quite a while, since the original consul-client library started in 2014. So, it is most likely not causing any issues for anyone, or else someone probably would have found it by now.

Here is a good discussion on SO: Referencing subclass in static variable

It is slightly different than the situation here, since the subclass is private. But the question is whether to simply suppress the IntelliJ warnings, or heed them and find a (hopefully better) way to design this to avoid the situation. ImmutableDeleteOptions is code generated by Immutables, so the field declaration cannot go there (unless there is something in Immutables I don't know about, which is always a possibility).

But assuming it can't be put in ImmutableDeleteOptions, then where should these fields be created and initialized? It seems annoying to create another parallel set of classes simply to hold one (or in a few cases, two) constant fields, e.g. a DeleteOptionsConstants class containing two fields, BLANK and RECURSE. It also seems annoying to even create one Constants class with various fields that would need to be prefixed, e.g. BLANK_DELETE_OPTIONS, BLANK_EVENT_OPTIONS, and so on.

If I can "prove" that the current design will never cause "class loading deadlock" then I'd rather leave them where they are. But, how to prove that?

[sleberknight]

Sonar also flagged these as:

Classes should not access their own subclasses during initialization

The rule is java:S2390

Ref:
https://rules.sonarsource.com/java/RSPEC-2390

[sleberknight]

From the Sonar rule:

Referencing a static member of a subclass from its parent during class initialization, makes the code more fragile and prone to future bugs. The execution of the program will rely heavily on the order of initialization of classes and their static members.

and regarding the potential impact:

This could create what is known as an "initialization cycle", or even a deadlock in some extreme cases. Additionally, if the order of the static class members is changed, the behavior of the program might change. These issues can be very hard to diagnose so it is highly recommended to avoid creating this kind of dependencies.

[sleberknight]

This will be handled by the following issues across versions 1.3.3, 1.40, and 2.0.0:

1.3.3 - Deprecate constants

• Deprecate usage of the "options" constants ⚠ #356

1.4.0 - Add replacements

• Add replacements for deprecated constants #357
• Update deprecation information in deprecated "options" constants #359

2.0.0 - Remove deprecated constants

• Remove deprecated constants from "options" classes ⛔ #358