changeset 61:1506350991d4

JS Wrapped python functions are now only GC'd by python once they've first been GC'd by JS.
author Atul Varma <varmaa@toolness.com>
date Sat, 25 Jul 2009 16:14:03 -0700
parents e557d84318a7
children 2b5696b91b01
files function.c function.h object.c test_pymonkey.py
diffstat 4 files changed, 72 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/function.c	Sat Jul 25 16:10:21 2009 -0700
+++ b/function.c	Sat Jul 25 16:14:03 2009 -0700
@@ -37,15 +37,54 @@
 #include "function.h"
 #include "utils.h"
 
+static JSBool
+getHeldFunction(JSContext *cx, JSObject *obj, PyObject **callable)
+{
+  jsval jsCallable;
+  if (!JS_GetReservedSlot(cx, obj, 0, &jsCallable)) {
+    JS_ReportError(cx, "JS_GetReservedSlot() failed.");
+    return JS_FALSE;
+  }
+  *callable = (PyObject *) JSVAL_TO_PRIVATE(jsCallable);
+  return JS_TRUE;
+}
+
+static void
+finalizeFunctionHolder(JSContext *cx, JSObject *obj)
+{
+  PyObject *callable;
+  if (getHeldFunction(cx, obj, &callable))
+    Py_DECREF(callable);
+}
+
+JSClass PYM_JS_FunctionHolderClass = {
+  "PymonkeyFunctionHolder", JSCLASS_HAS_RESERVED_SLOTS(1),
+  JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_PropertyStub,
+  JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub,
+  finalizeFunctionHolder,
+  JSCLASS_NO_OPTIONAL_MEMBERS
+};
+
+static JSObject *
+newFunctionHolder(JSContext *cx, PyObject *callable)
+{
+  JSObject *obj = JS_NewObject(cx, &PYM_JS_FunctionHolderClass, NULL, NULL);
+  if (obj) {
+    if (JS_SetReservedSlot(cx, obj, 0, PRIVATE_TO_JSVAL(callable)))
+      Py_INCREF(callable);
+    else {
+      obj = NULL;
+      PyErr_SetString(PYM_error, "JS_SetReservedSlot() failed");
+    }
+  } else {
+    PyErr_SetString(PYM_error, "JS_NewObject() failed");
+  }
+  return obj;
+}
+
 static void
 PYM_JSFunctionDealloc(PYM_JSFunction *self)
 {
-  // TODO: What if there's still a reference to the callable in
-  // JS-land?
-  if (self->callable) {
-    Py_DECREF(self->callable);
-    self->callable = NULL;
-  }
   PYM_JSObjectType.tp_dealloc((PyObject *) self);
 }
 
@@ -57,12 +96,15 @@
                            jsval *rval)
 {
   jsval callee = JS_ARGV_CALLEE(argv);
-  jsval jsCallable;
-  if (!JS_GetReservedSlot(cx, JSVAL_TO_OBJECT(callee), 0, &jsCallable)) {
+  jsval functionHolder;
+  if (!JS_GetReservedSlot(cx, JSVAL_TO_OBJECT(callee), 0, &functionHolder)) {
     JS_ReportError(cx, "JS_GetReservedSlot() failed.");
     return JS_FALSE;
   }
-  PyObject *callable = (PyObject *) JSVAL_TO_PRIVATE(jsCallable);
+
+  PyObject *callable;
+  if (!getHeldFunction(cx, JSVAL_TO_OBJECT(functionHolder), &callable))
+    return JS_FALSE;
 
   PYM_JSContextObject *context = (PYM_JSContextObject *)
     JS_GetContextPrivate(cx);
@@ -193,26 +235,29 @@
     return NULL;
   }
 
-  if (!JS_SetReservedSlot(context->cx, funcObj, 0,
-                          PRIVATE_TO_JSVAL(callable))) {
-    PyErr_SetString(PYM_error, "JS_SetReservedSlot() failed");
-    return NULL;
-  }
-
   PYM_JSFunction *object = PyObject_New(PYM_JSFunction,
                                         &PYM_JSFunctionType);
   if (object == NULL)
     return NULL;
 
-  object->callable = NULL;
   if (PYM_newJSObject(context, funcObj,
                       (PYM_JSObject *) object) == NULL)
     // Note that our object's reference count will have already
     // been decremented by PYM_newJSObject().
     return NULL;
 
-  object->callable = callable;
-  Py_INCREF(callable);
+  JSObject *functionHolder = newFunctionHolder(context->cx, callable);
+  if (functionHolder == NULL) {
+    Py_DECREF((PyObject *) object);
+    return NULL;
+  }
+
+  if (!JS_SetReservedSlot(context->cx, funcObj, 0,
+                          OBJECT_TO_JSVAL(functionHolder))) {
+    Py_DECREF((PyObject *) object);
+    PyErr_SetString(PYM_error, "JS_SetReservedSlot() failed");
+    return NULL;
+  }
 
   return object;
 }
--- a/function.h	Sat Jul 25 16:10:21 2009 -0700
+++ b/function.h	Sat Jul 25 16:14:03 2009 -0700
@@ -45,7 +45,6 @@
 
 typedef struct {
   PYM_JSObject base;
-  PyObject *callable;
 } PYM_JSFunction;
 
 extern PyTypeObject PYM_JSFunctionType;
--- a/object.c	Sat Jul 25 16:10:21 2009 -0700
+++ b/object.c	Sat Jul 25 16:14:03 2009 -0700
@@ -142,8 +142,6 @@
     if (JS_ObjectIsFunction(context->cx, obj)) {
       PYM_JSFunction *func = PyObject_New(PYM_JSFunction,
                                           &PYM_JSFunctionType);
-      if (func != NULL)
-        func->callable = NULL;
       object = (PYM_JSObject *) func;
     } else
       object = PyObject_New(PYM_JSObject,
--- a/test_pymonkey.py	Sat Jul 25 16:10:21 2009 -0700
+++ b/test_pymonkey.py	Sat Jul 25 16:14:03 2009 -0700
@@ -34,21 +34,28 @@
                          "pymonkey.undefined")
 
     def testJsWrappedPythonFuncIsNotGCd(self):
-        # TODO: Make this test pass.
         def define(cx, obj):
             def func(cx, this, args):
                 return u'func was called'
             jsfunc = cx.new_function(func, func.__name__)
             cx.define_property(obj, func.__name__, jsfunc)
             return weakref.ref(func)
-        cx = pymonkey.Runtime().new_context()
+        rt = pymonkey.Runtime()
+        cx = rt.new_context()
         obj = cx.new_object()
         cx.init_standard_classes(obj)
         ref = define(cx, obj)
+        cx.gc()
         self.assertNotEqual(ref(), None)
         result = cx.evaluate_script(obj, 'func()', '<string>', 1)
         self.assertEqual(result, u'func was called')
 
+        # Now ensure that the wrapped function is GC'd when it's
+        # no longer reachable from JS space.
+        cx.define_property(obj, 'func', 0)
+        cx.gc()
+        self.assertEqual(ref(), None)
+
     def testJsWrappedPythonFuncPassesContext(self):
         contexts = []