I have attached a patch with the rewrite of JavaGetEnv() and
the threads cleanup. I like this approach a lot better
because now there is only one synchronization point,
inside the JavaSetupJava function. I think this will
take care of all of the possible race conditions
during initilization. The old JavaGetEnv method is
now called JavaInitEnv.
Jiang, what do you think? I will check it into the
contract branch if you give me the thumbs up on
this patch.
Mo DeJong
Red Hat Inc
Index: src/native/java.h
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/java.h,v
retrieving revision 1.6.2.2
diff -u -r1.6.2.2 java.h
--- java.h 2000/08/07 00:50:09 1.6.2.2
+++ java.h 2000/08/08 07:17:41
@@ -140,7 +140,7 @@
TCLBLEND_EXTERN void JavaAlertNotifier();
TCLBLEND_EXTERN void JavaDisposeNotifier();
-TCLBLEND_EXTERN JNIEnv * JavaGetEnv(Tcl_Interp *interp);
+TCLBLEND_EXTERN JNIEnv * JavaGetEnv();
TCLBLEND_EXTERN Tcl_Interp * JavaGetInterp(JNIEnv *env, jobject interpObj);
TCLBLEND_EXTERN char * JavaGetString(JNIEnv *env, jstring str,
int *lengthPtr);
Index: src/native/javaCmd.c
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/javaCmd.c,v
retrieving revision 1.9.2.3
diff -u -r1.9.2.3 javaCmd.c
--- javaCmd.c 2000/08/07 08:50:16 1.9.2.3
+++ javaCmd.c 2000/08/08 07:17:42
@@ -10,7 +10,7 @@
* of this file, and for a DISCLAIMER OF ALL WARRANTIES.
*
*
- * RCS: @(#) $Id: javaCmd.c,v 1.9.2.3 2000/08/07 08:50:16 mo Exp $
+ * RCS: @(#) $Id: javaCmd.c,v 1.9.2.3 2000/08/07 00:50:09 mo Exp $
*/
/*
@@ -46,6 +46,7 @@
#include <stdlib.h>
#include <stdarg.h>
#include <errno.h>
+#include <assert.h>
/*
* Exported state variables.
@@ -94,6 +95,20 @@
static int initialized_javaVM = 0;
/*
+ * Declare a global mutex to protect the creation and initialization of the
+ * JVM from mutiple threads. This mutex is used in conjunction with the
+ * 'initialized_javaVM' flag. This mutex is used in javaCmd.c as well as
+ * javaInterp.c.
+ *
+ * FIXME: don't want to use the flag TCL_THREADS explicitly. This may be
+ * better if in the future the same TclBlend binary can be made to work with
+ * both threaded and non-threaded Tcl libraries. For now, we will use accessor
+ * functions lockJVMInitMutex() and unlockJVMInitMutex().
+ */
+TCL_DECLARE_MUTEX(javaVM_init_mutex)
+
+
+/*
* The following array contains the class names and jclass pointers for
* all of the classes used by this module. It is used to initialize
* the java structure's jclass slots.
@@ -179,6 +194,7 @@
*/
static int ToString(JNIEnv *env, Tcl_Obj *objPtr, jobject obj);
+static JNIEnv * JavaInitEnv(JNIEnv *env, Tcl_Interp *interp);
/*
*----------------------------------------------------------------------
@@ -242,24 +258,11 @@
#endif /* TCLBLEND_DEBUG */
/*
- * The first time JavaGetEnv is called, it will create a JVM.
- * Later calls will just attach the current thread to the JVM.
+ * Init the JVM, the JNIEnv pointer, and any global data. Pass a
+ * NULL JNIEnv pointer to indicate Tcl Blend is being loaded from Tcl.
*/
-
- if ((env = JavaGetEnv(interp)) == NULL) {
-
-#ifdef TCLBLEND_DEBUG
- fprintf(stderr, "TCLBLEND_DEBUG: JavaGetEnv returned NULL\n");
-#endif /* TCLBLEND_DEBUG */
-
- return TCL_ERROR;
- }
- /*
- * Make sure the global VM information is properly intialized.
- */
-
- if (JavaSetupJava(env, interp) != TCL_OK) {
+ if (JavaSetupJava(NULL, interp) != TCL_OK) {
return TCL_ERROR;
}
@@ -267,6 +270,7 @@
* Allocate a new Interp instance to wrap this interpreter.
*/
+ env = JavaGetEnv();
*(Tcl_Interp**)&lvalue = interp;
local = (*env)->NewObject(env, java.Interp,
java.interpC, lvalue);
@@ -349,28 +353,59 @@
vm_args.classpath, NULL);
#endif
}
+
/*
*----------------------------------------------------------------------
*
* JavaGetEnv --
*
- * Retrieve the JNI environment for the current thread.
+ * Retrieve the JNI environment for the current thread. This method
+ * is concurrent safe.
*
* Results:
- * Returns the JNIEnv pointer for the current thread. Returns
- * NULL on error with a message left in the interpreter result.
+ * Returns the JNIEnv pointer for the current thread.
+ * This method must be called after JavaInitEnv has been called.
*
* Side effects:
- * May create or attach to a new JavaVM if this is the first time
- * the JNIEnv is fetched from outside of a Java callback.
*
*----------------------------------------------------------------------
*/
TCLBLEND_EXTERN JNIEnv *
-JavaGetEnv(
- Tcl_Interp *interp) /* Interp for error reporting. */
+JavaGetEnv()
{
+ ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
+
+ assert(tsdPtr->initialized_currentEnv);
+
+ return tsdPtr->currentEnv;
+}
+
+/*
+ *----------------------------------------------------------------------
+ *
+ * JavaInitEnv --
+ *
+ * Init the JNIEnv for this thread.
+ *
+ * Results:
+ * Returns the JNIEnv pointer for the current thread. Returns
+ * NULL on error with a message left in the interpreter result.
+ *
+ * Side effects:
+ * If Tcl Blend is loaded into Tcl and this is the first thread
+ * to load Tcl Blend, a new JVM will be created. If another
+ * Tcl thread loads Tcl Blend, that thread will be attached to
+ * the existing JVM.
+ *----------------------------------------------------------------------
+ */
+
+JNIEnv *
+JavaInitEnv(
+ JNIEnv *env, /* JNIEnv pointer, NULL if loaded from Tcl Blend */
+ Tcl_Interp *interp /* Interp for error reporting. */
+)
+{
jsize nVMs;
char *path, *newPath;
int oldSize, size, maxOptions = 2;
@@ -385,14 +420,8 @@
ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
- if (tsdPtr->initialized_currentEnv) {
- return tsdPtr->currentEnv;
- }
-
- /* FIXME : we need to put a mutex around this create JVM/attach step */
-
#ifdef TCLBLEND_DEBUG
- fprintf(stderr, "TCLBLEND_DEBUG: JavaGetEnv for %s JVM\n",
+ fprintf(stderr, "TCLBLEND_DEBUG: JavaInitEnv for %s JVM\n",
#ifdef JDK1_2
"JDK1_2"
#elif defined TCLBLEND_KAFFE
@@ -404,10 +433,34 @@
#endif /* TCLBLEND_DEBUG */
/*
- * Check to see if the current process already has a Java VM. If so,
- * attach to it, otherwise create a new one.
+ * If init was already called for this thread, do nothing.
+ * This can happen if multiple interpreters are created in
+ * the same thread.
*/
+ if (tsdPtr->initialized_currentEnv) {
+ return tsdPtr->currentEnv;
+ }
+
+ /*
+ * If we were called with a non-NULL JNIEnv argument, it means
+ * Tcl Blend was loaded from Java. In this case, the JNIEnv is
+ * already attached to the JVM because it was created in Java.
+ * Since we do not need to create a JVM and we do not need to
+ * attach the current thread, we just set currentEnv and return.
+ */
+
+ if (env) {
+ tsdPtr->initialized_currentEnv = 1;
+ return (tsdPtr->currentEnv = env);
+ }
+
+ /*
+ * From this point on, deal with the case where Tcl Blend is loaded from Tcl.
+ * Check to see if the current process already has a Java VM. If so, attach
+ * the current thread to it, otherwise create a new JVM (automatic thread
+attach).
+ */
+
if (JNI_GetCreatedJavaVMs(&javaVM, 1, &nVMs) < 0) {
#ifdef TCLBLEND_DEBUG
@@ -415,7 +468,7 @@
#endif /* TCLBLEND_DEBUG */
Tcl_AppendResult(interp, "JNI_GetCreatedJavaVMs failed", NULL);
- return NULL;
+ goto error;
}
if (nVMs == 0) {
@@ -523,7 +576,7 @@
"Tcl Blend only: If the value is 'help', then JVM initialization stop\n",
"and this message is returned.",
NULL);
- return NULL;
+ goto error;
}
options[vm_args.nOptions].extraInfo = (void *)NULL;
vm_args.nOptions++;
@@ -568,7 +621,7 @@
"than the one Tcl Blend was compiled with?\n",
NULL);
appendClasspathMessage(interp);
- return NULL;
+ goto error;
}
} else {
@@ -589,12 +642,24 @@
#endif /* TCLBLEND_DEBUG */
Tcl_AppendResult(interp, "AttachCurrentThread failed", NULL);
- return NULL;
+ goto error;
}
}
+#ifdef TCLBLEND_DEBUG
+ fprintf(stderr, "TCLBLEND_DEBUG: JavaInitEnv returning successfully\n");
+#endif /* TCLBLEND_DEBUG */
+
tsdPtr->initialized_currentEnv = 1;
return tsdPtr->currentEnv;
+
+ error:
+
+#ifdef TCLBLEND_DEBUG
+ fprintf(stderr, "TCLBLEND_DEBUG: JavaInitEnv returning NULL\n");
+#endif /* TCLBLEND_DEBUG */
+
+ return NULL;
}
/*
@@ -688,7 +753,7 @@
Tcl_Interp *interp) /* Interpreter being deleted. */
{
jobject interpObj = (jobject) clientData;
- JNIEnv *env = JavaGetEnv(interp);
+ JNIEnv *env = JavaGetEnv();
/*
* Set the Interp.interpPtr field to 0 so any further attempts to use
@@ -705,7 +770,7 @@
(*env)->CallVoidMethod(env, interpObj, java.dispose);
(*env)->DeleteGlobalRef(env, interpObj);
- /* FIXME : detach the JNIEnv */
+ /* FIXME : detach the JNIEnv, but only if this is the last interp in the thread
+??? */
#ifdef TCLBLEND_DEBUG
fprintf(stderr, "TCLBLEND_DEBUG: called JavaInterpDeleted\n");
@@ -719,7 +784,11 @@
*
* JavaSetupJava --
*
- * Set up the cache of class and method ids.
+ * This is the entry point for a Tcl interpreter created from Java.
+ * This method will save the JVM JNIEnv pointer by calling JavaInitEnv
+ * if this was the first time JavaSetupJava was called for the current
+ * thread. It will also set up the cache of class and method ids if this
+ * was the first time JavaSetupJava was called in this process.
*
* Results:
* A standard Tcl result.
@@ -739,15 +808,33 @@
jfieldID field;
int i;
-
#ifdef TCLBLEND_DEBUG
fprintf(stderr, "TCLBLEND_DEBUG: called JavaSetupJava\n");
#endif /* TCLBLEND_DEBUG */
+ /*
+ * Acquire the init lock, we do not care if it is slow to call
+ * JavaSetupJava, it is only called when an Interp is created.
+ */
+
+ Tcl_MutexLock(&javaVM_init_mutex);
+
+ /*
+ * Check that the JNIEnv for this thread has been initialized.
+ * If Tcl Blend is getting loaded from Tcl, then the env argument
+ * would be passed as NULL.
+ */
+
+ if ((env = JavaInitEnv(env, interp)) == NULL) {
+ goto error;
+ }
+
+ /*
+ * If the global java struct was already initialized, just return
+ */
- /* FIXME : we need to put a mutex around this test/set */
if (initialized_javaVM) {
- return TCL_OK;
+ goto ok;
}
memset(&java, 0, sizeof(java));
@@ -766,7 +853,8 @@
(*env)->ExceptionDescribe(env);
obj = Tcl_GetObjResult(interp);
(*env)->ExceptionClear(env);
- /* We can't call ToString() here, we might not have
+ /*
+ * We can't call ToString() here, we might not have
* java.toString yet.
*/
(*env)->Throw(env, exception);
@@ -858,9 +946,21 @@
JavaObjInit();
initialized_javaVM = 1;
+
+ ok:
+#ifdef TCLBLEND_DEBUG
+ fprintf(stderr, "TCLBLEND_DEBUG: JavaSetupJava returning successfully\n");
+#endif /* TCLBLEND_DEBUG */
+
+ Tcl_MutexUnlock(&javaVM_init_mutex);
return TCL_OK;
error:
+#ifdef TCLBLEND_DEBUG
+ fprintf(stderr, "TCLBLEND_DEBUG: JavaSetupJava returning TCL_ERROR\n");
+#endif /* TCLBLEND_DEBUG */
+
+ Tcl_MutexUnlock(&javaVM_init_mutex);
(*env)->ExceptionClear(env);
return TCL_ERROR;
}
Index: src/native/javaIdle.c
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/javaIdle.c,v
retrieving revision 1.2.2.1
diff -u -r1.2.2.1 javaIdle.c
--- javaIdle.c 2000/07/30 07:17:09 1.2.2.1
+++ javaIdle.c 2000/08/08 07:17:42
@@ -118,7 +118,7 @@
JavaIdleProc(
ClientData clientData) /* Global IdleHandler reference */
{
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
jobject exception;
jobject idle = (jobject) clientData;
Index: src/native/javaInterp.c
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/javaInterp.c,v
retrieving revision 1.7.2.2
diff -u -r1.7.2.2 javaInterp.c
--- javaInterp.c 2000/08/07 00:50:09 1.7.2.2
+++ javaInterp.c 2000/08/08 07:17:43
@@ -823,7 +823,7 @@
jstring name1Str, name2Str;
jobject exception, interpObj;
Tcl_SavedResult state;
- JNIEnv *env = JavaGetEnv(interp);
+ JNIEnv *env = JavaGetEnv();
result = NULL;
if (tPtr->errMsg != NULL) {
@@ -953,7 +953,7 @@
ClientData clientData)
{
jobject cmd = (jobject)clientData;
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
if ((*env)->IsInstanceOf(env, cmd, java.CommandWithDispose)) {
(*env)->CallVoidMethod(env, cmd, java.disposeCmd);
@@ -990,7 +990,7 @@
jarray args;
jobject value, exception, interpObj;
int i, result;
- JNIEnv *env = JavaGetEnv(interp);
+ JNIEnv *env = JavaGetEnv();
interpObj = (jobject) Tcl_GetAssocData(interp, "java", NULL);
Index: src/native/javaNotifier.c
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/javaNotifier.c,v
retrieving revision 1.2.2.1
diff -u -r1.2.2.1 javaNotifier.c
--- javaNotifier.c 2000/07/30 07:17:09 1.2.2.1
+++ javaNotifier.c 2000/08/08 07:17:43
@@ -171,7 +171,7 @@
ClientData data, /* Not used. */
int flags) /* Same as for Tcl_DoOneEvent. */
{
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
if ((*env)->CallBooleanMethod(env, globalNotifierObj, java.hasEvents)
== JNI_TRUE) {
@@ -202,7 +202,7 @@
ClientData data, /* Not used. */
int flags) /* Same as for Tcl_DoOneEvent. */
{
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
Tcl_Event *ePtr;
/*
@@ -241,7 +241,7 @@
Tcl_Event *evPtr, /* The event that is being processed. */
int flags) /* The flags passed to Tcl_ServiceEvent. */
{
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
/*
* Call Notifier.serviceEvent() to handle invoking the next event and
Index: src/native/javaObj.c
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/javaObj.c,v
retrieving revision 1.4.2.1
diff -u -r1.4.2.1 javaObj.c
--- javaObj.c 2000/07/30 07:17:09 1.4.2.1
+++ javaObj.c 2000/08/08 07:17:43
@@ -128,7 +128,7 @@
Tcl_Obj *destPtr)
{
jobject object = (jobject)(srcPtr->internalRep.twoPtrValue.ptr2);
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
/*
* Add a global reference to represent the new copy.
@@ -162,7 +162,7 @@
Tcl_Obj *objPtr) /* Object to free. */
{
jobject object = (jobject)(objPtr->internalRep.twoPtrValue.ptr2);
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
(*env)->CallVoidMethod(env, object, java.release);
(*env)->DeleteGlobalRef(env, object);
@@ -218,7 +218,7 @@
{
jstring string;
jobject object = (jobject)(objPtr->internalRep.twoPtrValue.ptr2);
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
string = (*env)->CallObjectMethod(env, object, java.toString);
objPtr->bytes = JavaGetString(env, string, &objPtr->length);
@@ -704,7 +704,7 @@
} else if ((objPtr->typePtr == cmdTypePtr)
&& (objPtr->internalRep.twoPtrValue.ptr2 != NULL)) {
jobject object = (jobject)(objPtr->internalRep.twoPtrValue.ptr2);
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
/*
* If we are converting from something that is already a java command
Index: src/native/javaTimer.c
===================================================================
RCS file: /home/cvs/external/tcljava/src/native/javaTimer.c,v
retrieving revision 1.2.2.1
diff -u -r1.2.2.1 javaTimer.c
--- javaTimer.c 2000/07/30 07:17:09 1.2.2.1
+++ javaTimer.c 2000/08/08 07:17:43
@@ -114,7 +114,7 @@
ClientData clientData) /* Pointer to TimerInfo. */
{
TimerInfo *infoPtr = (TimerInfo *) clientData;
- JNIEnv *env = JavaGetEnv(NULL);
+ JNIEnv *env = JavaGetEnv();
jobject exception;
/*
