apache / apache/carbondata

CSDK Configuration leaks JNI local refs in get/set and stores FindClass/NewObject without NewGlobalRef

Open
#4,403 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Scala
Stars
1.5k
Forks
694
PR merge metrics
No merged PRs in 30d

Description

## Summary

`sdk/CSDK/src/Configuration.cpp` leaks JNI local references.

CSDK takes a caller-supplied `JNIEnv*` (typically after `JNI_CreateJavaVM` / `AttachCurrentThread`). Local refs on that thread are **not** freed when a C++ function returns. `get`/`set` never delete the `NewStringUTF` arguments; the constructor keeps `FindClass` / `NewObject` results as member fields without promoting them to global refs, and there is no destructor.

## Affected code

[`sdk/CSDK/src/Configuration.cpp`](https://github.com/apache/carbondata/blob/f86ac085ddbbd8381b0a5b65658731fe618eff4a/sdk/CSDK/src/Configuration.cpp)

## 1. `get` / `set` — `jstring` arguments never released (main leak)

```cpp
jvalue args[2];
args[0].l = jniEnv->NewStringUTF(key);
args[1].l = jniEnv->NewStringUTF(value);
jniEnv->CallObjectMethodA(configurationObject, setID, args);
// args[0].l and args[1].l never DeleteLocalRef'd

get only deletes the return value:

jobject result = jniEnv->CallObjectMethodA(configurationObject, getID, args);
char *str = (char *) jniEnv->GetStringUTFChars((jstring) result, JNI_FALSE);
jniEnv->DeleteLocalRef(result); // key/defaultValue jstrings still leaked
return str;

Each set/get leaves two extra local refs in the table. Loading many Hadoop/S3 properties will accumulate them until the thread detaches.

2. Constructor — local refs stored as members

configurationClass = env->FindClass("org/apache/hadoop/conf/Configuration");
configurationObject = jniEnv->NewObject(configurationClass, initID);

These are kept in Configuration.h as fields and reused by get/set. They must be global refs (NewGlobalRef then DeleteLocalRef on the originals). As written they occupy local-ref table slots for the object's lifetime.

CarbonSchemaReader::readSchema also constructs a stack Configuration conf(jniEnv) and never releases those refs, so each call leaks two more.

There is no ~Configuration() to DeleteGlobalRef even after a correct promotion.

Note: after NewObject, the null check still tests configurationClass instead of configurationObject.

Suggested fix
After CallObjectMethodA, release the argument strings:

jniEnv->DeleteLocalRef(args[0].l);
jniEnv->DeleteLocalRef(args[1].l);

In the constructor:

jclass localClass = env->FindClass("org/apache/hadoop/conf/Configuration");
configurationClass = (jclass) env->NewGlobalRef(localClass);
env->DeleteLocalRef(localClass);

jobject localObj = env->NewObject(configurationClass, initID);
configurationObject = env->NewGlobalRef(localObj);
env->DeleteLocalRef(localObj);

Add a destructor that DeleteGlobalRefs both. Prefer a small RAII wrapper so throws cannot skip cleanup.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read sdk/CSDK/src/Configuration.cpp and Configuration.h, starting with the Configuration constructor and its get/set methods; also inspect CarbonSchemaReader::readSchema for stack Configuration usage. Done means temporary JNI references are released, stored references have matching destructor cleanup, and the constructor checks the created object correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, java
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.