Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointer types don't work on the native side if allocated in java #747

Closed
iexavl opened this issue Mar 9, 2024 · 16 comments
Closed

Pointer types don't work on the native side if allocated in java #747

iexavl opened this issue Mar 9, 2024 · 16 comments

Comments

@iexavl
Copy link

iexavl commented Mar 9, 2024

I have a problem where if I allocate something on the java side and pass it to a function on the native side it seems like natively all the properties of that object retain their default value. In other words int , double , long =0 even if they aren't and so on. This happened most recently when I tried to make an std::vectorstd::string and pass it to a c++ function. Here's a short example:

//test_header.hpp
#include <vector>
#include <string>
#include <iostream>
#ifdef EXPORTIMPORT
#define EXPIMP __declspec(dllexport)
#else
#define EXPIMP __declspec(dllimport)
#endif // EXPORTIMPORT
struct Foo {
	EXPIMP int test(std::vector<std::string>* vec);
};
//test_source.cpp
#include "test_header.hpp"
int Foo::test(std::vector<std::string>* vec) {
	std::cout << vec->size() << std::endl;
	return 0;
}
//Mapper
@Properties(
        value=@Platform(
        includepath = {"D:\\JavaProjects\\nativemvntest\\src\\main\\java\\org\\example\\nat"},
                linkpath = {"D:\\JavaProjects\\nativemvntest\\src\\main\\java\\org\\example\\nat"},

        include ={"test_header.hpp"},
                link={"Mvntest"}

),
target = "org.example.parsed.NativeLibrary",
global = "org.example.parsed.global.NativeLibrary")
public class Mapper implements InfoMapper {
    @Override
    public void map(InfoMap infoMap) {
        infoMap.put(new Info("std::vector<std::string>").pointerTypes("StringVector").define());
    }
}
//main
public class Main {
    public static void main(String[] args) {
        Foo foo = new Foo();
        StringVector vec = new StringVector(5);

        foo.test(vec);
    }

}

The cpp gets compiled into a dll, called Mvntest.dll and gets linked against in the mapper. I parse the files, compile them and run the example and it prints out 0. Also If I try to add any strings to this vector via push_back and print them out on the native side I get an ACCESS_VIOLATION

@saudet
Copy link
Member

saudet commented Mar 9, 2024

It sounds like you're not using the same compiler for Mvntest and JavaCPP. We need to use the same C++ compiler for everything

@iexavl
Copy link
Author

iexavl commented Mar 9, 2024

It sounds like you're not using the same compiler for Mvntest and JavaCPP. We need to use the same C++ compiler for everything

Yup. That did the trick.

@iexavl iexavl closed this as completed Mar 9, 2024
@iexavl
Copy link
Author

iexavl commented Mar 9, 2024

It sounds like you're not using the same compiler for Mvntest and JavaCPP. We need to use the same C++ compiler for everything

So that did the trick but now I have another problem. At the moment instead of compiling the generated cpp files and my other cpp files separately I add them to the project solution and bundle them in the same dll ( in the example that would be Mvntest), but when Loader.load() is called in the static block of the generated java classes they try to load "jni" + c.getSimpleName() , where c is the class in the global package (in this case NativeLibrary, so it tries to load jniNativeLibrary ) but that library just doesn't exist because everything is packaged into Mvntest. I tried setting the library() in the Platform annotation on the Mapper, but that doesn't get inherited in the parsed classes (Foo and StringVector) so it just tries to load jniNativeLibrary again. Is there a way to do this or does my dll have to be called jniNativeLibrary.dll?
Oh and another thing which is when the generation of the cpp files happens I get this method:

static inline void JavaCPP_loadGlobal(JNIEnv* env, jclass cls, const char* filename) {
#ifdef _WIN32
    HMODULE handle = LoadLibrary(filename);
    if (handle == NULL) {
        char temp[256];
        sprintf(temp, "LoadLibrary() failed with 0x%lx", GetLastError());
        env->ThrowNew(JavaCPP_getClass(env, 14), temp);
    }
#else
  /*......*/
#endif
}

