LZ4JNI array methods leak the source critical array and throw from inside a critical region

[K-ANOY]

File: src/jni/net_jpountz_lz4_LZ4JNI.c

Functions:

• Java_net_jpountz_lz4_LZ4JNI_LZ4_1compress_1limitedOutput (line 101)
• Java_net_jpountz_lz4_LZ4JNI_LZ4_1compressHC (line 148)
• Java_net_jpountz_lz4_LZ4JNI_LZ4_1decompress_1fast (line 195)
• Java_net_jpountz_lz4_LZ4JNI_LZ4_1decompress_1safe (line 242)

All four acquire the source array with GetPrimitiveArrayCritical, then acquire the destination, and return immediately if the destination acquisition fails. That failure path violates two separate JNI obligations. The two newer functions in the same file already handle it correctly.

The affected code

Shown for LZ4_compress_limitedOutput; the other three are identical apart from the compression call.

if (srcArray != NULL) {
  in = (char*) (*env)->GetPrimitiveArrayCritical(env, srcArray, 0);
} else {
  in = (char*) (*env)->GetDirectBufferAddress(env, srcBuffer);
}

if (in == NULL) {
  throw_OOM(env);
  return 0;
}

if (destArray != NULL) {
  out = (char*) (*env)->GetPrimitiveArrayCritical(env, destArray, 0);
} else {
  out = (char*) (*env)->GetDirectBufferAddress(env, destBuffer);
}

if (out == NULL) {
  throw_OOM(env);
  return 0;
}

1. The source critical array is never released

When srcArray != NULL and the destination acquisition fails, the function returns with the source critical region still open. ReleasePrimitiveArrayCritical is only reached on the normal path further down.

Function	Failing return	Normal release
LZ4_compress_limitedOutput	125-128	133, 136
LZ4_compressHC	172-175	180, 183
LZ4_decompress_fast	219-222	227, 230
LZ4_decompress_safe	266-269	274, 277

(The in == NULL branch above it is fine — no region is open at that point.)

2. JNI functions are called while the source critical region is open

The JNI specification states that inside a critical region native code must not call other JNI functions. Two calls in these functions violate that while in is held:

• throw_OOM(env) at lines 126, 173, 220 and 267, which calls (*env)->ThrowNew(...). This allocates a Java exception object and can therefore trigger a garbage collection — exactly what the critical region is meant to prevent.
• (*env)->GetDirectBufferAddress(env, destBuffer) at lines 122, 169, 216 and 263, reached when the source is an array and the destination is a direct buffer. This one neither allocates nor blocks, but it is still a JNI call inside the region.

To be explicit about what is not being claimed here: acquiring the destination with a second GetPrimitiveArrayCritical while the source is held is fine. The specification says "Multiple pairs of GetPrimitiveArrayCritical and ReleasePrimitiveArrayCritical may be nested" and gives a two-array example with the same shape. Only the calls listed above are at issue.

The same file already has the correct pattern

LZ4_compress_fast_extState_fastReset (line 329) and LZ4_compress_HC_extStateHC_fastReset (line 418) were written later and get both points right — they release the source before throwing:

if (out == NULL) {
  if (srcArray != NULL) {
    (*env)->ReleasePrimitiveArrayCritical(env, srcArray, in, 0);
  }
  throw_OOM(env);
  return 0;
}

Six functions in one file share the same structure and two of them handle this path correctly, which is why the omission in the other four looks like an oversight rather than a deliberate difference.

These are the array-backed entry points used by LZ4Factory.nativeInstance(): LZ4JNICompressor.compress calls LZ4_compress_limitedOutput, LZ4HCJNICompressor.compress calls LZ4_compressHC, LZ4JNIFastDecompressor.decompress calls LZ4_decompress_fast, and LZ4JNISafeDecompressor.decompress calls LZ4_decompress_safe.

The out == NULL condition itself is not easy to trigger from the public API. The ByteBuffer overloads guard with (src.hasArray() || src.isDirect()) && (dest.hasArray() || dest.isDirect()), so a heap buffer is passed as a byte[] and GetDirectBufferAddress is only called on genuinely direct buffers. In practice the path is reached when GetPrimitiveArrayCritical on the destination fails, i.e. under memory pressure when the VM has to copy. The consequence is that an allocation failure does not surface as a clean OutOfMemoryError: the method returns with a critical region still held, and the exception is constructed from inside that region.

Suggested fix

Release the source before throwing, matching what the two newer functions already do:

if (out == NULL) {
  if (srcArray != NULL) {
    (*env)->ReleasePrimitiveArrayCritical(env, srcArray, in, 0);
  }
  throw_OOM(env);
  return 0;
}

One such edit per function removes both problems at once, so the patch is four small blocks even though two distinct obligations are involved. Moving the GetDirectBufferAddress call for the destination above the source acquisition would additionally keep that call out of the region, but it is a separate cleanup and not required to fix the failure path.