[tcljava-dev] Request for eyes, can anyone figure out what this code is for?

[Mo D. <md...@un...>]

Hello all

While looking at the JNI code for Tcl Blend, I ran into a very strange
thing in SetJavaCmdFromAny in javaObj.c. As far as I can tell, there
is no reason to handle a Tcl_Obj of type tclObjectType from one that
has a cmdTypePtr with a non-NULL ptr2. The existing code
will increment the TclObject.refCount and create a new global
JNI ref and then let it get deleted while freeing up the old object
internal rep. I just can't figure out for the life of me why this is
being done. I decided to remove the code to see if that broke anything,
and it did not cause any failures in the test suite.

I feel like I might just be missing something, so I was wondering if
someone else could take a look at this issue and see what you come
up with. I am planning on applying the appended patch.

cheers
Mo



Here is a test case:

package require java
append env(TCL_CLASSPATH) .

namespace eval foo {
    proc test {obj} {
        $obj nothing
    }
}

set obj [java::new JT]

$obj nothing

foo::test $obj



% cat JT.java 
import tcl.lang.*;

public class JT {
    public void nothing() {}
    public String toString() {return "JT";}
}





And the patch for javaObj.c:

Index: src/native/javaObj.c
===================================================================
RCS file: /cvsroot/tcljava/tcljava/src/native/javaObj.c,v
retrieving revision 1.13
diff -u -r1.13 javaObj.c
--- src/native/javaObj.c	21 Dec 2002 04:02:53 -0000	1.13
+++ src/native/javaObj.c	23 Dec 2002 01:34:26 -0000
@@ -873,45 +875,22 @@
 
     /*
      * Invoke the normal command type routine, but make sure
-     * it doesn't free the java object.  Note that we have to
-     * restore the ptr2 value after the conversion, since it gets
-     * set to NULL by the setFromAnyProc.
+     * it doesn't free the java object by setting the typePtr
+     * to NULL. Note that we have to restore the ptr2 value after
+     * the conversion, since it gets set to NULL by setFromAnyProc.
      */
 
-    if (objPtr->typePtr == &tclObjectType) {
+    if ((objPtr->typePtr == &tclObjectType) ||
+	    ((objPtr->typePtr == cmdTypePtr) &&
+	    (objPtr->internalRep.twoPtrValue.ptr2 != NULL))) {
 	VOID *ptr2;
 	if (objPtr->bytes == NULL) {
-	    UpdateTclObject(objPtr);	
+	    UpdateTclObject(objPtr);
 	}
 	objPtr->typePtr = NULL;
 	ptr2 = objPtr->internalRep.twoPtrValue.ptr2;
 	result = (oldCmdType.setFromAnyProc)(interp, objPtr);
 	objPtr->internalRep.twoPtrValue.ptr2 = ptr2;
-    } else if ((objPtr->typePtr == cmdTypePtr)
-	    && (objPtr->internalRep.twoPtrValue.ptr2 != NULL)) {
-	jobject object = (jobject)(objPtr->internalRep.twoPtrValue.ptr2);
-	JNIEnv *env = JavaGetEnv();
-	JavaInfo* jcache = JavaGetCache();
-
-	/*
-	 * If we are converting from something that is already a java command
-	 * reference we need to preserve the object handle so that it doesn't
-	 * get freed as a side effect of updating the command cache.  Note
-	 * that the new command may not refer to a java object, but we will
-	 * maintain the extra reference anyway since it is difficult to
-	 * detect this case.  The object reference will still be cleaned up
-	 * when the Tcl object is free.
-	 */
-
-	object = (*env)->NewGlobalRef(env, object);
-	(*env)->CallVoidMethod(env, object, jcache->preserve);
-	result = (oldCmdType.setFromAnyProc)(interp, objPtr);
-	if (result != TCL_OK) {
-	    (*env)->CallVoidMethod(env, object, jcache->release);
-	    (*env)->DeleteGlobalRef(env, object);
-	} else {
-	    objPtr->internalRep.twoPtrValue.ptr2 = (VOID*) object;
-	}
     } else {
 	result = (oldCmdType.setFromAnyProc)(interp, objPtr);
 
@@ -919,9 +898,8 @@
 	 * Ensure that the ptr2 is null for non-java commands.
 	 */
 
-	if (result == TCL_OK) {
-	    objPtr->internalRep.twoPtrValue.ptr2 = NULL;
-	}
+	if (objPtr->internalRep.twoPtrValue.ptr2 != NULL)
+	    panic("post setFromAnyProc, ptr2 is not NULL");
     }
     return result;
 }