JDK-8258185 : [JNI] Clarify the specification in relation to portable use of APIs that involve the Primitive Array Release Modes
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 17
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-12-14
  • Updated: 2025-01-29
  • Resolved: 2021-06-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17 JDK 18
17 b29Fixed 18Fixed
Related Reports
CSR :  
JDK-8269277 - [JNI] Clarify the specification in relation to portable use of APIs that involve the Primitive Array Release Modes
Relates :  
JDK-8300024 - Replace use of JNI_COMMIT mode with mode 0
Relates :  
JDK-8258077 - Using -Xcheck:jni can lead to a double-free after JDK-8193234
Relates :  
JDK-8258186 - Replace use of JNI_COMMIT mode with mode 0
Relates :  
JDK-8300117 - Replace use of JNI_COMMIT mode with mode 0
Description
The ReleasePrimitiveArrayCritical and Release<X>ArrayElements APIs both use a "release mode" to determine how to update the target array and how to free it if it were a copy:

---
Primitive Array Release Modes:
mode 	actions
0 	copy back the content and free the elems buffer
JNI_COMMIT 	copy back the content but do not free the elems buffer
JNI_ABORT 	free the buffer without copying back the possible changes

In most cases, programmers pass "0" to the mode argument to ensure consistent behavior for both pinned and copied arrays. The other options give the programmer more control over memory management and should be used with extreme care.
---

If only mode 0 is used then there are no issues. However if mode JNI_COMMIT is used with an API that introduces a copy of the array, then that copy will "leak" if a second call to the appropriate "release" function is not made with a mode that causes the copy to be freed.

This requirement is not made clear in the specification, and is actually somewhat contradictory in relation to the other aspects of the specification:

- the term "Release" in the API names suggests a single-use method after which the target array has been "released" and must not be referenced again, but we may need to call it again to free the copy

- the specification for Get<X>ArrayElements states "The result is valid until the corresponding Release<X>ArrayElements() function is called." which again suggests only a single call should be made and that the array must not be referenced/used again.

