add better handling of Proxy lifecycle

joeferner · joeferner · commit 3d08cdbd6715 · 2014-06-20T13:49:10.000-04:00
diff --git a/README.md b/README.md
@@ -384,8 +384,8 @@ __Example__
 
 Creates a new java Proxy for the given interface. Functions passed in will run on the v8 main thread and not a new thread.
 
-The returned object has two methods ref() and unref() which you can use to maintain references to prevent premature
-garbage collection. You must call these methods to ensure the proxy stays around.
+The returned object has a method unref() which you can use to free the object for
+garbage collection.
 
 __Arguments__
 
diff --git a/compile-java-code.sh b/compile-java-code.sh
@@ -8,3 +8,6 @@ javac ${JAVAC_OPTS} *.java
 
 cd ../src-java/node
 javac ${JAVAC_OPTS} *.java
+
+cd ../../
+javah -classpath src-java -d ./src node.NodeDynamicProxyClass
diff --git a/src-java/node/NodeDynamicProxyClass.class b/src-java/node/NodeDynamicProxyClass.class
diff --git a/src-java/node/NodeDynamicProxyClass.java b/src-java/node/NodeDynamicProxyClass.java
@@ -1,8 +1,10 @@
 package node;
 
-public class NodeDynamicProxyClass implements java.lang.reflect.InvocationHandler
-{
+import java.util.HashSet;
+
+public class NodeDynamicProxyClass implements java.lang.reflect.InvocationHandler {
   private native Object callJs(long ptr, java.lang.reflect.Method m, Object[] args) throws Throwable;
+  private native void unref(long ptr) throws Throwable;
   public long ptr;
 
   public NodeDynamicProxyClass(String path, long ptr) {
@@ -25,11 +27,7 @@ public Object invoke(Object proxy, java.lang.reflect.Method m, Object[] args) th
     return result;
   }
 
-  public void ref() {
-
-  }
-
-  public void unref() {
-
+  public void unref() throws Throwable {
+    unref(this.ptr);
   }
 }
diff --git a/src/java.cpp b/src/java.cpp
@@ -356,6 +356,7 @@ void Java::destroyJVM(JavaVM** jvm, JNIEnv** env) {
   if(result->IsNativeError()) {
     return ThrowException(result);
   }
+  dynamicProxyData->jsObject = v8::Persistent<v8::Value>::New(result);
   return scope.Close(result);
 }
 
@@ -989,3 +990,11 @@ JNIEXPORT jobject JNICALL Java_node_NodeDynamicProxyClass_callJs(JNIEnv *env, jo
   }
   return dynamicProxyData->result;
 }
+
+JNIEXPORT void JNICALL Java_node_NodeDynamicProxyClass_unref(JNIEnv *env, jobject src, jlong ptr) {
+  DynamicProxyData* dynamicProxyData = (DynamicProxyData*)ptr;
+  if(!dynamicProxyDataVerify(dynamicProxyData)) {
+    return;
+  }
+  dynamicProxyData->jsObject.Clear();
+}
diff --git a/src/javaObject.cpp b/src/javaObject.cpp
@@ -97,6 +97,8 @@ JavaObject::~JavaObject() {
     jfieldID ptrField = env->GetFieldID(nodeDynamicProxyClass, "ptr", "J");
     DynamicProxyData* proxyData = (DynamicProxyData*)(long)env->GetLongField(m_obj, ptrField);
     if(dynamicProxyDataVerify(proxyData)) {
+      proxyData->markerStart = 0;
+      proxyData->markerEnd = 0;
       delete proxyData;
     }
   }
diff --git a/src/node_NodeDynamicProxyClass.h b/src/node_NodeDynamicProxyClass.h
diff --git a/src/utils.cpp b/src/utils.cpp
@@ -694,7 +694,7 @@ int dynamicProxyDataVerify(DynamicProxyData* data) {
     return 1;
   }
 
-  printf("*** ERROR: Lost reference to the dynamic proxy. You must maintain a reference in javascript land using ref() and unref(). ***\n");
+  printf("*** ERROR: Lost reference to the dynamic proxy. You must maintain a reference in javascript land using ref() and unref(). (%p) ***\n", data);
   return 0;
 }
 
diff --git a/src/utils.h b/src/utils.h
@@ -35,6 +35,7 @@ struct DynamicProxyData {
   Java* java;
   std::string interfaceName;
   v8::Persistent<v8::Object> functions;
+  v8::Persistent<v8::Value> jsObject;
   std::string methodName;
   jobjectArray args;
   jobject result;

Original file line number	Diff line number	Diff line change
`@@ -356,6 +356,7 @@ void Java::destroyJVM(JavaVM jvm, JNIEnv env) {`
`356`	`356`	`if(result->IsNativeError()) {`
`357`	`357`	`return ThrowException(result);`
`358`	`358`	`}`
	`359`	`+ dynamicProxyData->jsObject = v8::Persistent<v8::Value>::New(result);`
`359`	`360`	`return scope.Close(result);`
`360`	`361`	`}`
`361`	`362`
`@@ -989,3 +990,11 @@ JNIEXPORT jobject JNICALL Java_node_NodeDynamicProxyClass_callJs(JNIEnv *env, jo`
`989`	`990`	`}`
`990`	`991`	`return dynamicProxyData->result;`
`991`	`992`	`}`
	`993`	`+`
	`994`	`+JNIEXPORT void JNICALL Java_node_NodeDynamicProxyClass_unref(JNIEnv *env, jobject src, jlong ptr) {`
	`995`	`+ DynamicProxyData* dynamicProxyData = (DynamicProxyData*)ptr;`
	`996`	`+ if(!dynamicProxyDataVerify(dynamicProxyData)) {`
	`997`	`+ return;`
	`998`	`+ }`
	`999`	`+ dynamicProxyData->jsObject.Clear();`
	`1000`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -97,6 +97,8 @@ JavaObject::~JavaObject() {`
`97`	`97`	`jfieldID ptrField = env->GetFieldID(nodeDynamicProxyClass, "ptr", "J");`
`98`	`98`	`DynamicProxyData* proxyData = (DynamicProxyData*)(long)env->GetLongField(m_obj, ptrField);`
`99`	`99`	`if(dynamicProxyDataVerify(proxyData)) {`
	`100`	`+ proxyData->markerStart = 0;`
	`101`	`+ proxyData->markerEnd = 0;`
`100`	`102`	`delete proxyData;`
`101`	`103`	`}`
`102`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -694,7 +694,7 @@ int dynamicProxyDataVerify(DynamicProxyData* data) {`
`694`	`694`	`return 1;`
`695`	`695`	`}`
`696`	`696`
`697`		`- printf("* ERROR: Lost reference to the dynamic proxy. You must maintain a reference in javascript land using ref() and unref(). *\n");`
	`697`	`+ printf("* ERROR: Lost reference to the dynamic proxy. You must maintain a reference in javascript land using ref() and unref(). (%p) *\n", data);`
`698`	`698`	`return 0;`
`699`	`699`	`}`
`700`	`700`