Problems upgrading beyond version 1.5.4

[Davidson, Josh]

Have a long-standing issue where our codebase is stuck on 1.5.4, but am interested in upgrading to 1.5.8 to test the unique_ptr changes to see if they address some problems we’ve had with those in the past. When I generate bindings in 1.5.5+ many of our methods that deal with byte arrays are busted. For example, this method:
virtual ErrorCode send(const void* msg_ptr, size_t msg_len_bytes, Duration timeout) = 0;

The generated java code for that (which doesn’t change between 1.5.4 and 1.5.5) looks like:
public native @Cast("ErrorCode") int send(@Cast("const void*") BytePointer msg_ptr, @Cast("size_t") long msg_len_bytes, @ByVal DurationNative timeout);

I have these setup in the parser config:
infoMap.put(new Info("std::string").annotations("@StdString").valueTypes("String").pointerTypes("BytePointer"));
infoMap.put(new Info("const char").valueTypes("byte").pointerTypes("String", "String"));
infoMap.put(new Info("const void").valueTypes("const void").pointerTypes("@Cast(\"const void*\") BytePointer"));
infoMap.put(new Info("void").valueTypes("void").pointerTypes("@Cast(\"void*\") BytePointer"));

If I look at the JNI C++ between 1.5.4 and 1.5.5, there aren’t any real differences when it comes to the method (class indexes change). Biggest differences I notice are inclusion of java/nio/charset/Charset and a bunch of new string utility functions that don’t seem like they’d be relevant. When I invoke the method from java, I get an NPE:

java.lang.RuntimeException: java.lang.NullPointerException
│ │ │ at <package>.MessagingImpl$ConnectionNative.send(Native Method)
│ │ │ at <package>.IConnectionAdapter.send(IConnectionAdapter.java:79)
│ │ │ at <package>.TestConnectionsLocator.testConnectionAddingSenderReceiverSegmenter(TestConnectionsLocator.java:178)

The adapter in between the UT and native call is a simple façade that translates native java types to javacpp types. UT code resembles:

byte[] messageSentBytes = getRandomByteArray(default_max_message_tx_size);
System.err.println("Sending from ut: " + messageSentBytes);
connection.send(messageSentBytes, timeout);

