RFR: JDK-8242451 : ensure semantics of non-capturing lambdas are preserved independent of execution mode

[Mandy Chung]

On 6/23/20 11:38 AM, Brian Goetz wrote:
>
>
> On 6/23/2020 2:08 PM, Gilles Duboscq wrote:
>> In 8232806, a system property was introduce to disable eager 
>> initialization of the classes generated by the 
>> InnerClassLambdaMetafactory 
>> (`jdk.internal.lambda.disableEagerInitialization`).
>>
>> However, when `disableEagerInitialization` is true, even for 
>> non-capturing lambdas, the capturing lambda path that uses a method 
>> handle to the constructor is taken.
>> This helps prevent eager initialization but also has the side effect 
>> of always returning a fresh instance of the lambda for every 
>> invocation instead of a singleton.
>> While this is allowed by the specs, this might change the behaviour 
>> of some (incorrect) programs that assume a singleton is used for 
>> non-capturing lambdas.
>
> I need to reiterate that such programs are broken, and coddling such 
> programs is likely to backfire in the long run.  When we switch to 
> using inline classes for lambda proxies, for example, programs 
> depending on the identity of lambda proxies likely will break, and 
> tough luck on them. 

Agree.   Such programs will break sooner or later.   Do you have any 
plan to act on fixing these programs?

> The JLS was exceedingly clear from day 1 that the identity behavior of 
> a lambda proxy is an implementation detail that should be _expected_ 
> to change.
>
> All that said, the change here seems fairly restrained, and I 
> understand there are other motivations here.  I have a few concerns, 
> which I think should be easily addressed:
>
>  - I would prefer not to use Lookup.IMPL_LOOKUP.  We have been on a 
> campaign to _reduce_ the use of IMPL_LOOKUP in code like this, both 
> based on "principle of least privilege" as well as because we would 
> like to move this code out of JLI into `java.lang.invoke` as soon as 
> practical (this has been queued up behind some other changes). 

LMF and its implementation classes do not use IMPL_LOOKUP any more.   I 
have a patch to reduce other use of IMPL_LOOKUP even further.
> Is there a translation strategy you can see that doesn't require 
> IMPL_LOOKUP?  It seems reasonable to make the field accessible, since 
> it is a cached instance that is freely dispensed to anyone who asks 
> anyway.  The hidden class generation produces a Lookup for the class 
> as part of the process; getting access to it might complicate the 
> patch a little bit, but not doing so is creating fresh technical debt 
> in the way of a refactor we have been trying to get to for a while.
>

You can use the caller Lookup.  The hidden class is a nestmate of the 
caller class and the caller Lookup can access private fields of the 
hidden class.

>  - I'm having a hard time reconciling this patch against the JDK tip, 
> which incorporates the changes for Hidden Classes.  Was this patch 
> done against an older version, or is this patch destined only for an 
> update release?
>
>  - Assuming we can resolve the above, I would like Mandy to review 
> these as she has most recently been in this code (eliminating the 
> dependence on `Unsafe::defineAnonymousClass` with the newly supported 
> "hidden classes.")

I'll post further comment.

Should this patch be a workaround to existing releases rather than the 
main line?   As Brian mentions, lambda proxy class may become inline 
class in valhalla repo (Roger has a patch already).   The earlier fixing 
those programs the better.

Mandy