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

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~dholmes/8194309/webrev.with-test-hooks/src/hotspot/share/runtime/jniHandles.cpp.udiff.html

These functions shouldn't use UseNewCode which is false by default. 
UseNewCode is for testing experimental features locally, not for checked 
in code.  You should use logging for this.

Coleen

On 7/23/20 4:52 AM, David Holmes 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