ClassLoader leak in MethodHandle.asType()

[Peter Levart]

On 04/25/2015 02:12 AM, John Rose wrote:
> On Apr 24, 2015, at 3:24 PM, Peter Levart <peter.levart at gmail.com 
> <mailto:peter.levart at gmail.com>> wrote:
>>
>> Anyway. The inexact invoke() always transforms a specific MH to a 
>> generic one (Object, Object, ...)Object, right?
>
> Yes.
>
>> So using inexact invoke() on any MH can't trigger the leak. It's just 
>> that if someone attempts to transform a generic MH to some more 
>> concrete one to be able to "connect" it with further transformations 
>> does the danger of the leak appear.
>
> Good point, thanks.
>
> This suggests that the right answer is to keep the 1-element cache as-is,
> but don't fill it with any mh1=mh0.asType(...) for which mh1.type 
> introduces
> a class loader not already present in mh0.type.
>
> I.e., don't use a weak reference for the 1-element cache, but don't 
> fill it
> with anything that might require a weak reference.  Use a backup cache
> (if needed) to handle exotic type changes.
>
>> So maybe, just for the purpose of inexact invoke(), the caching could 
>> be performed just in case when asType transformed method type is of 
>> the form (Object, Object, ...)Object. This would also prevent 
>> accidental thrashing of the cache when inexact invoke() is intermixed 
>> with other asType() invocations.
>
> The 1-element cache thrashes for other reasons already, and
> I think we need a multi-way (multi-MH) backup cache of some sort.
> The backup cache would have weak references.
>
> Having a 1-element front-side cache would cover inefficiencies
> in the backup cache, which in turn would cover inefficiencies
> for generating new conversion LFs and/or MHs.
>
> One interesting point:  If we change the 1-element cache to use
> a weak reference, the cost of reloading the cache will be higher,
> since each reload must create a new WeakReference instance.
> This is a possible reason to hold off on the suggested fix.
>
> — John

I thought so, yes. But checking for the cache-ability might not be free 
either. OTOH asTypeUncached() logic is not trivial as it is. It already 
includes checks like isConvertibleTo, etc, so another check might not be 
costly relatively. And not introducing another de-reference 
(WeakReference) makes the fast-path (the cache hit) unaffected.

Something like the following then?

http://cr.openjdk.java.net/~plevart/jdk9-dev/MethodHandle.asTypeCacheLeak/webrev.02/

Regards, Peter

>
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/mlvm-dev/attachments/20150425/4a9102f3/attachment.html>