Compile time checks for class initialization deadlocks

[carterkozak]

Description of the problem / feature request:

Class initialization deadlocks are common when a type uses one of its subtypes in a static field or block.

Feature requests: what underlying problem are you trying to solve with this feature?

I would like to catch these deadlocks at compile time rather than intermittently at runtime!
I have implemented such a check here, but I believe it would be a good candidate for error-prone upstream:

palantir/gradle-baseline#1598

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Example from guava:
google/guava#1977

class Base {
  static final Object FIELD = new Sub();
  static class Sub implements Base {
  }
}

What version of Error Prone are you using?

2.4.0

Have you found anything relevant by searching the web?

no

[carterkozak]

@cushon, I'm interested in your thoughts on this when you have a moment.

[magibney]

+1; this feature would have helped with apache/solr#819 (https://issues.apache.org/jira/browse/SOLR-16165).

It's worth mentioning @carterkozak's subsequent blog post on the topic.

[cushon]

I believe it would be a good candidate for error-prone upstream

@carterkozak sorry for the very slow response here, are you still interested in contributing this?

[carterkozak]

No worries, @cushon. I'd be happy to :-)

[cushon]

Great, thanks!

[hagbard]

I assume that for AutoValue, and assuming that the generated subclass only gets referenced after static initialization of the parent class has started (i.e. all references to "new AutoValue_Xxx(...)" occur inside the parent class), that this is not an issue?

[cushon]

@hagbard I think it would be theoretically possible to create deadlocks that involved AutoValue, but not with idiomatic usages of it, normally the hand-written class doesn't reference the generated subclass in static field initializers.

I guess we might have to be careful about stuff like

class Base {
  static final Object DEFAULT = new AutoValue_Base();
}

@carterkozak I'd still be happy to review a PR for this if you have time and interest :)

[eamonnmcmanus]

We already have AutoValueSubclassLeaked to guard against the most likely source of initialization deadlocks involving AutoValue. I don't think there's a concern with this sort of thing:

@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);
  }
}

If those are the only references to AutoValue_Base then there's no path by which a thread could start initializing AutoValue_Base before Base. If there were an Error Prone like the proposed one, it would trigger here. Users could work around that by writing of("bar") instead of the constructor call, though that in itself would not change the possibility or impossibility of deadlocking.

The text below gives more detail, but isn't all that relevant to the issue here, so take it as a footnote.

---

We've encountered a couple of cases at Google where people have used AutoValue with builders in a way that could lead to deadlock. I'm copying some excerpts from our internal issue-tracking system:

@AutoValue
public abstract class Foo {
  public static final Foo DEFAULT = Builder.builder().build();

  @AutoValue.Builder
  public abstract static class Builder {
    public static Builder builder() {
      return new AutoValue_Foo.Builder();
    }
    public abstract Foo build();
  }
}

Suppose none of these classes has been initialized, and two threads come along at the same time. Thread A references Foo.DEFAULT and Thread B calls Foo.Builder.builder().build() directly. Thread A will then try to initialize classes in this order: Foo > Foo.Builder > AutoValue_Foo.Builder > AutoValue_Foo. Thread B will try to initialize in this order: Foo.Builder > AutoValue_Foo.Builder > AutoValue_Foo > Foo. Because A's order has Foo before Foo.Builder and B's order has it after, the threads may deadlock.

If the static builder() method were in Foo we would have this:

@AutoValue
public abstract class Foo {
  public static final Foo DEFAULT = builder().build();
  public static Builder builder() {
    return new AutoValue_Foo.Builder();
  }

  @AutoValue.Builder
  public abstract static class Builder {
    public abstract Foo build();
  }
}

Now A still references Foo.DEFAULT but B calls Foo.builder().build(). A's order is: Foo > AutoValue_Foo.Builder > Foo.Builder > AutoValue_Foo. B's order is the same. So no deadlock.

AutoValue attempts to detect and warn about this particular pattern but it's still possible to run into it if your code doesn't match exactly what AutoValue is checking for or if you have a hand-written builder.

[cushon]

No worries, @cushon. I'd be happy to :-)

@carterkozak any chance this is still on your radar? :)

---

A couple more examples of class initialization deadlocks:

• Class initialization deadlock with TypeDescription.ForLoadedType and TypeDescription.Generic raphw/byte-buddy#1347
• Fix CommandLine class initialization deadlock. bazelbuild/bazel#21608

[cushon]

An initial implementation has been added in 1d99484.

Feedback or contributions are very welcome, e.g. if there are ideas about the heuristics or how to make it more precise.