RFR (S) 8158851: MH.publicLookup() init circularity, triggered by custom SecurityManager with String concat and -limitmods java.base

[Jim Laskey (Oracle)]

Generally I disagree with this kind of hack.  It will bite you in the @$$ at some point.  Why not just have a class file containing the bytes?

Otherwise, you should probably embed some of the ASM constants in the array to handle things like class version? 52 => CLASS_VERSION, et cetera. These would be compile time symbols and no less efficient and slightly more readable.

— Jim



> On Jun 7, 2016, at 11:56 AM, Aleksey Shipilev <aleksey.shipilev at oracle.com> wrote:
> 
> On 06/07/2016 04:51 PM, Alan Bateman wrote:
>> On 07/06/2016 14:38, Aleksey Shipilev wrote:
>>> Please review a fix for a MH.publicLookup() circularity, which is
>>> triggered if you run existing String concat tests with -limitmods
>>> java.base:
>>>   https://bugs.openjdk.java.net/browse/JDK-8158851
>>> 
>>> Webrev:
>>>   http://cr.openjdk.java.net/~shade/8158851/webrev.01/
>> As a short term fix then this looks okay. It may be (once there is
>> another round of work on MethodHandles w/modules) that PL moves back to
>> java.lang.Object but that has many discussion points for another thread.
>> 
>> If you want then the doPriv in LookupHelper can do away as it is not
>> required when you can guarantee that it is initialized before a security
>> manager is set.
> 
> Done so.
> 
>> One downside to initializing PL early is that is LookupHelper will
>> trigger some ASM to be loaded. This could avoid by generating it at link
>> time and loading the class bytes as resource. Hard to know if it's worth
>> it as the first lambda usage is going to trigger these classes to load
>> anyway.
> 
> Yes. I agree with Claes here, can't we inline the bytecode to avoid
> messing with ASM?
> 
> See:
>  http://cr.openjdk.java.net/~shade/8158851/webrev.02/
> 
> Still passes all java/lang tests.
> 
> Thanks,
> -Aleksey
> 
>