Changed Linux hosted mode code to use JSRuntime* instead of JSContext* for
adding/removing JS roots. This fixes the problem where the JSContext has
been destroyed by the time the Java finalizer runs -- there is one JSRuntime
instance and it always exists.
In the future, the JSContext will not be stored in the JsRootedValue at all,
but will be kept in a stack from gwtOnLoad calls much like the Safari code.
This will also cut in half the storage required for JsRootedValue object.
In addition to this basic change, two additional changes were incorporated:
* an unreferenced "staging" target in the jni/Linux Makefile to ease
development on Linux hosted mode
* the void-valued JsRootedValue constructor did not register itself as
a root. This was missed when we changed from only rooting values that
need to be GC'd, versus always rooting the value regardless of what it
holds. When the value is destroyed, the root would be removed that was
never added, which would fail depending on the value held at that time.
After discussion with Kelly and Miguel, this was committed since it was
causing intermittent test failures (JUnit unloading a module and reloading
the next one has the same basic behavior that refreshing in hosted mode
does). As Kelly is not familiar with the code in question, I will also have
Scott review it after the checkin since he is traveling.
Review by: knorton (desk review)
scottb (after commit)
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@666 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/jni/linux/ExternalWrapper.cpp b/jni/linux/ExternalWrapper.cpp
index 4f0ae72..eff2802 100644
--- a/jni/linux/ExternalWrapper.cpp
+++ b/jni/linux/ExternalWrapper.cpp
@@ -40,6 +40,8 @@
jsval *argv, jsval *rval)
{
Tracer tracer("gwtOnLoad");
+ tracer.log("context=%08x", unsigned(cx));
+ JsRootedValue::ensureRuntime(cx);
if (argc < 2) {
tracer.setFail("less than 2 args");
return JS_FALSE;
@@ -78,6 +80,9 @@
tracer.setFail("can't get module name in Java string");
return JS_FALSE;
}
+ tracer.log("module name=%s", JS_GetStringBytes(moduleName));
+ } else {
+ tracer.log("null module name");
}
jobject externalObject = NS_REINTERPRET_CAST(jobject, JS_GetPrivate(cx, obj));
@@ -94,6 +99,8 @@
return JS_FALSE;
}
+ tracer.log("scriptGlobal=%08x", unsigned(scriptGlobal.get()));
+
jboolean result = savedJNIEnv->CallBooleanMethod(externalObject, methodID,
NS_REINTERPRET_CAST(jint, scriptGlobal.get()), jModuleName);
if (savedJNIEnv->ExceptionCheck()) {
diff --git a/jni/linux/JsRootedValue.cpp b/jni/linux/JsRootedValue.cpp
index 08dd7cb..90b890b 100644
--- a/jni/linux/JsRootedValue.cpp
+++ b/jni/linux/JsRootedValue.cpp
@@ -19,6 +19,9 @@
// intialize static value used to hold the JavaScript String class.
JSClass* JsRootedValue::stringClass = 0;
+// intialize static reference to the sole JSRuntime value in Gecko
+JSRuntime* JsRootedValue::runtime = 0;
+
/*
* Actually get the stringClass pointer from JavaScript.
*/
diff --git a/jni/linux/JsRootedValue.h b/jni/linux/JsRootedValue.h
index 7aaf9bb..49df6ce 100644
--- a/jni/linux/JsRootedValue.h
+++ b/jni/linux/JsRootedValue.h
@@ -35,7 +35,6 @@
*
* See http://developer.mozilla.org/en/docs/JS_AddRoot for details.
*
- * TODO(jat): handle unboxing Javascript objects like Boolean/etc.
* TODO(jat): rewrite this to minimize the number of roots held and to
* improve 64-bit compatibility.
*/
@@ -43,8 +42,11 @@
{
private:
// the JavaScript String class
- static JSClass* stringClass;
+ static JSClass* stringClass;
+ // Javascript runtime
+ static JSRuntime* runtime;
+
// Javascript context
JSContext* context_;
// underlying Javascript value
@@ -114,19 +116,21 @@
: context_(rooted_value.context_), value_(rooted_value.value_)
{
Tracer tracer("JsRootedValue copy constr", this);
+ tracer.log("context=%08x", unsigned(context_));
tracer.log("other jsRootedVal=%08x", reinterpret_cast<unsigned>(&rooted_value));
- if (!JS_AddRoot(context_, &value_)) {
+ if (!JS_AddNamedRootRT(runtime, &value_, "JsRootedValue copy constr")) {
tracer.log("JsRootedValue copy constructor: JS_AddRoot failed");
// TODO(jat): handle errors
}
}
- JsRootedValue(JSContext* context, jsval value) : context_(context),
- value_(value)
+ JsRootedValue(JSContext* context, jsval value)
+ : context_(context), value_(value)
{
Tracer tracer("JsRootedValue jsval constr", this);
+ tracer.log("context=%08x", unsigned(context_));
tracer.log("jsval=%08x", value);
- if (!JS_AddRoot(context_, &value_)) {
+ if (!JS_AddNamedRootRT(runtime, &value_, "JsRootedValue jsval constr")) {
tracer.log("JsRootedValue jsval constructor: JS_AddRoot failed");
// TODO(jat): handle errors
}
@@ -135,17 +139,32 @@
/*
* Create a void value - safe since no errors can occur
*/
- JsRootedValue(JSContext* context) : context_(context), value_(JSVAL_VOID) { }
+ JsRootedValue(JSContext* context) : context_(context), value_(JSVAL_VOID) {
+ Tracer tracer("JsRootedValue void constr", this);
+ tracer.log("context=%08x", unsigned(context_));
+ if (!JS_AddNamedRootRT(runtime, &value_, "JsRootedValue void constr")) {
+ tracer.log("JsRootedValue jsval constructor: JS_AddRoot failed");
+ // TODO(jat): handle errors
+ }
+ }
/*
* Destroy this object.
*/
- virtual ~JsRootedValue() {
+ ~JsRootedValue() {
Tracer tracer("~JsRootedValue", this);
+ tracer.log("context=%08x", unsigned(context_));
// ignore error since currently it is not possible to fail
- JS_RemoveRoot(context_, &value_);
+ JS_RemoveRootRT(runtime, &value_);
}
-
+
+ /*
+ * Save a pointer to the JSRuntime if we don't have it yet
+ */
+ static void ensureRuntime(JSContext* context) {
+ if(!runtime) runtime = JS_GetRuntime(context);
+ }
+
/*
* Return the JSContext* pointer.
*/
diff --git a/jni/linux/Makefile b/jni/linux/Makefile
index 8c42ae3..ae60f33 100644
--- a/jni/linux/Makefile
+++ b/jni/linux/Makefile
@@ -133,7 +133,13 @@
$(OUT): $(OBJS) $(GWT_TOOLS)/sdk/mozilla-1.7.12/lib/libxpcomglue_s.a
@[ -d $(OUTDIR) ] || mkdir -p $(OUTDIR)
$(LD) -shared $(LDFLAGS) $(LIBPATH) -o $@ $^ $(LIBS)
- $(STRIP) --strip-unneeded $@
+# $(STRIP) --strip-unneeded $@
+
+##
+# copy to staging area for hosted-mode development
+##
+staging: $(OUT)
+ cp $(OUT) ../../build/staging/gwt-linux-0.0.0
##
# Clean rule
diff --git a/jni/linux/NativeWrapper.cpp b/jni/linux/NativeWrapper.cpp
index 8d74787..26014e2 100644
--- a/jni/linux/NativeWrapper.cpp
+++ b/jni/linux/NativeWrapper.cpp
@@ -30,6 +30,7 @@
JSObject *obj, jsval id, jsval *vp)
{
Tracer tracer("gwt_nativewrapper_getProperty");
+ tracer.log("context=%08x", unsigned(cx));
if (*vp != JSVAL_VOID)
return JS_TRUE;
@@ -61,7 +62,7 @@
static JSBool JS_DLL_CALLBACK gwt_nativewrapper_setProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
{
Tracer tracer("gwt_nativewrapper_setProperty");
-
+ tracer.log("context=%08x", unsigned(cx));
jclass dispClass;
jobject dispObj;
jstring ident;
diff --git a/jni/linux/prebuilt/libgwt-ll.so b/jni/linux/prebuilt/libgwt-ll.so
index 805552e..175bda8 100755
--- a/jni/linux/prebuilt/libgwt-ll.so
+++ b/jni/linux/prebuilt/libgwt-ll.so
Binary files differ
