Skip to content

Commit

Permalink
Fix segfaults when large numbers of contexts.
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
davisp committed Jun 26, 2009
1 parent 38872a8 commit 6d5e357
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 18 deletions.
3 changes: 3 additions & 0 deletions THANKS
Expand Up @@ -30,3 +30,6 @@ Keiji Costantini

Richard Boulton
* Initial patch for filtering Python access.

marc
* Report on Context turnover causing segfaults.
4 changes: 0 additions & 4 deletions spidermonkey/context.c
Expand Up @@ -344,10 +344,6 @@ Context_new(PyTypeObject* type, PyObject* args, PyObject* kwargs)
* garbage collection from happening on either side of the
* bridge.
*
* To make sure that the context stays alive we'll add a
* reference to the Context* anytime we wrap a Python
* object for use in JS.
*
*/
JS_SetContextPrivate(self->cx, self);

Expand Down
1 change: 1 addition & 0 deletions spidermonkey/pyiter.c
Expand Up @@ -34,6 +34,7 @@ finalize(JSContext* jscx, JSObject* jsobj)
if(pycx == NULL)
{
fprintf(stderr, "*** NO PYTHON CONTEXT ***\n");
JS_EndRequest(jscx);
return;
}

Expand Down
12 changes: 0 additions & 12 deletions spidermonkey/pyobject.c
Expand Up @@ -224,12 +224,6 @@ js_finalize(JSContext* jscx, JSObject* jsobj)
JS_EndRequest(jscx);

Py_DECREF(pyobj);

// Technically, this could turn out to be nasty. If
// this is the last object keeping the python cx
// alive, then this call could be deleting the cx
// we're about to return to.
Py_DECREF(pycx);
}

PyObject*
Expand Down Expand Up @@ -493,12 +487,6 @@ py2js_object(Context* cx, PyObject* pyobj)
goto error;
}

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

Expand Down
7 changes: 6 additions & 1 deletion spidermonkey/runtime.c
Expand Up @@ -20,12 +20,17 @@ Runtime_new(PyTypeObject* type, PyObject* args, PyObject* kwargs)
if(self == NULL) goto error;

self->rt = JS_NewRuntime(stacksize);
if(self->rt == NULL) goto error;
if(self->rt == NULL)
{
PyErr_SetString(JSError, "Failed to allocate new JSRuntime.");

This comment has been minimized.

Copy link
@pykler

pykler Sep 24, 2013

I am starting to get this error with Ming.mim ... essentially I am creating a new JSRuntime for every test case, and with my test suite it starts to raise this exc. Any ideas of how I can work around this error?

This comment has been minimized.

Copy link
@davisp

davisp Sep 25, 2013

Author Owner

If you can, cache your JSRuntime. If you create new contexts for each test you should be fine from an isolation standpoint.

goto error;
}

goto success;

error:
Py_XDECREF(self);
self = NULL;

success:
return (PyObject*) self;
Expand Down
2 changes: 1 addition & 1 deletion tests/test-context.py
Expand Up @@ -46,7 +46,7 @@ def test_exceed_time(cx):
script = """
var time = function() {return (new Date()).getTime();};
var start = time();
while((time() - start) < 2000) {}
while((time() - start) < 100000) {}
"""
cx.max_time(1)
t.raises(SystemError, cx.execute, script)
Expand Down
27 changes: 27 additions & 0 deletions tests/test-turnover.py
@@ -0,0 +1,27 @@
# Copyright 2009 Paul J. Davis <paul.joseph.davis@gmail.com>
#
# This file is part of the python-spidermonkey package released
# under the MIT license.
import t

#def test_churn_runtimes():
# for i in range(1000):
# rt = t.spidermonkey.Runtime()


class Session(object):
def __init__(self):
self.key = None

def set(self, key):
self.key = key
return self

@t.rt()
def test_churn_contexts(rt):
for i in range(1000):
cx = rt.new_context()
cx.add_global('session', Session)
data = cx.execute('new session().set("foo");')
t.eq(data.key, "foo")

0 comments on commit 6d5e357

Please sign in to comment.