The problem here is that the LoadLibrary method doesn't take a const char* , but a LPCWSTR instead. The method that takes
const char* as a parameter is LoadLibraryA and I have to change it manually each time I generate the files. Is there a fix for that?

@iexavl iexavl reopened this Mar 9, 2024
@saudet
Copy link
Member

saudet commented Mar 10, 2024

Sure, that's what @Platform(library=... is for:
http://bytedeco.org/javacpp/apidocs/org/bytedeco/javacpp/annotation/Platform.html#library--

You could add a @Platform(define="LoadModule LoadModuleA", ... for that.
You can do whatever you need to make it work!

@iexavl
Copy link
Author

iexavl commented Mar 10, 2024

Sure, that's what @Platform(library=... is for: http://bytedeco.org/javacpp/apidocs/org/bytedeco/javacpp/annotation/Platform.html#library--

You could add a @Platform(define="LoadModule LoadModuleA", ... for that. You can do whatever you need to make it work!

I already have the @Platform(library=... and it does work but only for the top level class which is the Mapper. Any classes that inherit it's @Properties don't inherit the @Platform property and because they don't they attempt to load the library with the default name "jni" + the name that Class.getSimpleName() returns (quoted from the documentation). Also the define did work, but the LoadLibrary is actually in the generated jnijavacpp.cpp file and not the generated cpp file for my header files ( jniNativeLibrary.cpp in this case). When I put the @Platform(define="LoadModule LoadModuleA", ... it puts it in jniNativeLibrary.cpp

@saudet
Copy link
Member

saudet commented Mar 10, 2024

Well, I guess we can consider those as bugs that need to be fixed then

@saudet saudet added the bug label Mar 10, 2024
@iexavl
Copy link
Author

iexavl commented Mar 10, 2024

Well, I guess we can consider those as bugs that need to be fixed then

I mean I see where the problem is, but I'm not entirely sure if it's exactly a problem or intended behavior. My question is: are classes that have a @Properties(inherit =... supposed to also inherit the @Platform annotations from their parent classes? Or at least in the cases where the inheritors don't have a @Platform or @Properties(values=... annotations set themselves? Or at the very least in the cases where the parent has a @Properties(library=... (since that is pretty much my exact issue with this)?

saudet added a commit that referenced this issue Mar 11, 2024
@saudet
Copy link
Member

saudet commented Mar 11, 2024

I've pushed potential fixes to commit e824acf but I haven't tested them. Could you test them for me?
They should be available as part of the snapshots: http://bytedeco.org/builds/

@iexavl
Copy link
Author

iexavl commented Mar 11, 2024

I've pushed potential fixes to commit e824acf but I haven't tested them. Could you test them for me? They should be available as part of the snapshots: http://bytedeco.org/builds/

The LoadLibraryA fix works, but the inheritance of the library() doesn't. I have presumably fixed it, but to be honest I haven't ever made a commit to a github repository so I'll have to look into that before I can commit the changes for review :D

@iexavl
Copy link
Author

iexavl commented Mar 11, 2024

I've pushed potential fixes to commit e824acf but I haven't tested them. Could you test them for me? They should be available as part of the snapshots: http://bytedeco.org/builds/

Here what fixed it for me 896f288 here the code searches through all enclosing classes, starting with the ones that are in the @Properties(inherit=...

@saudet
Copy link
Member

saudet commented Mar 12, 2024

The original code already searches through all enclosing classes. It should be possible to fix this by changing only a line or two somewhere.

@iexavl
Copy link
Author

iexavl commented Mar 12, 2024

The original code already searches through all enclosing classes. It should be possible to fix this by changing only a line or two somewhere.

It does? That would be great, but I couldn't find it. I saw the getEnclosingClass(Class cls) function, which from what I can tell is the only function that gets called to look through enclosing classes, and that function does look through enclosing classes, but it only searches through the super classes . It does not search through the classes that are marked in the current class's @Properties annotation as part of it's inherit array, and that's where the problem was coming from. It's possible that I missed some other function that does it along the way, but I have read this piece of code more times than I would like to admit. And this wouldn't be an issue if every generated class inherited from whatever InfoMapper implementation was run to generate them, but the generated classes extend Pointer (which makes sense) and have only one thing as their annotation, which is @Properties(inherit = <package>.<InfoMapperImplementation>.class).

@saudet
Copy link
Member

saudet commented Mar 12, 2024

Well, anyway, please provide a complete but small example that fails, and I'll see what we can do about it

@iexavl
Copy link
Author

iexavl commented Mar 12, 2024

Well, anyway, please provide a complete but small example that fails, and I'll see what we can do about it

Okay, here is an example:

test_header.hpp

struct Foo {
	void test();
};

test_source.cpp

#include "test_header.hpp"
void Foo::test() {
	//do stuff...
}

Mapper:

package org.example;

import org.bytedeco.javacpp.annotation.Platform;
import org.bytedeco.javacpp.annotation.Properties;
import org.bytedeco.javacpp.tools.InfoMap;
import org.bytedeco.javacpp.tools.InfoMapper;

@Properties(
        value=@Platform(
        includepath = {"<path to test_header.hpp>"},
        include ={"test_header.hpp"},
                library = "GeneratedLibrary"

),
target = "org.example.parsed.NativeLibrary",
global = "org.example.parsed.global.NativeLibrary")
public class Mapper implements InfoMapper {
    @Override
    public void map(InfoMap infoMap) {
    }
}

Main:

package org.example;
import org.example.parsed.NativeLibrary.Foo;
public class Main {
    public static void main(String[] args) {
    Foo f= new Foo();
    }

}

Now you just have to parse the test_header.hpp file, compile the parsed .java files into .class files , link the test_source.cpp any way you wish, generate and compile the cpp files, run the program and you have yourself a crash.
This example crashes on a few different levels:

  1. After the classes have been parsed and compiled (in this case it's only Foo.java, along with NativeLibrary.java in the global package) when the generator runs it generates a cpp file and assuming all the symbols have been resolved ,
    the generated file is called jniNativeLibrary.cpp . Accordingly, the .dll file that gets generated is called jniNativeLibrary.dll . After that when the program runs and the class in the global package ( org.example.parsed.global.NativeLibrary.class ) tries to call Loader.load() in it's static block, it attempts to load GeneratedLibrary.dll . As I said before though the actual generated and compiled file is called jniNativeLibrary.dll (it results in an UnsatistfiedLinkError)
  2. Lets assume we manually change the name of jniNativeLibrary.dll to GeneratedLibrary.dll so org.example.parsed.global.NativeLibrary.class successfully loads. Now we go back to Foo.java (which has been compiled into Foo.class). Loader.load() is called in Foo.class and because of the aforementioned not inheriting of the library() property of parent classes, it attempts to load jniNativeLibrary.dll. That file though has been renamed back to what it should have originally been: GeneratedLibrary.dll . This again ends up with an UnsatistfiedLinkError .

And although I haven't noticed that before, the fix for the library property inheritance also fixed the generated cpp file being called jni + c.getSimpleName(); even though there is a library() property set (To be honest I didn't even notice that was happening until recently).

@saudet
Copy link
Member

saudet commented Mar 12, 2024

With my fixes from commit e824acf, it both generates a GeneratedLibrary library just fine, and loads it just fine. I can't fix what isn't broken! Please provide me with an example that doesn't work.

@iexavl
Copy link
Author

iexavl commented Mar 12, 2024

With my fixes from commit e824acf, it both generates a GeneratedLibrary library just fine, and loads it just fine. I can't fix what isn't broken! Please provide me with an example that doesn't work.

Okay, I was wrong, the fix does work. I am not entirely sure what I did last 2 times I ran it for it to go awry, but when I just reset everything to how it was before and recompiled it with the simple fix, it worked just fine. Sorry for the trouble and thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants