Skip to content

Commit a31f2af

Browse files
PetterSPetterS
committed
Fix lifetime issue.
Object needs to be able to keep a Context alive.
1 parent 1726795 commit a31f2af

File tree

2 files changed

+50
-32
lines changed

2 files changed

+50
-32
lines changed

‎module.c‎

100755100644
Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,23 @@
22

33
#include "third-party/quickjs.h"
44

5-
static PyObject *JSException = NULL;
6-
static PyObject *quickjs_to_python(JSContext *context, JSValue value);
7-
8-
//
9-
// Object type
10-
//
5+
typedef struct {
6+
PyObject_HEAD JSRuntime *runtime;
7+
JSContext *context;
8+
} ContextData;
119

1210
typedef struct {
1311
PyObject_HEAD;
14-
JSContext *context;
12+
ContextData *context;
1513
JSValue object;
1614
} ObjectData;
1715

16+
static PyObject *JSException = NULL;
17+
static PyObject *quickjs_to_python(ContextData *context_obj, JSValue value);
18+
19+
//
20+
// Object type
21+
//
1822
static PyObject *object_new(PyTypeObject *type, PyObject *args, PyObject *kwds) {
1923
ObjectData *self;
2024
self = (ObjectData *)type->tp_alloc(type, 0);
@@ -26,27 +30,30 @@ static PyObject *object_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
2630

2731
static void object_dealloc(ObjectData *self) {
2832
if (self->context) {
29-
JS_FreeValue(self->context, self->object);
33+
JS_FreeValue(self->context->context, self->object);
34+
Py_DECREF(self->context);
3035
}
3136
Py_TYPE(self)->tp_free((PyObject *)self);
3237
}
3338

3439
static PyObject *object_call(ObjectData *self, PyObject *args, PyObject *kwds);
3540

3641
static PyObject *object_json(ObjectData *self) {
37-
JSValue global = JS_GetGlobalObject(self->context);
38-
JSValue JSON = JS_GetPropertyStr(self->context, global, "JSON");
39-
JSValue stringify = JS_GetPropertyStr(self->context, JSON, "stringify");
42+
JSContext *context = self->context->context;
43+
JSValue global = JS_GetGlobalObject(context);
44+
JSValue JSON = JS_GetPropertyStr(context, global, "JSON");
45+
JSValue stringify = JS_GetPropertyStr(context, JSON, "stringify");
4046
JSValueConst args[1] = {self->object};
41-
JSValue json_string = JS_Call(self->context, stringify, JSON, 1, args);
42-
JS_FreeValue(self->context, global);
43-
JS_FreeValue(self->context, JSON);
44-
JS_FreeValue(self->context, stringify);
47+
JSValue json_string = JS_Call(context, stringify, JSON, 1, args);
48+
JS_FreeValue(context, global);
49+
JS_FreeValue(context, JSON);
50+
JS_FreeValue(context, stringify);
4551
return quickjs_to_python(self->context, json_string);
4652
}
4753

4854
static PyMethodDef object_methods[] = {
49-
{"json", object_json, METH_NOARGS, "Converts to a JSON string."}, {NULL} /* Sentinel */
55+
{"json", (PyCFunction)object_json, METH_NOARGS, "Converts to a JSON string."},
56+
{NULL} /* Sentinel */
5057
};
5158

5259
static PyTypeObject Object = {PyVarObject_HEAD_INIT(NULL, 0).tp_name = "_quickjs.Object",
@@ -68,7 +75,7 @@ static PyObject *object_call(ObjectData *self, PyObject *args, PyObject *kwds) {
6875
PyObject *item = PyTuple_GetItem(args, i);
6976
if (PyLong_Check(item)) {
7077
} else if (PyUnicode_Check(item)) {
71-
} else if (PyObject_IsInstance(item, (PyObject*)&Object)) {
78+
} else if (PyObject_IsInstance(item, (PyObject *)&Object)) {
7279
} else {
7380
PyErr_Format(PyExc_ValueError, "Unsupported type when calling quickjs object");
7481
return NULL;
@@ -80,20 +87,21 @@ static PyObject *object_call(ObjectData *self, PyObject *args, PyObject *kwds) {
8087
if (PyLong_Check(item)) {
8188
jsargs[i] = JS_MKVAL(JS_TAG_INT, PyLong_AsLong(item));
8289
} else if (PyUnicode_Check(item)) {
83-
jsargs[i] = JS_NewString(self->context, PyUnicode_AsUTF8(item));
84-
} else if (PyObject_IsInstance(item, (PyObject*)&Object)) {
85-
jsargs[i] = JS_DupValue(self->context, ((ObjectData *)item)->object);
90+
jsargs[i] = JS_NewString(self->context->context, PyUnicode_AsUTF8(item));
91+
} else if (PyObject_IsInstance(item, (PyObject *)&Object)) {
92+
jsargs[i] = JS_DupValue(self->context->context, ((ObjectData *)item)->object);
8693
}
8794
}
88-
JSValue value = JS_Call(self->context, self->object, JS_NULL, nargs, jsargs);
95+
JSValue value = JS_Call(self->context->context, self->object, JS_NULL, nargs, jsargs);
8996
for (int i = 0; i < nargs; ++i) {
90-
JS_FreeValue(self->context, jsargs[i]);
97+
JS_FreeValue(self->context->context, jsargs[i]);
9198
}
9299
free(jsargs);
93100
return quickjs_to_python(self->context, value);
94101
}
95102

96-
static PyObject *quickjs_to_python(JSContext *context, JSValue value) {
103+
static PyObject *quickjs_to_python(ContextData *context_obj, JSValue value) {
104+
JSContext *context = context_obj->context;
97105
int tag = JS_VALUE_GET_TAG(value);
98106
PyObject *return_value = NULL;
99107

@@ -122,7 +130,8 @@ static PyObject *quickjs_to_python(JSContext *context, JSValue value) {
122130
} else if (tag == JS_TAG_OBJECT) {
123131
return_value = PyObject_CallObject((PyObject *)&Object, NULL);
124132
ObjectData *object = (ObjectData *)return_value;
125-
object->context = context;
133+
Py_INCREF(context_obj);
134+
object->context = context_obj;
126135
object->object = JS_DupValue(context, value);
127136
} else {
128137
PyErr_Format(PyExc_ValueError, "Unknown quickjs tag: %d", tag);
@@ -144,11 +153,6 @@ struct module_state {};
144153
//
145154
// Context type
146155
//
147-
typedef struct {
148-
PyObject_HEAD JSRuntime *runtime;
149-
JSContext *context;
150-
} ContextData;
151-
152156
static PyObject *context_new(PyTypeObject *type, PyObject *args, PyObject *kwds) {
153157
ContextData *self;
154158
self = (ContextData *)type->tp_alloc(type, 0);
@@ -171,7 +175,7 @@ static PyObject *context_eval(ContextData *self, PyObject *args) {
171175
return NULL;
172176
}
173177
JSValue value = JS_Eval(self->context, code, strlen(code), "<input>", JS_EVAL_TYPE_GLOBAL);
174-
return quickjs_to_python(self->context, value);
178+
return quickjs_to_python(self, value);
175179
}
176180

177181
static PyObject *context_get(ContextData *self, PyObject *args) {
@@ -182,7 +186,7 @@ static PyObject *context_get(ContextData *self, PyObject *args) {
182186
JSValue global = JS_GetGlobalObject(self->context);
183187
JSValue value = JS_GetPropertyStr(self->context, global, name);
184188
JS_FreeValue(self->context, global);
185-
return quickjs_to_python(self->context, value);
189+
return quickjs_to_python(self, value);
186190
}
187191

188192
static PyMethodDef context_methods[] = {

‎test_quickjs.py‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,21 @@ def test_error(self):
5757
with self.assertRaisesRegex(quickjs.JSException, "ReferenceError: missing is not defined"):
5858
self.context.eval("missing + missing")
5959

60+
def test_lifetime(self):
61+
def get_f():
62+
context = quickjs.Context()
63+
f = context.eval("""
64+
a = function(x) {
65+
return 40 + x;
66+
}
67+
""")
68+
return f
69+
70+
f = get_f()
71+
self.assertTrue(f)
72+
# The context has left the scope after f. f needs to keep the context alive for the
73+
# its lifetime. Otherwise, we will get problems.
74+
6075

6176
class Object(unittest.TestCase):
6277
def setUp(self):
@@ -86,7 +101,6 @@ def test_function_call_int_two_args(self):
86101
""")
87102
self.assertEqual(f(3, -1), 42)
88103

89-
90104
def test_function_call_many_times(self):
91105
n = 1000
92106
f = self.context.eval("""

0 commit comments

Comments
 (0)