When running I see this output: Sending from ut: [B@710b30ef

The adapter implementation changes that byte array to a byte pointer:
System.err.println("Bytes: " + message);
BytePointer ptr = new BytePointer(message);
int returncode = connection.send(ptr, message.length, TimeIface.nanoseconds(timeout.toNanos())); //NPE here

Of course, when running I see the output: Bytes: [B@710b30ef

Any thoughts on what I need to change?

[Josh Davidson]

It doesn't appear that it's an issue with passing in byte arrays.  That does work ok in both 1.5.4 and 1.5.5.  I do think the string charges in 1.5.5 are causing problems (there may be other issues).  I instantiate a bound logger object from Java that I pass into the constructor of various C++ classes.  When I turn up the log level and run in 1.5.5 the c-style string messages passed to the logger from native C++ show up as blanks.  When I do the same with 1.5.4, they show up.

[Samuel Audet]

Are you sure it's not just GC prematurely deallocating that BytePointer?
Try to deallocate it manually, or try to disable GC by setting the
"org.bytedeco.javacpp.nopointergc" system property to "true", and check
the log by setting the "org.bytedeco.javacpp.logger.debug" system
property to "true".

[Davidson, Josh]

I'm attaching a smallish example that demonstrates one of the issues when migrating to 1.5.5. Remove the .allow from the extension and extract. *NOTE* you'll need to copy in javacpp-1.5.4.jar and javacpp-1.5.5.jar into the directory as I left them as symlinks in the tarball to keep the file size small. The code is very hackish; was just trying to create a minimal example. It might look like a lot of files, but it's really just two skeleton interface classes and two corresponding concrete classes. Contents:

* example/build.sh - If you're on an x86_64 system should hopefully build copies of the code using both 1.5.4 and 1.5.5
* example/run_cpp_main.sh - Runs a native/C++ main built in buid.sh mimicking what's in HelloWorld.java
* example/run54.sh - Run the build.sh JavaCPP 1.5.5outputs using JavaCPP 1.5.4
* example/run55.sh - Run the build.sh JavaCPP 1.5.5 outputs
* example/native/ - Two C++ interfaces and concrete classes for the interfaces that are bound (plus a main)
* example/shims/ - Shim/adapter classes that are used to adapt interfaces that are not easily translated to Java (in this case varargs in in ILogger)
* example/java/bindings/ - JavaCPP parser configuration
* example/java/src/ - Wrappers for bindings and HelloWorld (see run54.sh and run55.sh)

Since it's short, the HelloWorld main is:
public static void main(String [] args) {
ILogger logger = new Logger();
IConnection connection = new Connection(logger);
connection.send("Taco".getBytes());
}

The implementation of connection.send just logs the bytes received. When you run the 1.5.4 build you'll see:
[LOGGING] Send called with 4 bytes
[LOGGING] Byte[0]: 84
[LOGGING] Byte[1]: 97
[LOGGING] Byte[2]: 99
[LOGGING] Byte[3]: 111

When you run the 1.5.5 build you'll see:
[LOGGING]
[LOGGING]
[LOGGING]
[LOGGING]
[LOGGING]

The "[LOGGING]" is just a simple prefix added by the Logger class to each log message received, so with 1.5.5 the string messages aren't getting passed correctly.

Would really appreciate any insight. Let me know if anything is unclear.

[Samuel Audet]

I'd say there's something wrong with your Logger mechanism. If I change
Connection.cpp with the following function, I get "Taco" displayed as
expected, regardless of the version of JavaCPP:

int Connection::send(const void* msg_ptr, size_t msg_len_bytes) const {
std::cout << std::string((char*)msg_ptr, msg_len_bytes).c_str() <<
std::endl;
return msg_len_bytes;
}

Could you clarify what the issue is exactly? Are you trying to debug the
Logger? It sounds like you need to implement the Logger in Java, in
which case you'll need to "virtualize" it, but this gets cancelled when
you "purify" it, so at the very least you'll need to remove this
"purify" to have any chance of having it work as expected:

infoMap.put(new
Info("test::ILogger").purify(true).virtualize().pointerTypes("ILoggerInternal"));


On 11/12/22 15:03, Davidson, Josh wrote:
> I'm attaching a smallish example that demonstrates one of the issues when migrating to 1.5.5. Remove the .allow from the extension and extract.*NOTE* you'll need to copy in javacpp-1.5.4.jar and javacpp-1.5.5.jar into the directory as I left them as symlinks in the tarball to keep the file size small. The code is very hackish; was just trying to create a minimal example. It might look like a lot of files, but it's really just two skeleton interface classes and two corresponding concrete classes. Contents:

[Davidson, Josh]

> I'd say there's something wrong with your Logger mechanism. If I change

> Connection.cpp with the following function, I get "Taco" displayed as

> expected, regardless of the version of JavaCPP:

Yeah, data gets passed into the direct call to the connection send method, Ok. The problem is when the connection object attempts to interact (via string passing) with a dependency that was instantiated and passed in from Java. I thought the problem was with passing in void* since many/most of code we have was blowing up when invoking methods that took parameters of that data type. However, I tried to recreate a very minimal example and discovered that the actual problem was something else.

> Could you clarify what the issue is exactly? Are you trying to debug the

> Logger? It sounds like you need to implement the Logger in Java, in

> which case you'll need to "virtualize" it, but this gets cancelled when

> you "purify" it, so at the very least you'll need to remove this

> "purify" to have any chance of having it work as expected:

Sorry, I should have provided more context. I am trying to support both approaches where Java can provide an implementation of the logger interface that gets passed into native code as well as the case where Java interacts with native code that implements that Java interface. The example code I am providing is demonstrating the latter (although it also has the scaffolding in place to support the former).



The reason that the native ILogger interface is purified is because it’s not implementable in Java. That’s where ILoggerShim.h comes into play. It implements the “un-implementable” methods in-terms-of new pure virtual methods that can be overloaded in Java. If you remove the purify, the code won’t compile. If you look at the parser config (LogIfaceConfig.java), you can see that the unimplementable methods are skipped in both the original ILogger (pure virtual) and ILoggerShim (implemented in-terms-of new method that can be implemented in Java). I don’t want to expose the original ILogger interface to Java, only the ILoggerShim. There are adapters in both native code (LoggerAdapter.h) and Java (ILoggerAdapter.h) that are used to wrap the undesirable interfaces or objects with the idealized interfaces. I realize there’s a bit of indirection that can take a minute to wrap your head around.



I don’t have issues with this approach for methods that don’t take string types. I’m attaching a new example where I added a second method to the native ILogger interface:

virtual void log(const char* msg, ...) const = 0;

virtual void logInt(int x) const = 0;



I thenupdated Connection.cpp to call that second method just before returning:

int Connection::send(const void* msg_ptr, size_t msg_len_bytes) const {

std::stringstream ss;

ss << "Send called with " << msg_len_bytes << " bytes";

logger_->log(ss.str().c_str());

ss.str(std::string());

ss.clear();

for (size_t i = 0; i < msg_len_bytes; ++i) {

ss << "Byte[" << i << "]: " << int(reinterpret_cast<const char*>(msg_ptr)[i]);

logger_->log(ss.str().c_str());

ss.str(std::string());

ss.clear();

}



logger_->logInt(static_cast<int>(msg_len_bytes));

return msg_len_bytes;

}



If I run with 1.5.4:

[LOGGING] Send called with 4 bytes

[LOGGING] Byte[0]: 84

[LOGGING] Byte[1]: 97

[LOGGING] Byte[2]: 99

[LOGGING] Byte[3]: 111

[LOGGING INT] 4



And again with 1.5.5:

[LOGGING]

[LOGGING]

[LOGGING]

[LOGGING]

[LOGGING]

[LOGGING INT] 4



You can see that the call into the Java instantiated logger for the logInt method works, but the strings are not passed through correctly. I’m attaching an updated example. Same instructions apply. As always, really appreciate your insight into this.

[Davidson, Josh]

One other quick follow up, I added some tracing code that provides a bit more insight. When we look at the last call to the logger (logInt) in the connection method between 1.5.4 and 1.5.5:

1.5.4:
ILoggerShim::lognt(int)
ILoggerAdapter[Java]::logAnInt(int)
LoggerAdapter::logAnInt(int)
[LOGGING INT] 4e

1.5.5:
ILoggerShim::lognt(int)
ILoggerAdapter[Java]::logAnInt(int)
LoggerAdapter::logAnInt(int)
[LOGGING INT] 4

It's example the same. If we look at the first call to the base "log" method that takes the string type...
1.5.4:
ILoggerShim::log(const char*, ...) Send called with 4 bytes
ILoggerAdapter[Java]::log(String) Send called with 4 bytes
LoggerAdapter::log(string)

[LOGGING] Send called with 4 bytes

1.5.5:
ILoggerShim::log(const char*, ...) Send called with 4 bytes
ILoggerAdapter[Java]::log(String)
LoggerAdapter::log(string)
[LOGGING]

So the call stack is the same between the 2. In both versions, the "shim" overload is called correctly with the c-style string, but in 1.5.5 build, the C++ string isn't passed through to the Java overload.

--
You received this message because you are subscribed to the Google Groups "javacpp" group.
To unsubscribe from this group and stop receiving emails from it, send an email to javacpp-proje...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/javacpp-project/FAC24681-725A-495F-928F-09F411BF7144%40lmco.com.

[Samuel Audet]

On 11/14/22 13:38, Davidson, Josh wrote:
> You can see that the call into the Java instantiated logger for the logInt method works, but the strings are not passed through correctly. I’m attaching an updated example. Same instructions apply. As always, really appreciate your insight into this.

I figured out what the problem was. There was a JavaCPP_createString()
JNI helper method that didn't use the size from the adapter that got
upgraded to JavaCPP_createStringFromBytes() that does take into account
the size, but GCC in this case would evaluate the size argument before
the pointer one, and it ended up always being 0. Other compilers in
different circumstances may evaluate them in a different order, it's
unspecified, leading to undefined behavior:
https://en.cppreference.com/w/cpp/language/eval_order

I've fixed that in the latest commit, so please give it a try with
1.5.9-SNAPSHOT: http://bytedeco.org/builds/

BTW, if you have other bugs like that to report, if possible, please
open issues on GitHub. It's easier to keep track of them like that:
https://github.com/bytedeco/javacpp/issues

In any case, thanks for the help debugging this one!

Samuel

[Davidson, Josh]

Samuel,

Thanks so much for addressing that issue so quickly. I can confirm that the new snapshot addresses the string issues we were seeing. I will use GitHub issues to report future issues. I think we're still having issues with unique_ptr's, but I'm having a hard time getting a good example. Let's say you have an interface that looks like:

class IFactory {
public:
virtual ~IFactory() {}
virtual std::unique_ptr<IItem> createItem(const std::string& name) = 0;
}


If I virtualize this interface and then implement it in Java, I think I'm running into problems when I pass it into native C++. In some environments I'm running into seg fault issues in destructors, which I strongly suspect is related to the native code calling "createItem", storing the return value in a unique_ptr which gets deleted when it goes out of scope. Does anything special need to be done to implement "createItem" in Java such that if it the return value is deleted from native C++, I won't run into issues. For reference, the generated code for that method in the parser looks like:
@Virtual(true) public native @Cast({"IItem*", "std::unique_ptr<IItem>"}) @UniquePtr IItem createItem(String name);

--
You received this message because you are subscribed to the Google Groups "javacpp" group.
To unsubscribe from this group and stop receiving emails from it, send an email to javacpp-proje...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/javacpp-project/6a102640-2812-6481-869e-89408d3cf34c%40gmail.com.

[Samuel Audet]

In theory, Java shouldn't be able to do anything with the object that
gets created in createItem(), after it returns. If it still looks
accessible, that would cause problems, yes. Is that the case?

[Davidson, Josh]

Well, the implementation in Java for that interface just looks like:
public IItem createItem(String name) {
return new ConcreteItem(name);
}

And that is invoked from native code (i.e. no references are kept in Java).

[Samuel Audet]

Unless you're doing something special when creating a ConcreteItem, a
Deallocator is going to get registered, and that might get called on GC
when it becomes unreachable. That Deallocator needs to see a null
pointer here, and that's probably not happening... For now, calling
deallocate(false) before returning should do the trick.

[Davidson, Josh]

Thanks; I gave that a shot but I'm still having problems. I'm wondering if creating the "ConcreteItem" from Java, which implements a virtualized C++ interface isn't "deleteable" from C++. If I release the unique_ptr in the native code's destructor so that it's not deleted, the problem goes away. I've also tried deallocating the native object that holds the reference manually (so that its destructor is called before any of the Java objects go out of scope), and it didn't make a difference.

This is a representative stack trace:
Current thread (0x00007efe149e1000): JavaThread "JavaCPP Deallocator" daemon [_thread_in_native, id=2418, stack(0x00007efdb9f16000,0x00007efdba017000)]

Stack: [0x00007efdb9f16000,0x00007efdba017000], sp=0x00007efdba0157e8, free space=1021k
Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
C 0x0000000000000025
j org.bytedeco.javacpp.Pointer$NativeDeallocator.deallocate()V+27
j org.bytedeco.javacpp.Pointer$DeallocatorReference.clear()V+46
j org.bytedeco.javacpp.Pointer$DeallocatorThread.run()V+11
v ~StubRoutines::call_stub
V [libjvm.so+0x8adf03] JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, Thread*)+0x333
V [libjvm.so+0x8ac090] JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x190
V [libjvm.so+0x8ac165] JavaCalls::call_virtual(JavaValue*, Handle, Klass*, Symbol*, Symbol*, Thread*)+0x85
V [libjvm.so+0x95d0af] thread_entry(JavaThread*, Thread*)+0x7f
V [libjvm.so+0xe75619] JavaThread::thread_main_inner()+0x1e9
V [libjvm.so+0xe7141c] Thread::call_run()+0x15c
V [libjvm.so+0xc26d5e] thread_native_entry(Thread*)+0xee

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
j org.bytedeco.javacpp.Pointer$NativeDeallocator.deallocate(JJ)V+0
j org.bytedeco.javacpp.Pointer$NativeDeallocator.deallocate()V+27
j org.bytedeco.javacpp.Pointer$DeallocatorReference.clear()V+46
j org.bytedeco.javacpp.Pointer$DeallocatorThread.run()V+11
v ~StubRoutines::call_stub

siginfo: si_signo: 11 (SIGSEGV), si_code: 1 (SEGV_MAPERR), si_addr: 0x0000000000000025


Again, ConcreteItem is a Java class that implements a virtualized C++ interface (IItem). Should native code be code be capable of calling delete on references to those Java objects? Interactions between the native code and the ConcreteItem appear to work Ok. I just run into problems when the native code deletes it. I'm also wondering about DeallocatorThread.run() showing up in the stack trace. Should I be seeing that if I explicitly call "deallocate()" on a bound object? If not, it would seem that the problem is a subsequent call to deallocate that's causing problems.

[Samuel Audet]

As long as your abstract class IItem has a virtual destructor, it
doesn't matter how it gets deallocated. To make extra sure it doesn't
get deallocated in Java though, set the
"org.bytedeco.javacpp.nopointergc" system property to "true", and also
set the "org.bytedeco.javacpp.logger.debug" one to "true", and check the
messages on the console for anything suspicious.

[Samuel Audet]

Hi,

Any progress on that front? If you're still having problems with that,
and if you can prepare a small example to reproduce this here, I'd be
glad to take a look at it. And if possible, please open an issue on
GitHub since it makes it easier to track issues. Thanks!

Samuel

[Davidson, Josh]

Hey Samuel, thanks for following up. Setting -Dorg.bytedeco.javacpp.nopointergc=true does resolve the sporadic crashes. I wasn't able to spot anything obvious in the verbose logs on runs that failed. So far I haven't been able to reliably recreate the failures outside of our CI environment. I've had to put this on the backburner to work other year-end closeout stuff, but I do need to get to the bottom of it. I'll post back here or on github when I'm finally able to create a minimal example that recreates the problem.

Josh