However it is not hard to conceive an intended usage where the array is updated in pieces and the result written back using ReleaseX with mode JNI_COMMIT, before finally (potentially) freeing the buffer with ReleaseX and mode 0/JNI_ABORT. Such usage seems disallowed. Although the spec states "The other options [modes] give the programmer more control over memory management and should be used with extreme care." it sheds no light on the expected or allowed usage of the returned array when JNI_COMMIT is used.
Comments
Suggested update: In most cases, programmers pass "0" to the mode argument to ensure consistent behavior for both pinned and copied arrays. The other options give the programmer more control over memory management and should be used with extreme care. <new text> If `JNI_COMMIT` is passed as the `mode` argument when `elems is a copy of the elements in `array`, then a final call to *Release\<PrimitiveType\>ArrayElements* passing a `mode` argument of "0" or `JNI_ABORT`, should be made to free the `elems` buffer.</new text>
24-06-2021
The more I look into this the more I think JNI_COMMIT was introduced to address some obscure usecase that nothing seems to reference any longer. The naming of the modes causes nothing but confusion as noted by JDK-4636182, and as demonstrated in the "JNI CookBook" [1] which incorrectly uses JNI_COMMIT and JNI_ABORT as duals, rather than 0 and JNI_ABORT, and also this book [2] just uses JNI_COMMIT. Various other examples on the web also incorrectly use JNI_COMMIT [2]. At least IBM ignore JNI_COMMIT in their documentation (i.e. they do not describe any circumstances where you should use JNI_COMMIT, though they of course list it as an available mode)[4]. While we can clarify the specification to state that after using JNI_COMMIT you must also use 0 or JNI_ABORT to free the array, we cannot fix any of the existing errant code that will only call release with JNI_COMMIT. [1] http://jnicookbook.owsiak.org/recipe-no-013/ [2] https://books.google.com.ph/books?id=NYKMBAAAQBAJ&pg=PA572&lpg=PA572&dq=JNI_COMMIT&source=bl&ots=x0JK95lfud&sig=ACfU3U0aDlX0A_hQziLGu8390dDIvajJ-Q&hl=en&sa=X&ved=2ahUKEwjExo3tsM_tAhV_73MBHQOtAYU4HhDoATAJegQIBxAC#v=onepage&q=JNI_COMMIT&f=false [3] https://cpp.hotexamples.com/examples/-/JNIEnv/ReleaseByteArrayElements/cpp-jnienv-releasebytearrayelements-method-examples.html [4] https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.vm.80.doc/docs/jni_copypin.html
15-06-2021
I went back to the "source" and asked John Rose for his historical perspective on this: The name COMMIT should have been COMMIT_RETAINING_BUFFER and the constant 0 should be COMMIT_FREEING_BUFFER (if not COMMIT, but that ship has sailed). Updating the spec. is expensive, but perhaps it’s worth it as part of the clarification task. The constant COMMIT could arguably be subjected to diagnostic processing in some “JNI checkout” mode of the JVM. At the very least, the JVM could detect whether the “leaked” buffer value is not in the Java heap but rather is a value in a range previously returned by NEW_C_HEAP_ARRAY for one of the GetXArrayElements calls, and report a possible leak. It could also compare the buffer address with the array address(es), when it “knows” that (*isCopy) was JNI_FALSE, and report a bad buffer address. For the XCritical functions, the “carray” plays logically the same role as “buf”, I’m pretty sure. But the Critical functions don’t copy while the others do, which distinguishes the role of buffer pointers in the two cases. *But*, that is a HotSpot convention, not part of the spec. The fact that (*isCopy) can have either boolean value implies that the spec. must contemplate what happens when users do any of the following: 1. Use only the buffer pointers supplied by the JVM, for single uses. 2. Use the buffer pointers supplied by the JVM, for multiple uses, when (*isCopy) is JNI_TRUE. (Implied by JNI_COMMIT, sadly.) 3. Supply their own buffer pointers for writing data to arrays, when (*isCopy) is JNI_TRUE. (Probably *not* an error, sadly.) 4. Supply their own buffer pointers for writing data to arrays, when (*isCopy) is JNI_FALSE. (Probably an error.) In any case, I think it is a semi-legitimate expectation by JNI programmers that they can supply their own buffers and/or recycle buffers supplied by the JNI entry points. Under that theory, the COMMIT option means “I have my own buffer; please copy the data into the array”. For the special case of GetPrimitiveArrayCritical, the returned value is always a base pointer (for HotSpot), so the JVM doesn’t expect a copy operation. Perhaps it should perform one if the buffer pointer is outside of the array’s range of addresses? (That would be an extension of today’s code, which is hardly desirable.) More likely, the JVM could simply require that the buffer pointer be equal to the array base address (or NULL??), and reject all other usages. After all, the (*isCopy) value was JNI_FALSE, so the programmer should not be passing back anything other than the base address of the array. Also, it’s hard to tell what JNI code “in the wild” is doing, which means that we have to be careful about shutting down semi-legitimate expectations, when they possibly can make sense. (But we can supply diagnostic modes to flag gray areas, and then deprecate over time.) Even for usages that can’t possibly make sense, we may be forced to retain existing non-crashing behaviors, at least until we can diagnose and deprecate. — John --- I have a couple of takeaways from that. The first is that while allowing for a user-supplied buffer when the GetX function returns a copy, motivates the existence of the JNI_COMMIT mode, this intent was hidden so well that later JVM implementors were unaware of it. The Xcheck:jni logic for the Get/Release functions cannot work with user-supplied buffers - you must pass to Release the buffer you got from Get. So it seems highly unlikely this is actually a use-case "in the wild". Secondly, a Release with JNI_COMMIT mode should only keep the "transaction" on accessing the array contents open, if a copy was provided. So for the Critical API where hotspot never returns a copy, any call to Release actually does "release" the array. This is consistent with the fact the specification states that the mode is ignored if "carray" is not a copy.
17-12-2020