On Tue, 5 Nov 2024 19:21:58 GMT, Chen Liang <li...@openjdk.org> wrote:

>> After the ClassFile API migration, when serializable lambdas are requested 
>> for hidden class callers, illegal class name is generated for the 
>> serialization methods, which previously had legal yet unusable class names, 
>> as hidden classes cannot be described by a `CONSTANT_Class_info`.
>> 
>> This patch restores a similar older behavior of using legal yet unusable 
>> class names.  Previously, the invalid `.` in hidden class names was replaced 
>> by a `/` as if a package separator; this patch changes it to a `_` like that 
>> in the generated lambda's class name.
>> 
>> The bug report shows some unintended and erroneous usages of 
>> `LambdaMetafactory`.  To clarify and to persuade against misuse, I added a 
>> paragraph of API notes to `LambdaMetafactory`, describing the impact of this 
>> API being designed to support Java language features.  In particular, I used 
>> the scenario where people assumed LMf generates weak hidden classes, which 
>> happened in this issue report and in #12493, that misuse can lead to 
>> resource leaks.
>
> Chen Liang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add a test to ensure serializable lambda creation and basic execution in 
> hidden classes

src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 210:

> 208:  * referenced method; no desugaring is needed.)
> 209:  *
> 210:  * <p>Uses besides evaluation of lambda expressions and method 
> references are

I think this paragraph should go at the end (just before the `@since` tag), so 
it doesn't interrupt the explanation.

src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 212:

> 210:  * <p>Uses besides evaluation of lambda expressions and method 
> references are
> 211:  * unintended.  These linkage methods may change their unspecified 
> behaviors at
> 212:  * any time to better suit the Java language features, which may impact

Suggestion:

 * any time to better suit the Java language features they were designed to 
support, which may impact

src/java.base/share/classes/java/lang/invoke/LambdaMetafactory.java line 221:

> 219:  * where the caller is {@linkplain 
> MethodHandles.Lookup#defineHiddenClass not
> 220:  * strongly reachable} from its defining class loader, and such use 
> cases may
> 221:  * lead to resource leaks.

Not sure about having an example here, I think something more punchy is needed. 
IMO we should just specify the behavior, not motivate the design decisions 
(which are irrelevant to most users).

Maybe a simple warning such as: 'Unintended uses of these linkage methods may 
lead to resource leaks, or other unspecified negative effects.' would be enough.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21912#discussion_r1836411298
PR Review Comment: https://git.openjdk.org/jdk/pull/21912#discussion_r1836403675
PR Review Comment: https://git.openjdk.org/jdk/pull/21912#discussion_r1836396166

Reply via email to