I don't understand the "this-escape" warning under this context

[David Alayachew]

Hello Rémi and Archie,

Thank you both for your response!

> Rémi:
> The first error seems to be a false positive because as
> you said the method createBottomPanel() is private

Thank you for acknowledging this -- this is the part that threw me off and
prompted me to make this post. It appeared to me to be a false positive as
well, but I figured that I was missing info.

> Rémi:
> (BTW, all private methods can not be overriden so
> declaring a private method final is not really useful).

Fair. I did that out of desperation, since I truly did not understand what
was happening.

> Rémi:
> The second error is due to the fact that the lambda can
> be called before the end of the constructor, the compiler
> is not smart enough to know if this is the case or not.
>
> Archie:
> Note that the compiler only looks at the one file you are
> compiling. It does not try to inspect JButton to see what
> addActionListener() actually does. So for step 4 it just
> assumes the worst case, which is that addActionListener()
> immediately invokes the lamba. Of course you and I know
> this is not actually how it behaves.

I see what you both are saying. At first glance, this feels like it
cheapens the warning, but after thinking about it more, I think that
assumption is only really applicable to the small scale. Once you start
designing massive applications, this "false positive" becomes a lot more
plausible.

> Rémi:
> The usual workaround is to create the GUI inside static
> methods, the constructor being a static factory named by
> example createGUI() and createBottomPanel() being
> declared static.
>
> If you need objects, pass them as parameter of the static
> methods, so they are known to be fully initialized.
>
> Apart for the bugs with subclassing, the problem of
> publication in a concurrency context, having a
> constructor that escapes "this" also prevents a class to
> be a value class.

Thanks for the solutions and context.

Knowing that this is critical for value objects solidifies the warnings
usefulness to me. I have several projects that are throwing this warning,
but knowing what I get in return for fixing this warning makes fixing all
of those projects completely worth it.

And the solutions presented are all easy refactors, so thank you for
pointing me to them.

I do have a request -- documentation on this particular warning, as well as
the info you both brought up here, is difficult to find. Could we get some
documentation or an article by Oracle going in-depth about this warning and
how to avoid it?

My anecdotal justification for why this warning deserves this more than any
other warning that doesn't already have one is that over 40% of my projects
are now throwing this warning. This is the first time that upgrading to a
new JDK has caused this many warnings to show up in such a large percentage
of my projects. Furthermore, remedying this isn't immediately obvious, not
just because of lambdas, but because you must consider the "overridability"
of all methods that receive a reference to this -- including library
methods! That can be a difficult concept to wrap your head around with the
minimal documentation I found [1][2], so some more documentation/guidance
would be appreciated. Finally, if this is going to be useful to us in
utilizing value classes, then that is a solid reason why this should
further be documented.

I now have enough information to solve my current problem, but I'm sure
others would appreciate easy access to this information too.

Thank you for both for your time and help!
David Alayachew

[1]=javac --help-lint
[2]=
https://docs.oracle.com/en/java/javase/21/docs/specs/man/javac.html#examples-of-using--xlint-keys

On Sat, Sep 30, 2023 at 10:05 AM Archie Cobbs <archie.cobbs at gmail.com>
wrote:

> On Sat, Sep 30, 2023 at 12:55 AM David Alayachew <davidalayachew at gmail.com>
> wrote:
>
>> package Paint;
>>
>> import javax.swing.*;
>>
>> public class GUI
>> {
>>
>>    private final JFrame frame;
>>
>>    public GUI()
>>    {
>>
>>       this.frame = new JFrame();
>>
>>       this.frame.add(this.createBottomPanel());
>>
>>    }
>>
>>    private final JPanel createBottomPanel()
>>    {
>>
>>       final JButton save = new JButton();
>>
>>       save
>>          .addActionListener
>>          (
>>             actionEvent ->
>>             {
>>
>>                this.toString();
>>
>>             }
>>
>>          )
>>          ;
>>
>>       return null;
>>
>>    }
>>
>> }
>> ```
>> Can someone help me understand the what, why, and how of this warning? I
>> don't understand it at all.
>>
>
> First note there is really only one warning, even though the compiler says
> "2 warnings". It's just trying to show you the full "stack trace" where the
> leak occurs.
>
> Here's the sequence of events that the compiler is fretting about:
>
>    1. GUI constructor invokes this.createBottomPanel()
>    2. createBottomPanel() creates a lambda that, if/when it ever gets
>    invoked, could invoke this.toString()
>    3. createBottomPanel() passes this lambda to save.addActionListener()
>    4. save.addActionListener() (which the compiler does NOT inspect)
>    might possibly invoke the lambda
>    5. the lambda invokes GUI.toString()
>
> Since your class GUI is public and non-final, it could be subclassed, and
> since GUI.toString() is not final, that subclass could override toString().
>
> So in summary the compiler is deducing that it's *possible* (in theory)
> that a subclass could unwittingly operate on an incompletely initialized
> object - i.e., a 'this' escape.
>
> Note that the compiler only looks at the one file you are compiling. It
> does not try to inspect JButton to see what addActionListener() actually
> does. So for step 4 it just assumes the worst case, which is that
> addActionListener() immediately invokes the lamba. Of course you and I know
> this is not actually how it behaves.
>
> So is this a "false positive"?
>
> Yes in the sense that we "know" JButton.addActionListener() doesn't
> immediately invoke the lambda. But in general the compiler can't make
> assumptions about the behavior of other classes, even those that are part
> of the current compilation, because you could swap out class files at
> runtime and change that behavior.
>
> So all inferences are made based only on what can be deduced from the one
> file you are currently compiling. In that sense, this warning is not a
> false positive.
>
> -Archie
>
> --
> Archie L. Cobbs
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/compiler-dev/attachments/20230930/05211047/attachment-0001.htm>