diff options
3 files changed, 66 insertions, 21 deletions
diff --git a/src/java/com/jogamp/gluegen/CMethodBindingEmitter.java b/src/java/com/jogamp/gluegen/CMethodBindingEmitter.java index 9be4a36..b2b9cbd 100644 --- a/src/java/com/jogamp/gluegen/CMethodBindingEmitter.java +++ b/src/java/com/jogamp/gluegen/CMethodBindingEmitter.java @@ -330,17 +330,20 @@ public class CMethodBindingEmitter extends FunctionEmitter { protected void emitReturnType() { if( null != javaCallback ) { LOG.log(INFO, "BindCFunc.R.JavaCallback: {0}: {1}", binding.getName(), javaCallback); + final String staticCallbackName = "func"+jcbNativeBasename; + final Type userParamType = javaCallback.cbFuncBinding.getCArgumentType(javaCallback.cbFuncUserParamIdx); final String userParamArgName = javaCallback.cbFuncBinding.getArgumentName(javaCallback.cbFuncUserParamIdx); final Type cReturnType = javaCallback.cbFuncBinding.getCReturnType(); final JavaType jretType = javaCallback.cbFuncBinding.getJavaReturnType(); unit.emitln("typedef struct {"); + unit.emitln(" jobject lockObj;"); unit.emitln(" jobject cbFunc;"); unit.emitln(" jmethodID cbMethodID;"); unit.emitln(" jobject userParam;"); unit.emitln("} T_"+jcbNativeBasename+";"); unit.emitln(); // javaCallback.cbFuncCEmitter.emitSignature(); - unit.emit("static "+cReturnType.getCName()+" func"+jcbNativeBasename+"("); + unit.emit("static "+cReturnType.getCName()+" "+staticCallbackName+"("); // javaCallback.cbFuncCEmitter.emitArguments(); unit.emit(javaCallback.cbFuncBinding.getCParameterList(new StringBuilder(), false, null).toString()); unit.emitln(") {"); @@ -364,22 +367,49 @@ public class CMethodBindingEmitter extends FunctionEmitter { unit.emitln(" // C Params: "+javaCallback.cbFuncBinding.getCParameterList(new StringBuilder(), false, null).toString()); unit.emitln(" // J Params: "+javaCallback.cbFuncBinding.getJavaParameterList(new StringBuilder()).toString()); + final String returnStatement; if( !cReturnType.isVoid() ) { - unit.emit(" "+cReturnType.getCName()+" _res = ("+cReturnType.getCName()+") "); + unit.emit(" "+cReturnType.getCName()+" _res = 0;"); + returnStatement = "return _res;"; + } else { + returnStatement = "return;"; + } + unit.emitln(" if( NULL == cb ) { fprintf(stderr, \"Info: Callback '"+staticCallbackName+"(..)': NULL "+userParamArgName+", skipping!\\n\"); "+returnStatement+" }"); + unit.emitln(" T_"+jcbNativeBasename+" cb2 = *cb; // use a copy to avoid data-race between GetObjectRefType() and MonitorEnter()"); + unit.emitln(); + unit.emitln(" jobjectRefType refType = (*env)->GetObjectRefType(env, cb2.lockObj);"); + unit.emitln(" if( 0 == refType ) { fprintf(stderr, \"Info: Callback '"+staticCallbackName+"(..)': User after free(lock), skipping!\\n\"); "+returnStatement+" }"); + unit.emitln(" jint lockRes = (*env)->MonitorEnter(env, cb2.lockObj);"); + unit.emitln(" if( 0 != lockRes ) { fprintf(stderr, \"Info: Callback '"+staticCallbackName+"(..)': MonitorEnter failed %d, skipping!\\n\", lockRes); "+returnStatement+" }"); + unit.emitln(" // synchronized block"); + /** + * Since we have acquired the lock, in-sync w/ our Java code, cb2.cbFunc and cb2.userParam could not have been changed! + * + unit.emitln(" refType = (*env)->GetObjectRefType(env, cb2.userParam);"); + unit.emitln(" if( 0 == refType ) {"); + unit.emitln(" fprintf(stderr, \"Info: Callback '"+staticCallbackName+"(..)': User after free(userParam), skipping!\\n\");"); + unit.emitln(" lockRes = (*env)->MonitorExit(env, cb2.lockObj);"); + unit.emitln(" if( 0 != lockRes ) { fprintf(stderr, \"Info: Callback '"+staticCallbackName+"(..)': MonitorExit failed %d\\n\", lockRes); }"); + unit.emitln(" "+returnStatement); + unit.emitln(" }"); + */ + if( !cReturnType.isVoid() ) { + unit.emit(" _res = ("+cReturnType.getCName()+") "); } else { unit.emit(" "); } - unit.emit("(*env)->Call" + CodeGenUtils.capitalizeString( jretType.getName() ) +"Method(env, cb->cbFunc, cb->cbMethodID, "); + unit.emit("(*env)->Call" + CodeGenUtils.capitalizeString( jretType.getName() ) +"Method(env, cb2.cbFunc, cb2.cbMethodID, "); // javaCallback.cbFuncCEmitter.emitBodyPassCArguments(); - jcbCMethodEmitter.emitJavaCallbackBodyPassJavaArguments(javaCallback, "cb->userParam"); + jcbCMethodEmitter.emitJavaCallbackBodyPassJavaArguments(javaCallback, "cb2.userParam"); unit.emitln(");"); // javaCallback.cbFuncCEmitter.emitBodyUserVariableAssignments(); // javaCallback.cbFuncCEmitter.emitBodyVariablePostCallCleanup(); // javaCallback.cbFuncCEmitter.emitBodyMapCToJNIType(-1 /* return value */, true /* addLocalVar */) - if( !cReturnType.isVoid() ) { - unit.emitln(" return _res;"); - } + + unit.emitln(" lockRes = (*env)->MonitorExit(env, cb2.lockObj);"); + unit.emitln(" if( 0 != lockRes ) { fprintf(stderr, \"Info: Callback '"+staticCallbackName+"(..)': MonitorExit failed %d\\n\", lockRes); }"); + unit.emitln(" "+returnStatement); } unit.emitln("}"); unit.emitln(); @@ -495,7 +525,7 @@ public class CMethodBindingEmitter extends FunctionEmitter { final JavaCallbackInfo jcb = this.javaCallback; if( null != jcb ) { LOG.log(INFO, "BindCFunc.A.JavaCallback: {0}: {1}", binding.getName(), jcb); - unit.emit(", jstring jcallbackSignature, jlongArray jnativeUserParam"); + unit.emit(", jstring jcallbackSignature, jobject jlockObj, jlongArray jnativeUserParam"); numEmitted+=2; } else { LOG.log(INFO, "BindCFunc.JavaCallback: {0}: NONE", binding.getName()); @@ -522,7 +552,8 @@ public class CMethodBindingEmitter extends FunctionEmitter { unit.emitln(" // JavaCallback handling"); unit.emitln(" "+jcb.cbFuncTypeName+" "+nativeCBFuncVarName+";"); unit.emitln(" T_"+jcbNativeBasename+"* "+nativeUserParamVarName+";"); - // unit.emit(", jstring jcallbackSignature, jlongArray jnativeUserParam"); + // unit.emit(", jstring jcallbackSignature, jobject jlockObj, jlongArray jnativeUserParam"); + unit.emitln(" if( NULL == jlockObj ) { (*env)->FatalError(env, \"Null jlockObj in '"+jcbNativeBasename+"'\"); }"); unit.emitln(" if( NULL == jnativeUserParam ) { (*env)->FatalError(env, \"Null jnativeUserParam in '"+jcbNativeBasename+"'\"); }"); unit.emitln(" const size_t jnativeUserParam_size = (*env)->GetArrayLength(env, jnativeUserParam);"); unit.emitln(" if( 1 > jnativeUserParam_size ) { (*env)->FatalError(env, \"nativeUserParam size < 1 in '"+jcbNativeBasename+"'\"); }"); @@ -530,6 +561,8 @@ public class CMethodBindingEmitter extends FunctionEmitter { unit.emitln(" if( NULL == "+userParamArgName+" ) { (*env)->FatalError(env, \"Null "+userParamArgName+" in '"+jcbNativeBasename+"'\"); }"); unit.emitln(" "+nativeUserParamVarName+" = (T_"+jcbNativeBasename+"*) calloc(1, sizeof(T_"+jcbNativeBasename+"));"); unit.emitln(" if( NULL == "+nativeUserParamVarName+" ) { (*env)->FatalError(env, \"Can't alloc "+nativeUserParamVarName+" in '"+jcbNativeBasename+"'\"); }"); + unit.emitln(" "+nativeUserParamVarName+"->lockObj = (*env)->NewGlobalRef(env, jlockObj);"); + unit.emitln(" if( NULL == "+nativeUserParamVarName+"->lockObj ) { (*env)->FatalError(env, \"Failed NewGlobalRef(lock) in '"+jcbNativeBasename+"'\"); }"); unit.emitln(" "+nativeUserParamVarName+"->cbFunc = (*env)->NewGlobalRef(env, "+cbFuncArgName+");"); unit.emitln(" if( NULL == "+nativeUserParamVarName+"->cbFunc ) { (*env)->FatalError(env, \"Failed NewGlobalRef(func) in '"+jcbNativeBasename+"'\"); }"); unit.emitln(" "+nativeUserParamVarName+"->userParam = (*env)->NewGlobalRef(env, "+userParamArgName+");"); @@ -570,8 +603,10 @@ public class CMethodBindingEmitter extends FunctionEmitter { unit.emitln("JNIEXPORT void JNICALL"); unit.emit(JavaEmitter.getJNIMethodNamePrefix(getJavaPackageName(), getJavaClassName())); unit.emitln("_release"+capIfaceName+"Impl(JNIEnv *env, jobject _unused, jlong jnativeUserParam) {"); + unit.emitln(" // already locked"); unit.emitln(" T_"+jcbNativeBasename+"* nativeUserParam = (T_"+jcbNativeBasename+"*) (intptr_t) jnativeUserParam;"); unit.emitln(" if( NULL != nativeUserParam ) {"); + unit.emitln(" (*env)->DeleteGlobalRef(env, nativeUserParam->lockObj);"); unit.emitln(" (*env)->DeleteGlobalRef(env, nativeUserParam->cbFunc);"); unit.emitln(" (*env)->DeleteGlobalRef(env, nativeUserParam->userParam);"); unit.emitln(" free(nativeUserParam);"); diff --git a/src/java/com/jogamp/gluegen/JavaMethodBindingEmitter.java b/src/java/com/jogamp/gluegen/JavaMethodBindingEmitter.java index 1737ba1..6a93973 100644 --- a/src/java/com/jogamp/gluegen/JavaMethodBindingEmitter.java +++ b/src/java/com/jogamp/gluegen/JavaMethodBindingEmitter.java @@ -421,7 +421,7 @@ public class JavaMethodBindingEmitter extends FunctionEmitter { if (needComma) { unit.emit(", "); } - unit.emit("String callbackSignature, long[/*1*/] nativeUserParam"); + unit.emit("String callbackSignature, Object lockObj, long[/*1*/] nativeUserParam"); ++numEmitted; } return numEmitted; @@ -1095,10 +1095,12 @@ public class JavaMethodBindingEmitter extends FunctionEmitter { ++numArgsEmitted; } if( null != javaCallback ) { + final String lowIfaceName = CodeGenUtils.decapitalizeString( getInterfaceName() ); + final String lockInstanceName = lowIfaceName+"Lock"; if (needComma) { unit.emit(", "); } - unit.emit("\"" + javaCallback.cbMethodSignature + "\", nativeUserParam"); + unit.emit("\"" + javaCallback.cbMethodSignature + "\", "+lockInstanceName+", nativeUserParam"); ++numArgsEmitted; } return numArgsEmitted; diff --git a/src/junit/com/jogamp/gluegen/test/junit/generation/Test4JavaCallback.java b/src/junit/com/jogamp/gluegen/test/junit/generation/Test4JavaCallback.java index 54a8c59..9478f04 100644 --- a/src/junit/com/jogamp/gluegen/test/junit/generation/Test4JavaCallback.java +++ b/src/junit/com/jogamp/gluegen/test/junit/generation/Test4JavaCallback.java @@ -735,16 +735,24 @@ public class Test4JavaCallback extends BaseClass { Assert.assertEquals(false, keys.contains(buffer3Key)); } - if( false ){ - bt2.alBufferCallback0Inject(buffer2, 1, 10); // unmapped, no change in data - Assert.assertEquals( 1+ 10+2, id_res[0]); - Assert.assertEquals( 1, myUserParam01.i); - Assert.assertEquals(11+101+1, myUserParam01.j); - Assert.assertEquals( 1, myUserParam01.buffer); - Assert.assertEquals( 2, myUserParam02.i); - Assert.assertEquals( 1+ 10+2, myUserParam02.j); - Assert.assertEquals( 2, myUserParam02.buffer); - } + // The native callback is still registered, + // we 'just' pulled the resource via release*()! + // + // So we no only test no-change in data + // but also whether the native callback handles this case well, + // i.e. detects the released data-resource and *NOT* issuing the java callback. + // The latter would end up in a SIGSEGV otherwise! + // + // Note: After successfully checking a correct jobject reference, + // the native callback also enters and leaves the monitor (Object sync/lock). + bt2.alBufferCallback0Inject(buffer2, 1, 10); + Assert.assertEquals(10*100+2, id_res[0]); + Assert.assertEquals( 1, myUserParam01.i); + Assert.assertEquals(10+100+1, myUserParam01.j); + Assert.assertEquals( 1, myUserParam01.buffer); + Assert.assertEquals( 2, myUserParam02.i); + Assert.assertEquals(10*100+2, myUserParam02.j); + Assert.assertEquals( 2, myUserParam02.buffer); } public static class CustomAlBufferCallback1Key { |