Skip to content

Commit 6d5e357

Browse files
committed
Fix segfaults when large numbers of contexts.
Turns out that I had an error in my logic about how to reference count the python context object. I removed references to it from Python objects that are wrapped in the JavaScript VM. I'm more than 50% certain that this is correct as when the Python context dies, it'll destroy the JSContext* and along with it all references to Python objects that need refernces to the Context. [#21 state:resolved]
1 parent 38872a8 commit 6d5e357

File tree

7 files changed

+38
-18
lines changed

7 files changed

+38
-18
lines changed

THANKS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,6 @@ Keiji Costantini
3030

3131
Richard Boulton
3232
* Initial patch for filtering Python access.
33+
34+
marc
35+
* Report on Context turnover causing segfaults.

spidermonkey/context.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,6 @@ Context_new(PyTypeObject* type, PyObject* args, PyObject* kwargs)
344344
* garbage collection from happening on either side of the
345345
* bridge.
346346
*
347-
* To make sure that the context stays alive we'll add a
348-
* reference to the Context* anytime we wrap a Python
349-
* object for use in JS.
350-
*
351347
*/
352348
JS_SetContextPrivate(self->cx, self);
353349

spidermonkey/pyiter.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ finalize(JSContext* jscx, JSObject* jsobj)
3434
if(pycx == NULL)
3535
{
3636
fprintf(stderr, "*** NO PYTHON CONTEXT ***\n");
37+
JS_EndRequest(jscx);
3738
return;
3839
}
3940

spidermonkey/pyobject.c

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -224,12 +224,6 @@ js_finalize(JSContext* jscx, JSObject* jsobj)
224224
JS_EndRequest(jscx);
225225

226226
Py_DECREF(pyobj);
227-
228-
// Technically, this could turn out to be nasty. If
229-
// this is the last object keeping the python cx
230-
// alive, then this call could be deleting the cx
231-
// we're about to return to.
232-
Py_DECREF(pycx);
233227
}
234228

235229
PyObject*
@@ -493,12 +487,6 @@ py2js_object(Context* cx, PyObject* pyobj)
493487
goto error;
494488
}
495489

496-
/*
497-
As noted in Context_new, here we must ref the Python context
498-
to make sure it stays alive while a Python object may be
499-
referenced in the JS VM.
500-
*/
501-
Py_INCREF(cx);
502490
ret = OBJECT_TO_JSVAL(jsobj);
503491
goto success;
504492

spidermonkey/runtime.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,17 @@ Runtime_new(PyTypeObject* type, PyObject* args, PyObject* kwargs)
2020
if(self == NULL) goto error;
2121

2222
self->rt = JS_NewRuntime(stacksize);
23-
if(self->rt == NULL) goto error;
23+
if(self->rt == NULL)
24+
{
25+
PyErr_SetString(JSError, "Failed to allocate new JSRuntime.");
26+
goto error;
27+
}
2428

2529
goto success;
2630

2731
error:
2832
Py_XDECREF(self);
33+
self = NULL;
2934

3035
success:
3136
return (PyObject*) self;

tests/test-context.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def test_exceed_time(cx):
4646
script = """
4747
var time = function() {return (new Date()).getTime();};
4848
var start = time();
49-
while((time() - start) < 2000) {}
49+
while((time() - start) < 100000) {}
5050
"""
5151
cx.max_time(1)
5252
t.raises(SystemError, cx.execute, script)

tests/test-turnover.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Copyright 2009 Paul J. Davis <paul.joseph.davis@gmail.com>
2+
#
3+
# This file is part of the python-spidermonkey package released
4+
# under the MIT license.
5+
import t
6+
7+
#def test_churn_runtimes():
8+
# for i in range(1000):
9+
# rt = t.spidermonkey.Runtime()
10+
11+
12+
class Session(object):
13+
def __init__(self):
14+
self.key = None
15+
16+
def set(self, key):
17+
self.key = key
18+
return self
19+
20+
@t.rt()
21+
def test_churn_contexts(rt):
22+
for i in range(1000):
23+
cx = rt.new_context()
24+
cx.add_global('session', Session)
25+
data = cx.execute('new session().set("foo");')
26+
t.eq(data.key, "foo")
27+

0 commit comments

Comments
 (0)