RFR: 8291736: find_method_handle_intrinsic leaks Method* [v3]

[Coleen Phillimore]

On Wed, 31 Aug 2022 05:04:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Sorry, I didn't read what you actually wrote (just what I thought you wrote). Line 2080 can also throw an exception for metaspace allocation failure.  I don't believe that we always do this outside a lock.  This is a subtle interaction that only you noticed.  I'm not sure what to do with the metaspace allocation failure.  Moving throwing the other exception outside the lock uglifies the logic but isn't hard to do.
>
>> This is a subtle interaction that only you noticed.
> 
> You mean in this review?  The original code clearly knew this had to be done outside the lock:
> 
> http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/e5b0439ef4ae#l33.106
> 
> Now perhaps the current code is actually safe (as @iklam suggests) because it can't actually invoke any Java code? But that is something we would need to establish. In any case it seems an anti-pattern to allow any kind of use of CHECK from inside a locked region when we "know" we always have to throw exceptions outside of locked regions.

The problem with the original code was the ranking and multi-purpose use of the SystemDictionary_lock.  Creating the adapters takes out a lock whose ranking is above SystemDictionary_lock.  Also the tables were shared with code that could call Java code for calling Java to create MethodType, so that code had to release the lock.  Leaking the java.lang.invoke.MethodType is ok because it'll be GC'd.

We've leaked the Method* and adapter for many years now, maybe it's fine to keep leaking it and I should just close this and not try to fix it.

The case of throwing exceptions inside Mutex locked regions is something I thought we have all over the source code.  I couldn't find any explicit calls but the pattern of doing metaspace allocation inside of a MutexLocker is in enough places that adding an   assert(!thread->owns_locks(), "must release all locks when possibly throwing exceptions"); during in the TRAPS version of Metaspace::allocate() fails immediately.

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

PR: https://git.openjdk.org/jdk/pull/9983