Processing a string into JNI layer by passing through java API and creating a buffer using malloc

Question

I am working on an java API which will be used to process a string in cpp and return a processed string.

The Cpp code in the jni I am using is mentioned below

char * buffer;
static
void process(char *str, int shift) {
    // some processing 
}
extern "C" JNIEXPORT jstring JNICALL Java_com_example_process (
        JNIEnv *env,
        jobject,
        jstring string,
        jint shift) {
    const char *key = env->GetStringUTFChars(string, nullptr);
    buffer = (char *)malloc(sizeof(key));

    strcpy(buffer, key);

    char *p = buffer;
    process(buffer, shift);

    env->ReleaseStringUTFChars(string, key);
    return env->NewStringUTF(p);

}
extern "C" JNIEXPORT void JNICALL Java_com_example_freeN (
        JNIEnv *env,
        jobject) {
    free(buffer);
    buffer = nullptr;
}

Java API

fun processStr(string: String): String {

            val str = process(string, 10)
            freeN()
            return str
        }

Couple of points to note here:

1. Since the string passed from java to C using JNI, it is received as const, I cannot do any operation, so buffer is created and using strcpy to copy and then manipulating the string

2. Calling freeN() from java code itself, to free the allocated buffer.

3. There could be another approach of passing java's ByteBuffer from java then using the buffer in JNI This way freeing memory wont be an issue?

are these a right approach to go about this ? What could be better approach? How i can make it thread safe ? Do you think calling this java API asynchronously is a good idea or API should be synchronised ?

Answer 1

I could not figure out from your question whether process relies on its input being null terminated, but note that this is not required by the specification. I also could not figure out what the shape of process' inputs and outputs should be. If you are going to pass in or out arbitrary data, that should be a Java byte[] or ByteBuffer, not a java.lang.String.

env->NewStringUTF(p) allocates a new Java string whose memory is managed by Java.
Once the string has been allocated you can already safely free buffer.

Second, by storing buffer globally, you made your function not reentrant and you made your life harder. Imagine what happens if two threads call into Java_com_example_process simultaneously.

Finally, sizeof(key) returns the size of the pointer, not the thing it points to. So this will fail horribly if your string goes beyond 4 or 8 bytes (depending on architecture). Luckily, you can simply ask the JNI what your string length will be.

I suggest the following rewrite, where the JVM is responsible for disposing of the returned string so you do not have to call freeN anymore.

static void process(const char *in, char *out, int shift) {
    // some processing where you take data from `in` and write to `out`
}

extern "C" JNIEXPORT jstring JNICALL Java_com_example_process (
        JNIEnv *env,
        jobject,
        jstring string,
        jint shift) {
    jsize key_size = env->GetStringUTFLength(string);
    const char *key = env->GetStringUTFChars(string, nullptr);

    char * buffer = new char[key_size];
    process(key, buffer, shift);

    env->ReleaseStringUTFChars(string, key);
    jstring ret = env->NewStringUTF(buffer);
    delete[] buffer;

    return ret;
}