RFR (S): 8194309: JNI handle allocation failure not reported correctly

[Kim Barrett]

> On Jul 23, 2020, at 4:52 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> webrev: http://cr.openjdk.java.net/~dholmes/8194309/webrev/
> bug: https://bugs.openjdk.java.net/browse/JDK-8194309
> 
> The JNI specification states that NewLocalref and NewGloalRef return NULL on out-of-memory, whilst NewWeakGlobalRef throws OutofMemoryError. But the hotspot implementation will abort on out-of-memory in all cases.
> 
> The oop storage code for global and weak-global handles already supports taking an AllocFailStrategy, so we simply pass the RETURN_NULL strategy through - and in the weak case throw a newly defined OutOfMemoryError for "C heap space" (as a corollary for "Java heap space").
> 
> For NewLocalRef we pass the strategy through to JNIHandleBlock::allocate_block, where we explicitly use "new (std::nothrow)" to get a NULL on out-of-memory, so that we can pass it back.
> 
> There are three internal calls to NewGlobalRef in jni.cpp that needed a NULL check added to support the new behaviour.
> 
> In reality we know that if any of these things actually return NULL because C-heap is exhausted then chances are we are going to abort soon in any case. But to be spec compliant we make the changes.
> 
> Note that I deliberately do not change any of the internal JNIHandle::make_local calls (contained in the majority of JNI methods) to get NULL on out-of-memory. This is because none of those APIs are specified in a way that even considers what should happen if an internal request to create a local-ref fails - so we can neither return NULL nor throw an exception in general. All this fix addresses are the three specific JNI entry points themselves.
> 
> Also note there is no attempt with this changeset to add NULL checks to all the JNI code in the other JDK libraries that uses these API's. Interestingly quite a number already include the NULL checks.
> 
> Testing:
> 
> There is no practical way to test this for real so I had to use fault-injection. A version of the webrev with the fault-injection hooks and a test case, is presented here (for the record):
> 
> http://cr.openjdk.java.net/~dholmes/8194309/webrev.with-test-hooks/
> 
> The test is constructed so that only the JNI calls in the test should possibly encounter the NULL returns.
> 
> Otherwise only sanity testing of tiers 1-3.
> 
> Thanks,
> David

For consistency, should this:
  57 jobject JNIHandles::make_local(oop obj) {
have an optional AllocFailType argument?

Other than that, this looks good.