0

I wrote the following classes which uses member pointer functions:

#include <stdlib.h>
#include <vector>

template<class Type>
class PrimitiveAccessor {
    public :
        PrimitiveAccessor(
            JNIEnv* env, const char name[], const char ctorSig[],
            Type (JNIEnv::*callTypeMethodFunction) (jobject, jmethodID)
        ) {
            this->env = env;
            this->type = (jclass)env->NewGlobalRef(env->FindClass(name));
            this->callTypeMethodFunction = callTypeMethodFunction;
        }
        ~PrimitiveAccessor(){
            env->DeleteGlobalRef(this->type);
        }

    private:
        JNIEnv* env;
        jclass type;
        jmethodID constructorId;
        jmethodID callTypeMethodId;
        Type (JNIEnv::*callTypeMethodFunction) (jobject, jmethodID);
};

class Environment {
    public:
        Environment(JNIEnv* env) {
            this->env = env;
            this->init();
        }
        ~Environment(){
            this->env = 0;
            delete(this->jintAccessor);
            this->jintAccessor = 0;
        }
    private:
        JNIEnv* env;
        PrimitiveAccessor<jint>* jintAccessor;

        void init() {
            jintAccessor = new PrimitiveAccessor<jint>(
                env, "java/lang/Integer",
                "(I)V", &JNIEnv::CallIntMethod
            );
        }
};

But on compiling I obtain the following compilation error:

F:\Shared\Workspaces\Projects\DriverFunctionSupplierNative.cpp: In member function 'void Environment::init()':
F:\Shared\Workspaces\Projects\JNIDriverFunctionSupplierNative.cpp:75:4: error: no matching function for call to 'PrimitiveAccessor<long int>::PrimitiveAccessor(JNIEnv*&, const char [18], const char [5], jint (JNIEnv_::*)(jobject, jmethodID, ...))'
    );
    ^
