Invalid free for methods that return array with @Const and @Adapter annotations

[equeim]

I.e. suppose we have this native function

const std::string& get_stdstring() const;

On Java side we will have this method:

public native @Const @ByRef @StdString byte[] get_stdstring();

And generated JNI code will be:

JNIEXPORT jbyteArray JNICALL Java_com_example_myapplication_nat_NativeClass_get_1stdstring(JNIEnv* env, jobject obj) {
    NativeLibrary::NativeClass* ptr = (NativeLibrary::NativeClass*)jlong_to_ptr(env->GetLongField(obj, JavaCPP_addressFID));
    if (ptr == NULL) {
        env->ThrowNew(JavaCPP_getClass(env, 6), "This pointer address is NULL.");
        return 0;
    }
    jbyteArray rarg = NULL;
    const signed char* rptr;
    jthrowable exc = NULL;
    try {
        StringAdapter< char > radapter((dynamic_cast<JavaCPP_NativeLibrary_0003a_0003aNativeClass*>(ptr) != NULL ? ((JavaCPP_NativeLibrary_0003a_0003aNativeClass*)ptr)->get_stdstring() : ptr->get_stdstring()));
        rptr = radapter;
        jlong rcapacity = (jlong)radapter.size;
        void (*deallocator)(void*) = &StringAdapter< char >::deallocate;
        if (rptr != NULL) {
            rarg = env->NewByteArray(rcapacity < INT_MAX ? rcapacity : INT_MAX);
            env->SetByteArrayRegion(rarg, 0, rcapacity < INT_MAX ? rcapacity : INT_MAX, (jbyte*)rptr);
        }
        if (deallocator != 0 && rptr != NULL) {
            (*(void(*)(void*))jlong_to_ptr(deallocator))((void*)rptr);
        }
    } catch (...) {
        exc = JavaCPP_handleException(env, 7);
    }

    if (exc != NULL) {
        env->Throw(exc);
    }
    return rarg;
}

The problem is that rptr is const signed char* due to @Const annotation and StringAdapter's conversion operator doesn't allocate memory when returning const pointer. Therefore, when we will call deallocator, process will be terminated due to invalid free. I suppose that this code should check if method has @Const annotation and call deallocator only if it hasn't. Or better, just always declare rptr as const, and never call deallocator. rptr is immediately passed to SetByteArrayRegion() which does its own copy and that's that. There is no need for rptr not to be const.

[saudet]

I've made changes that fixes this in the latest commit. Please give it a try, and thanks for reporting!

[equeim]

Yep, it fixed this for me. Although I still don't really understand why we need to perform additional allocation and subsequent deletion of char* buffer if get_stdstring() returns string by value or non-const reference.

[saudet]

It's just how it's assuming the adapter works, which is useful when returned as a Pointer, in this case. We need more documentation I guess, but it's not crystal clear what's the best way to do all that either. If you'd like to work on that, it would be great:
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#using-adapters-for-c-container-types

[equeim]

But in this case it doesn't return a Pointer, it returns an array. Only thing that is done with the pointer returned from adapter is SetByteArrayRegion() which doesn't take ownership over it, it does a deep copy. Then the pointer is deleted if it was non-const.
My point is that if we return an array, we should always get the const pointer from adapter regardless of whether C++ return type that is passed to adapter is const or not. We don't need to copy data because, unlike when we return a Pointer, we don't take ownership in this case.

[saudet]

Yes, I got that, that's fine. Like I said, you're more than welcome to work on that.

[saudet]

The bug is fixed in JavaCPP 1.5.1, but please do send a pull request with your additional enhancement when you have the chance! Thanks for your help

[saudet]

Actually, I think commit 55e3bdc solves the second issue you're describing. With these changes, I'm pretty sure that the deallocator will only be needed and used in the case of non-const data returned as a Pointer. Let me know what you think! Thanks