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

[Aleksey Shipilev]

All right, I will fix the issue that *actually* bites me in the bottom
all the time: MH.publicLookup() init circularity. Here's a webrev
without an inline blob:
  http://cr.openjdk.java.net/~shade/8158851/webrev.03/

If you want to improve startup time along the way, file a separate issue.

(I still don't get where the *profit* is? There are 10 small lines of
inline bytecode, together with the generator used to get it. This is a
fallback code anyway. Standalone .class is the same byte blob, but
resting somewhere else. It is as opaque as the inline byte blob. Trying
to hook class generation to the build sequence takes another 100+ lines
of assorted source code. I don't get why should one bother, if there is
little profit for a lot of hassle)

Thanks,
-Aleksey

On 06/07/2016 07:01 PM, Jim Laskey (Oracle) wrote:
> If it’s in the jimage, I/O should not be an issue (defineClass from a direct memory mapped buffer) vs assembling and assigning a byte array.  Measure first?
> 
> 
>> On Jun 7, 2016, at 12:44 PM, Aleksey Shipilev <aleksey.shipilev at oracle.com> wrote:
>>
>> On 06/07/2016 06:09 PM, Jim Laskey (Oracle) wrote:
>>> 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?
>>
>> What's the difference between a small binary blob in a
>> filesystem/resource vs a small binary blob inlined? Apart from the
>> additional hassle of packing things into resources (which probably
>> requires dealing with build?), hitting the I/O for loading it (which
>> probably defeats the purpose of constructing the class on the fly), etc.
>>
>> I wonder if the whole LookupHelper business is here to stay: Alan said
>> that part of mechanics is up to a reimplementation?
>>
>> Thanks,
>> -Aleksey
>>
>>>> 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
>>>>
>>>>
>>>
>>
>>
>