Problem with JNI

[Elvandar]

Hi all,

I have a problem calling a java method from a DLL written with
JNI...this is the situation: the DLL has some methods to interact with
a RS232 serial port (open it, close it and send a byte array), and when
the port is opened it starts a thread that "listens" to the serial
waiting for incoming data; when some data becomes ready, the thread
calls a java method.
The problem is that the receiving thread can't find the method to call,
so the jmethodID returned from the GetMethodID is always 0...and if I
try to move the GetMethodID instruction in the function
"openSerialPort" it works correctly (the method is found and I can call
it), but if I pass this jmethodID to the thread the entire application
hangs...
Here is the code, hoping someone could help me!!! Sorry for my bad
English
Thanks to all!


=== SerialPort.java ===

package it.ribes.serialDriver;

public class SerialPort {

static {
System.loadLibrary("RS232Driver");
}

public SerialPort() {
}

public native boolean openSerialPort();
public native boolean closeSerialPort();
public native boolean send(byte[] packet, int length);

public boolean openPort() {
return openSerialPort();
}

public boolean closePort() {
return closeSerialPort();
}

public boolean sendPacket(byte[] packet) {
return send(packet,packet.length);
}

public void readPacket(byte[] packet, int dataLength) {
byte[] data = new byte[dataLength];
for(int i = 0; i < dataLength; i++)
data = packet;
this.manager.readPacket(data);
}
}



=== SerialPort.c ===

#include "SerialPort.h"
#include "Windows.h"
#include "Serial.h"
#include <process.h>

#define SERIAL_WAIT_TIMEOUT 250
#define REC_BUFFER_DIM 1024

HANDLE CommHandle = INVALID_HANDLE_VALUE;
HANDLE RecThread = NULL;
BOOL ExitRecThread;

struct event_info_struct
{
JNIEnv *env;
jobject *jobj;
jclass jclazz;
jmethodID send_event;
jmethodID checkMonitorThread;
};

void Receiving_Function (void* param);

JNIEXPORT jboolean JNICALL
Java_it_ribes_serialDriver_SerialPort_openSerialPort (JNIEnv *env,
jobject jobj)
{
CommHandle = SerialInit("COM1",19200);
if(CommHandle == INVALID_HANDLE_VALUE)
return false;
else
{
struct event_info_struct eis;
eis.jclazz = env->GetObjectClass(jobj);
eis.env = env;
eis.jobj = &jobj;
ExitRecThread = FALSE;
RecThread =
(HANDLE)_beginthread(Receiving_Function,1024,(void*)&eis);
return true;
}
}

JNIEXPORT jboolean JNICALL
Java_it_ribes_serialDriver_SerialPort_closeSerialPort (JNIEnv *env,
jobject jobj)
{
ExitRecThread = TRUE;
if(CommHandle == INVALID_HANDLE_VALUE || ClosePort(CommHandle))
{
CommHandle = INVALID_HANDLE_VALUE;
return true;
}
return false;
}

JNIEXPORT jboolean JNICALL Java_it_ribes_serialDriver_SerialPort_send
(JNIEnv *env, jobject jobj, jbyteArray packet, jint length)
{
jbyte* rawJavaData = (jbyte*)malloc(length*sizeof(jbyte));
LPBYTE rawData = (LPBYTE)malloc(length*sizeof(BYTE));
int i = 0, ret;

rawJavaData = env->GetByteArrayElements(packet,NULL);

for(; i < length; i++)
rawData = rawJavaData;

ret = SerialWriteStream(CommHandle,rawData,length);

free(rawData);
free(rawJavaData);

if(ret)
return true;
else
return false;
}

