RFR: 8015831: Add lint check for calling overridable methods from a constructor

[Alan Bateman]

On Fri, 6 Jan 2023 23:08:27 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

> I've moved the `@SuppressWarnings` annotations onto a new branch `ThisEscapeAnnotations`. This is just for future reference in case somebody wants to add them back someday and doesn't want to start from scratch.
> 
> > I suspect some of the suggestions to document leaking this in implNotes will need discussion too because they amount to documenting "hooks" that people will rely on, e.g. documenting in ArrayDeque that its constructor invokes addList could be read as an invite to override it.
> 
> Hmm. Hasn't that horse already left the barn? You kind of implied that when you said:
> 
> > I see a few examples in your list where there have been attempts to avoid leaking "this" but where we backed away out of concern that 3rd party code was extending some class and overriding a method known to be invoked by the constructor.
> 
> In other words, it doesn't sound like changing the behavior of these constructors is viable option at this point. And if that's the case, we might as well document and warn about the current behavior.
> 
> Of course I'd love to be wrong... in which case, we can fix these constructors.

I hope at least some of these can be fixed as leaking a part initialized object to an untrusted subclass or some other code is problematic in many ways. Skimming through the original list, then some of these may be accidental and some may be fixable without any compatibility concerns, esp. in the JDK internal classes. Yes, some horses have left the barn. A horse that got out a long time ago is j.net.ServerSocket where 3 of its constructors call an overridable bind method. Recent re-implementation would have fixed this but there were concerns that subclasses may be relying on this long standing undocumented behavior. There are other cases in the list where the leaking may be necessary part of the contract for subclasses. So it will probably require working through them on a case by case basis. I don't think the implementation notes should be included as part of the adding the lint warning as I think it creates an attractive nuisance. Developers reading these implementation notes may c
 reate code that depends on these "hooks" and it will make it very hard for the JDK to ever change the behavior. Where the leaking is a necessary part of the contract for subclassing then it may be that it has to be documented as normative text rather than an implementation note.

-------------

PR: https://git.openjdk.org/jdk/pull/11874