F:\Shared\Workspaces\Projects\JNIDriverFunctionSupplierNative.cpp:36:3: note: candidate: 'PrimitiveAccessor<Type>::PrimitiveAccessor(JNIEnv*, const char*, const char*, Type (JNIEnv_::*)(jobject, jmethodID)) [with Type = long int; JNIEnv = JNIEnv_; jobject = _jobject*; jmethodID = _jmethodID*]'
   PrimitiveAccessor(
   ^~~~~~~~~~~~~~~~~
F:\Shared\Workspaces\Projects\JNIDriverFunctionSupplierNative.cpp:36:3: note:   no known conversion for argument 4 from 'jint (JNIEnv_::*)(jobject, jmethodID, ...)' {aka 'long int (JNIEnv_::*)(_jobject*, _jmethodID*, ...)'} to 'long int (JNIEnv_::*)(jobject, jmethodID)' {aka 'long int (JNIEnv_::*)(_jobject*, _jmethodID*)'}
F:\Shared\Workspaces\Projects\JNIDriverFunctionSupplierNative.cpp:34:7: note: candidate: 'constexpr PrimitiveAccessor<long int>::PrimitiveAccessor(const PrimitiveAccessor<long int>&)'
 class PrimitiveAccessor {
       ^~~~~~~~~~~~~~~~~

I temporarily fixed it by casting the member function pointer:

void init() {
    jintAccessor = new PrimitiveAccessor<jint>(
        env, "java/lang/Integer",
        "(I)V", (long (JNIEnv::*) (jobject, jmethodID))&JNIEnv::CallIntMethod
    );
}

I noticed that the type passed to the template is expanded: is there a way to avoid this cast?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
JJB
  • 202
  • 4
  • 11
  • `&JNIEnv::CallIntMethod` seems to not match your expected signature, change one of them to match each other (or use something else than member pointer as `std::function`, so you can adapt method though a lambda or equivalent). – Jarod42 Sep 07 '21 at 08:39
  • Yes it is: https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/include/jni.h#L327 The problem is that PrimitiveAccessor is expanded to PrimitiveAccessor during compilation – JJB Sep 07 '21 at 08:42
  • 4
    There is extra ellipsis (as for `printf`)... `Type (JNIEnv::*) (jobject, jmethodID)` != `Type (JNIEnv::*) (jobject, jmethodID, ...)`. – Jarod42 Sep 07 '21 at 08:44
  • 2
    Nothing’s “expanded”. `jint` is simply an *alias* for `long int`. This isn’t the issue here (Jarod’s comment pinpoints the actual problem). – Konrad Rudolph Sep 07 '21 at 08:44
  • 1
    Aside: whilst `0` is a null pointer literal, the preferred one is `nullptr`. Not that you need to modify pointers that are about to cease to exist. – Caleth Sep 07 '21 at 09:05
  • 2
    Aside 2: why do you need a pointer to `PrimitiveAccessor`? `new` isn't needed to create objects in C++ – Caleth Sep 07 '21 at 09:07
  • because i will have to use it in static functions written in C and i need a global variable – JJB Sep 07 '21 at 09:09
  • 1
    You can take the address of a data member. `PrimitiveAccessor jintAccessor;` works fine here – Caleth Sep 07 '21 at 09:11
  • @JJB That’s not a reason. You can still pass a pointer to an automatic variable to your C functions. `PrimitiveAccessor` inside `Environment` almost certainly doesn’t need to be a pointer. – Konrad Rudolph Sep 07 '21 at 09:12
  • I noticed in the link I supplied that `jint (JNICALL *CallIntMethod) (JNIEnv *env, jobject obj, jmethodID methodID, ...);` has a varargs as last parameter: could this be the problem? How do you define a member function pointer that contains a varargs? – JJB Sep 07 '21 at 09:13
  • @JJB Yes, that *is* the problem, as several comments have have already pointed out. – Konrad Rudolph Sep 07 '21 at 09:15
  • @Konrad Rudolph Environment must be defined only once and shared between functions I will write: environment initialization contains operations that will slow down static functions if they were to instantiate the environment every time. The environment will be destroyed on JNI_OnUnload function call executed by the JVM – JJB Sep 07 '21 at 09:15
  • Thanks @Jarod42: solved with using `Type (JNIEnv::*callTypeMethodFunction) (jobject, jmethodID, ...)` – JJB Sep 07 '21 at 09:19
  • 2
    @JJB You have a fundamental misconception here: changing the member type of `Environment` does not change how handle `Environment`, it’s an *implementation detail*. And I know how the JNI works, I’ve written very similar wrappers myself. – Konrad Rudolph Sep 07 '21 at 09:24
  • 1
    `private: JNIEnv* env;` has quite the [code smell](https://en.wikipedia.org/wiki/Code_smell). A `JNIEnv *` value passed from the JVM to a native call is valid only for that one thread and that one native call. Given that you're creating global references, that more than implies that you're planning on caching that `JNIEnv *` value. You can't do that. – Andrew Henle Sep 07 '21 at 09:34

1 Answers1

5

You need to specify a matching type, not a type that is similar.

I have also cleaned up lots of your unidiomatic C++. Don't use new; it isn't required. You don't need a user-defined destructor if your data members clean themselves up. You should initialise your data members in the member initialiser list, not in the body of the constructor. Use std::string for strings.

#include <stdlib.h>
#include <vector>
#include <memory>
#include <string>

struct GlobalRefDeleter {
    JNIEnv* env;
    void operator()(jobject type) {
        env->DeleteGlobalRef(type);
    }
};

using JClass = std::unique_ptr<_jclass, GlobalRefDeleter>;

JClass getClass(JNIEnv* env, const std::string & name) {
    return { static_cast<jclass>(env->NewGlobalRef(env->FindClass(name.c_str()))), env };
}

template<class Type>
class PrimitiveAccessor {
    using CallMethod = Type (JNIEnv::*) (jobject, jmethodID, ...);
    public :
        PrimitiveAccessor(
            JNIEnv* env, const std::string & name, const std::string & ctorSig,
            CallMethod callMethod)
        ) : env(env), type(getClass(env, name)), callMethod(callMethod)
        {
        }

    private:
        JNIEnv* env;
        JClass type;
        jmethodID constructorId;
        jmethodID callTypeMethodId;
        CallMethod callMethod;
};


class Environment {
    public:
        Environment(JNIEnv* env) : env(env), jintAccessor(env, "java/lang/Integer", "(I)V", &JNIEnv::CallIntMethod)
        {
        }
    private:
        JNIEnv* env;
        PrimitiveAccessor<jint> jintAccessor;
};
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Caleth
  • 52,200
  • 2
  • 44
  • 75
  • Good solution, but I must delete the Environment on `JNI_Onunload` function called by the JVM – JJB Sep 07 '21 at 10:13
  • 1
    @JJB so? you can still do that. I haven't changed the public behaviour of `Environment`. The implicitly created destructors do all the same clean-up as your explicit destructors – Caleth Sep 07 '21 at 10:14