void Receiving_Function (void* param)
{
struct event_info_struct *eis = ( struct event_info_struct * )
param;
LPBYTE RecBuffer = (LPBYTE)malloc(REC_BUFFER_DIM * sizeof(BYTE));
BYTE b;
WORD Dim = 0;
DWORD dwTimeout;

while(!ExitRecThread)
{
dwTimeout = GetTickCount () + SERIAL_WAIT_TIMEOUT;
while (!SerialReadChar (CommHandle, &b))
{
if (GetTickCount () > dwTimeout)
{
if(Dim > 0 && !ExitRecThread)
{
SerialWriteString(CommHandle,"passing data to
java\r\n");
jmethodID mid =
eis->env->GetMethodID(eis->jclazz,"readPacket","([BI)V");
if(mid != 0) // NEVER ENTERS HERE!!!
{
SerialWriteString(CommHandle,"Found!\r\n");
eis->env->CallVoidMethod(*(eis->jobj), mid,
RecBuffer, Dim);
}
else
{
SerialWriteString(CommHandle,"Not found\r\n");
}
Dim = 0;
}
}
}
if(!ExitRecThread)
RecBuffer[Dim++] = b;
}
free(RecBuffer);
_endthread();
}

 

[Chris Uppal]

void Receiving_Function (void* param)
{ [...]
jmethodID mid =
eis->env->GetMethodID(eis->jclazz,"readPacket","([BI)V");

You seem to be using a JNIEnv pointer from a thread other that the one it
belongs to, and doing the same with the local reference, "jclazz". You are not
allowed to do either of those things.

See this (recent!) thread for some more details, and some recommendations for
stuff to read.

<http://groups.google.co.uk/group/comp.lang.java.programmer/tree/browse_frm/thr
ead/c5b61c546b1ed073/4192a51b5ac7bcf0>

-- chris

 

[Gordon Beaton]

The problem is that the receiving thread can't find the method to
call, so the jmethodID returned from the GetMethodID is always
0...and if I try to move the GetMethodID instruction in the function
"openSerialPort" it works correctly (the method is found and I can
call it), but if I pass this jmethodID to the thread the entire
application hangs...

It is important to understand that the JNIEnv pointer is specific to
the thread that it was originally created for. You should never cache
the JNIEnv anywhere, and absolutely never pass it to a different
thread. Your receiving thread should use AttachCurrentThread() to get
its own JNIEnv.

Similarly, you should create global references from the "jobj" and its
class before passing them to the receiving thread. As written, these
are local references that may become invalid after you return from the
initial JNI call. Make sure you free any such global references when
the receiving thread has finished.

In your send() method, you have a serious memory leak and likely
corruption of the JVM. First, you allocate a buffer and assign it to
rawJavaData, but immediately leak the pointer when you reassign
rawJavaData with the result of GetByteArrayElements(). Then you free
*that* pointer, instead of calling ReleaseByteArrayElements() as you
should. This will leave your JVM in an unstable state, and I would
expect it to crash eventually.

Your long-running receiving thread should explicitly free every object
reference it creates, or use PushLocalFrame()/PopLocalFrame() at
appropriate places in the code.

Also, it isn't only unnecessary to cast the return type of malloc(),
it's bad style.

/gordon

 

[Elvandar]

Gordon Beaton laid this down on his screen :

First of all, thanks to you and also to Chris for your answers, they
were very important to me. I just started programming with JNI and I
still have to understand all its mechanisms...

It is important to understand that the JNIEnv pointer is specific to
the thread that it was originally created for. You should never cache
the JNIEnv anywhere, and absolutely never pass it to a different
thread. Your receiving thread should use AttachCurrentThread() to get
its own JNIEnv.

I thought I could pass them to other threads in the way I showed, but I
was wrong With AttachCurrentThread() all works!

Similarly, you should create global references from the "jobj" and its
class before passing them to the receiving thread. As written, these
are local references that may become invalid after you return from the
initial JNI call. Make sure you free any such global references when
the receiving thread has finished.

Same as before, I used the NewGlobalRef() as suggested in the thread
posted by Chris and now all goes well

In your send() method, you have a serious memory leak and likely
corruption of the JVM. First, you allocate a buffer and assign it to
rawJavaData, but immediately leak the pointer when you reassign
rawJavaData with the result of GetByteArrayElements(). Then you free
*that* pointer, instead of calling ReleaseByteArrayElements() as you
should. This will leave your JVM in an unstable state, and I would
expect it to crash eventually.

Your long-running receiving thread should explicitly free every object
reference it creates, or use PushLocalFrame()/PopLocalFrame() at
appropriate places in the code.

Thanks for these hints. The library for now is only a prototype, my
objective is only to see if I can send and receive data on a RS232 with
full interaction with java and then show results to the boss...we have
an app using JavaComm to interact with the RS232, but we discovered a
memory leak, and cannot pass to RXTX library because it doesn't work
with some gsm modems, so I have to check if it's possible to write our
own driver... :'(
Just a question...you said to use ReleaseByteArrayElements() to release
my rawJavaData array, which is a jbyte *...but I saw that jbyte ==
signed char, so I thought using the free() function was the same (as I
already do with the rawData array)...am I wrong?

Also, it isn't only unnecessary to cast the return type of malloc(),
it's bad style.

If I don't do that the compiler shows me an error... "cannot convert
from 'void *' to 'unsigned char *' Conversion from 'void*' to pointer
to non-'void' requires an explicit cast" (I tried eliminating the
LPBYTE cast in the rawData array initialization in the send
function)...

Thanks for all.

 

[Chris Uppal]

Also, it isn't only unnecessary to cast the return type of malloc(),
it's bad style.

Quibble: not if you're writing in C++ it isn't. At least if I remember my C++
correctly, that language does not consider void* to be assignment compatible
with <pointer to xxx> in the way that C does.

-- chris

 

[Gordon Beaton]

Quibble: not if you're writing in C++ it isn't.

Mea culpa - I didn't look close enough at the code, saw malloc() and
thought "C".

/gordon

 

[Gordon Beaton]

I just started programming with JNI and I still have to understand
all its mechanisms...

In addition to the JNI spec and FAQ, this online book might help:

http://java.sun.com/docs/books/jni/

Just a question...you said to use ReleaseByteArrayElements() to
release my rawJavaData array, which is a jbyte *...but I saw that
jbyte == signed char, so I thought using the free() function was the
same (as I already do with the rawData array)...am I wrong?

The difference is that you don't know how GetByteArrayElements()
allocated this resource for you. It might be part of a larger struct,
or a pointer directly into the Java array object. The JVM expects to
manage it itself, and by calling free() you are messing with that.

If I don't do that the compiler shows me an error...

Sorry, I was thinking in C, not C++.

/gordon

 

[Elvandar]

Gordon Beaton laid this down on his screen :

The difference is that you don't know how GetByteArrayElements()
allocated this resource for you. It might be part of a larger struct,
or a pointer directly into the Java array object. The JVM expects to
manage it itself, and by calling free() you are messing with that.

Ok, now it's clearer . Thanks for your help!