Fix Python reference count leak

tests/test_context.py

@@ -1,7 +1,7 @@
 import pytest
 import time
 
-from v8py import JavaScriptTerminated, current_context, new
+from v8py import JavaScriptTerminated, Context, current_context, new
 
 def test_glob(context):
  context.eval('foo = "bar"')
@@ -108,3 +108,11 @@ def f():
  assert current_context() is context
  context.expose(f)
  context.eval('f()')
+
+def test_object_sharing(context):
+ shared = Context()
+ shared.foo = {'x': object()}
+ output = str(shared.eval('foo'))
+ context.foo = shared.foo
+ del shared
+ assert str(context.eval('foo')) == output

v8py/context.cpp

@@ -99,11 +99,7 @@ PyObject *context_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) {
  context->SetEmbedderData(OBJECT_PROTOTYPE_SLOT, Object::New(isolate)->GetPrototype());
  context->SetEmbedderData(ERROR_PROTOTYPE_SLOT, Exception::Error(String::Empty(isolate)).As<Object>()->GetPrototype());
 
- PyObject *weakref_module = PyImport_ImportModule("weakref");
- PyErr_PROPAGATE(weakref_module);
- PyObject *weak_key_dict = PyObject_GetAttrString(weakref_module, "WeakKeyDictionary");
- PyErr_PROPAGATE(weak_key_dict);
- self->js_object_cache = PyObject_CallObject(weak_key_dict, NULL);
+ self->js_object_cache = PyDict_New();
  PyErr_PROPAGATE(self->js_object_cache);
 
  self->scripts = PySet_New(NULL);
@@ -118,6 +114,7 @@ PyObject *context_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) {
 
 void context_dealloc(context_c *self) {
  self->js_context.Reset();
+ PyDict_Clear(self->js_object_cache);
  Py_DECREF(self->js_object_cache);
  Py_DECREF(self->scripts);
  Py_TYPE(self)->tp_free((PyObject *) self);

v8py/convert.cpp

@@ -82,9 +82,9 @@ PyObject *py_from_js(Local<Value> value, Local<Context> context) {
  return dict;
  }
  if (obj_value->InternalFieldCount() == OBJECT_INTERNAL_FIELDS) {
- Local<Value> magic = obj_value->GetInternalField(0);
+ Local<Value> magic = obj_value->GetInternalField(MAGIC_INTERNAL_FIELD);
  if (magic == IZ_DAT_OBJECT) {
- PyObject *object = (PyObject *) obj_value->GetInternalField(1).As<External>()->Value();
+ PyObject *object = (PyObject *) obj_value->GetInternalField(PYOBJECT_INTERNAL_FIELD).As<External>()->Value();
  Py_INCREF(object);
  return object;
  }

v8py/exception.cpp

@@ -79,9 +79,9 @@ extern "C" {
 void py_throw_js(Local<Value> js_exc, Local<Message> js_message) {
  if (js_exc->IsObject() && js_exc.As<Object>()->InternalFieldCount() == OBJECT_INTERNAL_FIELDS) {
  Local<Object> exc_object = js_exc.As<Object>();
- PyObject *exc_type = (PyObject *) exc_object->GetInternalField(2).As<External>()->Value();
- PyObject *exc_value = (PyObject *) exc_object->GetInternalField(1).As<External>()->Value();
- PyObject *exc_traceback = (PyObject *) exc_object->GetInternalField(3).As<External>()->Value();
+ PyObject *exc_type = (PyObject *) exc_object->GetInternalField(EXCEPTION_TYPE_INTERNAL_FIELD).As<External>()->Value();
+ PyObject *exc_value = (PyObject *) exc_object->GetInternalField(PYOBJECT_INTERNAL_FIELD).As<External>()->Value();
+ PyObject *exc_traceback = (PyObject *) exc_object->GetInternalField(TRACEBACK_INTERNAL_FIELD).As<External>()->Value();
  if (exc_type == NULL && exc_traceback == NULL) {
  PyErr_SetObject((PyObject *) Py_TYPE(exc_value), exc_value);
  } else {
@@ -155,8 +155,8 @@ void js_throw_py() {
  exception = ((js_exception *) exc_value)->exception.Get(isolate).As<Object>();
  } else {
  exception = js_from_py(exc_value, context).As<Object>();
- exception->SetInternalField(2, External::New(isolate, exc_type));
- exception->SetInternalField(3, External::New(isolate, exc_traceback));
+ exception->SetInternalField(EXCEPTION_TYPE_INTERNAL_FIELD, External::New(isolate, exc_type));
+ exception->SetInternalField(TRACEBACK_INTERNAL_FIELD, External::New(isolate, exc_traceback));
  }
  isolate->ThrowException(exception);
 }

v8py/pyclass.cpp

@@ -262,19 +262,31 @@ Local<Function> py_class_get_constructor(py_class *self, Local<Context> context)
 void py_class_object_weak_callback(const WeakCallbackInfo<Persistent<Object>> &info) {
  HandleScope hs(isolate);
  Local<Object> js_object = info.GetParameter()->Get(isolate);
- assert(js_object->GetInternalField(0) == IZ_DAT_OBJECT);
- PyObject *py_object = (PyObject *) js_object->GetInternalField(1).As<External>()->Value();
 
- // the entire purpose of this weak callback
- Py_DECREF(py_object);
+ assert(js_object->GetInternalField(MAGIC_INTERNAL_FIELD) == IZ_DAT_OBJECT);
+ PyObject *py_object = (PyObject *) js_object->GetInternalField(PYOBJECT_INTERNAL_FIELD).As<External>()->Value();
+ PyObject *js_object_cache = (PyObject *) js_object->GetInternalField(JS_OBJECT_CACHE_INTERNAL_FIELD).As<External>()->Value();
+
+ if (PyDict_Size(js_object_cache) != 0) {
+ if (PyMapping_HasKey(js_object_cache, py_object)) {
+ PyObject_DelItem(js_object_cache, py_object);
+ }
+ }
+ Py_DECREF(js_object_cache);
 
  info.GetParameter()->Reset();
  delete info.GetParameter();
 }
 
+
 void py_class_init_js_object(Local<Object> js_object, PyObject *py_object, Local<Context> context) {
- js_object->SetInternalField(0, IZ_DAT_OBJECT);
- js_object->SetInternalField(1, External::New(isolate, py_object));
+ js_object->SetInternalField(MAGIC_INTERNAL_FIELD, IZ_DAT_OBJECT);
+ js_object->SetInternalField(PYOBJECT_INTERNAL_FIELD, External::New(isolate, py_object));
+ // the v8::Context will get garbage collected before the JS objects,
+ // so for the weak callback we keep a pointer to the object cache
+ context_c *py_ctx = (context_c *)context->GetEmbedderData(CONTEXT_OBJECT_SLOT).As<External>()->Value();
+ Py_INCREF(py_ctx->js_object_cache);
+ js_object->SetInternalField(JS_OBJECT_CACHE_INTERNAL_FIELD, External::New(isolate, py_ctx->js_object_cache));
 
  // find out if the object is supposed to inherit from Error
  // the information is in an internal field on the last prototype
@@ -309,6 +321,7 @@ Local<Object> py_class_create_js_object(py_class *self, PyObject *py_object, Loc
  object = self->templ->Get(isolate)->InstanceTemplate()->NewInstance(context).ToLocalChecked();
  Py_INCREF(py_object);
  py_class_init_js_object(object, py_object, context);
+ Py_DECREF(py_object);
 
  return hs.Escape(object);
 }

v8py/pyclass.h

@@ -21,11 +21,12 @@ Local<Function> py_class_get_constructor(py_class *self, Local<Context> context)
 Local<Object> py_class_create_js_object(py_class *self, PyObject *py_object, Local<Context> context);
 void py_class_init_js_object(Local<Object> js_object, PyObject *py_object, Local<Context> context);
 
-// first one is magic pointer
-// second one is actual object
-// third is exception traceback (usually unused)
-// fourth is exception type (also usually unused)
-#define OBJECT_INTERNAL_FIELDS 4
+#define MAGIC_INTERNAL_FIELD 0
+#define PYOBJECT_INTERNAL_FIELD 1
+#define EXCEPTION_TYPE_INTERNAL_FIELD 2
+#define TRACEBACK_INTERNAL_FIELD 3
+#define JS_OBJECT_CACHE_INTERNAL_FIELD 4
+#define OBJECT_INTERNAL_FIELDS 5
 
 void py_class_construct_callback(const FunctionCallbackInfo<Value> &info);
 void py_class_method_callback(const FunctionCallbackInfo<Value> &info);

v8py/pyclasshandlers.cpp

@@ -42,7 +42,7 @@ void py_class_method_callback(const FunctionCallbackInfo<Value> &info) {
  if (js_self == context->Global()) {
  js_self = js_self->GetPrototype().As<Object>();
  }
- PyObject *self = (PyObject *) js_self->GetInternalField(1).As<External>()->Value();
+ PyObject *self = (PyObject *) js_self->GetInternalField(PYOBJECT_INTERNAL_FIELD).As<External>()->Value();
  PyObject *args = pys_from_jss(info, context);
  JS_PROPAGATE_PY(args);
  // add self onto the beginning of the arg list
@@ -79,7 +79,7 @@ template <class T> inline extern PyObject *get_self(const PropertyCallbackInfo<T
  if (js_self == context->Global()) {
  js_self = js_self->GetPrototype().As<Object>();
  }
- return (PyObject *) js_self->GetInternalField(1).template As<External>()->Value();
+ return (PyObject *) js_self->GetInternalField(PYOBJECT_INTERNAL_FIELD).template As<External>()->Value();
 }
 
 #define Info(T) const PropertyCallbackInfo<T> &info