Skip to content

Commit

Permalink
Fix Locker crash issue
Browse files Browse the repository at this point in the history
  • Loading branch information
flier committed Feb 7, 2017
1 parent aa7a0d4 commit 3ab967e
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 107 deletions.
30 changes: 15 additions & 15 deletions PyV8.py
Expand Up @@ -31,6 +31,12 @@
except ImportError:
import simplejson as json

if __name__ == '__main__':
if "-p" in sys.argv:
sys.argv.remove("-p")
print("Press any key to continue or attach process #%d..." % os.getpid())
raw_input()

import _PyV8

__author__ = 'Flier Lu <flier.lu@gmail.com>'
Expand Down Expand Up @@ -766,11 +772,11 @@ def __exit__(self, exc_type, exc_value, traceback):

JSStackTrace = _PyV8.JSStackTrace
JSStackTrace.Options = _PyV8.JSStackTraceOptions
JSStackTrace.GetCurrentStackTrace = staticmethod(lambda frame_limit, options: _PyV8.JSIsolate.current.GetCurrentStackTrace(frame_limit, options))
JSStackTrace.GetCurrentStackTrace = staticmethod(lambda frame_limit, options: _PyV8.JSManagedIsolate.current.GetCurrentStackTrace(frame_limit, options))
JSStackFrame = _PyV8.JSStackFrame


class JSIsolate(_PyV8.JSIsolate):
class JSIsolate(_PyV8.JSManagedIsolate):
def __enter__(self):
self.enter()

Expand Down Expand Up @@ -805,16 +811,16 @@ def __enter__(self):
def __exit__(self, exc_type, exc_value, tb):
self.leave()

if hasattr(JSLocker, 'lock'):
self.lock.leave()
self.lock = None

if exc_type:
logging.warn("throw exceptions in %r", self)
logging.debug(''.join(traceback.format_exception(exc_type, exc_value, tb)))

self.dispose()

if hasattr(JSLocker, 'lock'):
self.lock.leave()
self.lock = None

del self


Expand Down Expand Up @@ -1995,7 +2001,7 @@ def run():

class TestEngine(unittest.TestCase):
def setUp(self):
self.isolate = JSIsolate(True)
self.isolate = JSIsolate()
self.isolate.enter()

def tearDown(self):
Expand All @@ -2004,9 +2010,8 @@ def tearDown(self):
del self.isolate

def testClassProperties(self):
with JSContext() as ctxt:
self.assertTrue(str(JSEngine.version).startswith("5."))
self.assertFalse(JSEngine.dead)
self.assertTrue(str(JSEngine.version).startswith("5."))
self.assertFalse(JSEngine.dead)

def testCompile(self):
with JSContext() as ctxt:
Expand Down Expand Up @@ -2795,11 +2800,6 @@ def onSwitchStatement(self, stmt):
else:
level = logging.WARN

if "-p" in sys.argv:
sys.argv.remove("-p")
print("Press any key to continue or attach process #%d..." % os.getpid())
raw_input()

logging.basicConfig(level=level, format='%(asctime)s %(levelname)s %(message)s')

logging.info("testing PyV8 module %s with V8 v%s (boost v%s)",
Expand Down
39 changes: 23 additions & 16 deletions src/Context.cpp
Expand Up @@ -44,13 +44,9 @@ void CContext::Expose(void)

.def("__nonzero__", &CContext::IsEntered, "the context has been entered.");

py::objects::class_value_wrapper<boost::shared_ptr<CIsolate>,
py::objects::make_ptr_instance<CIsolate,
py::objects::pointer_holder<boost::shared_ptr<CIsolate>, CIsolate>>>();

py::objects::class_value_wrapper<boost::shared_ptr<CContext>,
py::objects::class_value_wrapper<CContextPtr,
py::objects::make_ptr_instance<CContext,
py::objects::pointer_holder<boost::shared_ptr<CContext>, CContext>>>();
py::objects::pointer_holder<CContextPtr, CContext>>>();
}

CContext::CContext(v8::Handle<v8::Context> context, v8::Isolate *isolate) : m_context(isolate, context)
Expand Down Expand Up @@ -89,21 +85,32 @@ CContext::CContext(py::object global, py::list extensions, v8::Isolate *isolate)
if (!ext_ptrs.empty())
cfg.reset(new v8::ExtensionConfiguration(ext_ptrs.size(), &ext_ptrs[0]));

v8::TryCatch try_catch(isolate);
v8::Handle<v8::Context> context = v8::Context::New(isolate, cfg.get());

m_context.Reset(isolate, context);

BOOST_LOG_SEV(logger(), trace) << "context created";
if (context.IsEmpty())
{
if (try_catch.HasCaught())
CJavascriptException::ThrowIf(isolate, try_catch);

if (!global.is_none())
BOOST_LOG_SEV(CIsolate::Current().Logger(), warning) << "failed to create context";
}
else
{
v8::Context::Scope context_scope(context);
m_context.Reset(isolate, context);

context->Global()->Set(context, v8::String::NewFromUtf8(isolate, "__proto__"), CPythonObject::Wrap(global));
BOOST_LOG_SEV(logger(), trace) << "context created";

m_global = global;
if (!global.is_none())
{
v8::Context::Scope context_scope(context);

context->Global()->Set(context, v8::String::NewFromUtf8(isolate, "__proto__"), CPythonObject::Wrap(global));

Py_DECREF(global.ptr());
m_global = global;

Py_DECREF(global.ptr());
}
}
}

Expand All @@ -125,7 +132,7 @@ void CContext::Dispose(bool disposed, v8::Isolate *isolate)

logger_t &CContext::GetLogger(v8::Handle<v8::Context> context)
{
auto logger = GetEmbedderData<logger_t>(context, EmbedderDataFields::LoggerIndex, [context]() -> logger_t * {
auto logger = GetEmbedderData<logger_t>(context, EmbedderDataFields::LoggerIndex, [context]() {
auto logger = new logger_t();

logger->add_attribute(ISOLATE_ATTR, attrs::constant<const v8::Isolate *>(context->GetIsolate()));
Expand Down Expand Up @@ -208,7 +215,7 @@ py::object CContext::GetCurrent(v8::Isolate *isolate)

v8::Handle<v8::Context> current = isolate->GetCurrentContext();

return (current.IsEmpty()) ? py::object() : py::object(py::handle<>(boost::python::converter::shared_ptr_to_python<CContext>(CContextPtr(new CContext(current)))));
return (current.IsEmpty()) ? py::object() : py::object(py::handle<>(py::converter::shared_ptr_to_python<CContext>(CContextPtr(new CContext(current)))));
}

py::object CContext::GetCalling(v8::Isolate *isolate)
Expand Down
9 changes: 8 additions & 1 deletion src/Context.h
Expand Up @@ -50,7 +50,14 @@ class CContext final

static logger_t &GetLogger(v8::Handle<v8::Context> context);

logger_t &logger(void) { return GetLogger(Context()); }
logger_t &logger(v8::Isolate *isolate = v8::Isolate::GetCurrent())
{
v8::HandleScope handle_scope(isolate);

auto context = m_context.Get(isolate);

return GetLogger(context);
}

public:
CContext(v8::Handle<v8::Context> context, v8::Isolate *isolate = v8::Isolate::GetCurrent());
Expand Down
53 changes: 29 additions & 24 deletions src/Isolate.cpp
Expand Up @@ -2,25 +2,30 @@

void CManagedIsolate::Expose(void)
{
py::class_<CManagedIsolate, boost::noncopyable>("JSIsolate", py::no_init)
.def(py::init<>("Creates a new isolate. Does not change the currently entered isolate."))
py::class_<CIsolateWrapper, boost::noncopyable>("JSIsolate", py::no_init)
.add_property("locked", &CIsolateWrapper::IsLocked)
.add_property("used", &CIsolateWrapper::InUse, "Check if this isolate is in use.")

.add_property("locked", &CManagedIsolate::IsLocked)
.add_property("used", &CManagedIsolate::InUse, "Check if this isolate is in use.")

.def("enter", &CManagedIsolate::Enter,
.def("enter", &CIsolateWrapper::Enter,
"Sets this isolate as the entered one for the current thread. "
"Saves the previously entered one (if any), so that it can be "
"restored when exiting. Re-entering an isolate is allowed.")

.def("leave", &CManagedIsolate::Leave,
.def("leave", &CIsolateWrapper::Leave,
"Exits this isolate by restoring the previously entered one in the current thread. "
"The isolate may still stay the same, if it was entered more than once.")

.add_static_property("current", &CManagedIsolate::GetCurrent,
.add_static_property("current", &CIsolateWrapper::GetCurrent,
"Returns the entered isolate for the current thread or NULL in case there is no current isolate.")

.def("GetCurrentStackTrace", &CManagedIsolate::GetCurrentStackTrace);
.def("GetCurrentStackTrace", &CIsolateWrapper::GetCurrentStackTrace);

py::class_<CManagedIsolate, py::bases<CIsolateWrapper>, boost::noncopyable>("JSManagedIsolate", py::no_init)
.def(py::init<>("Creates a new isolate. Does not change the currently entered isolate."));

py::objects::class_value_wrapper<CIsolateWrapperPtr,
py::objects::make_ptr_instance<CIsolateWrapper,
py::objects::pointer_holder<CIsolateWrapperPtr, CIsolateWrapper>>>();
}

logger_t &CIsolateBase::Logger(void)
Expand All @@ -36,9 +41,19 @@ logger_t &CIsolateBase::Logger(void)
return *logger;
}

CIsolate::CIsolate(v8::Isolate *isolate) : CIsolateBase(isolate)
py::object CIsolateWrapper::GetCurrent(void)
{
BOOST_LOG_SEV(Logger(), trace) << "isolated wrapped";
v8::Isolate *isolate = v8::Isolate::GetCurrent();

v8::HandleScope handle_scope(isolate);

return !isolate ? py::object() : py::object(py::handle<>(py::converter::shared_ptr_to_python<CIsolateWrapper>(
CIsolateWrapperPtr(new CIsolate(isolate)))));
}

CIsolate::CIsolate(v8::Isolate *isolate) : CIsolateWrapper(isolate)
{
BOOST_LOG_SEV(Logger(), trace) << "isolate wrapped";
}

v8::Local<v8::ObjectTemplate> CIsolate::ObjectTemplate(void)
Expand All @@ -52,14 +67,14 @@ v8::Local<v8::ObjectTemplate> CIsolate::ObjectTemplate(void)
return object_template->Get(m_isolate);
}

CManagedIsolate::CManagedIsolate() : CIsolateBase(CreateIsolate())
CManagedIsolate::CManagedIsolate() : CIsolateWrapper(CreateIsolate())
{
BOOST_LOG_SEV(Logger(), trace) << "isolated created";
BOOST_LOG_SEV(Logger(), trace) << "isolate created";
}

CManagedIsolate::~CManagedIsolate(void)
{
BOOST_LOG_SEV(Logger(), trace) << "isolated destroyed";
BOOST_LOG_SEV(Logger(), trace) << "isolate destroyed";

ClearDataSlots();

Expand All @@ -80,13 +95,3 @@ v8::Isolate *CManagedIsolate::CreateIsolate()

return v8::Isolate::New(params);
}

py::object CManagedIsolate::GetCurrent(void)
{
v8::Isolate *isolate = v8::Isolate::GetCurrent();

v8::HandleScope handle_scope(isolate);

return !isolate ? py::object() : py::object(py::handle<>(boost::python::converter::shared_ptr_to_python<CIsolate>(
CIsolatePtr(new CIsolate(isolate)))));
}
49 changes: 27 additions & 22 deletions src/Isolate.h
Expand Up @@ -53,27 +53,10 @@ class CIsolateBase
logger_t &Logger(void);
};

class CIsolate : public CIsolateBase
class CIsolateWrapper : public CIsolateBase
{
public:
CIsolate(v8::Isolate *isolate);
virtual ~CIsolate() = default;

public: // Internal Properties
static CIsolate Current(void) { return CIsolate(v8::Isolate::GetCurrent()); }

v8::Local<v8::ObjectTemplate> ObjectTemplate(void);
};

class CManagedIsolate : public CIsolateBase, private boost::noncopyable
{
static v8::Isolate *CreateIsolate();

void ClearDataSlots() const;

public:
CManagedIsolate();
virtual ~CManagedIsolate(void);
protected:
CIsolateWrapper(v8::Isolate *isolate) : CIsolateBase(isolate) {}

public: // Properties
inline bool IsLocked(void) const { return v8::Locker::IsLocked(m_isolate); }
Expand Down Expand Up @@ -110,9 +93,31 @@ class CManagedIsolate : public CIsolateBase, private boost::noncopyable
}

static py::object GetCurrent(void);
};

class CIsolate : public CIsolateWrapper
{
public:
CIsolate(v8::Isolate *isolate);
virtual ~CIsolate() = default;

public: // Internal Properties
static CIsolate Current(void) { return CIsolate(v8::Isolate::GetCurrent()); }

v8::Local<v8::ObjectTemplate> ObjectTemplate(void);
};

class CManagedIsolate : public CIsolateWrapper, private boost::noncopyable
{
static v8::Isolate *CreateIsolate();

void ClearDataSlots() const;

public:
CManagedIsolate();
virtual ~CManagedIsolate(void);

static void Expose(void);
};

typedef boost::shared_ptr<CIsolate> CIsolatePtr;
typedef boost::shared_ptr<CManagedIsolate> CManagedIsolatePtr;
typedef boost::shared_ptr<CIsolateWrapper> CIsolateWrapperPtr;
46 changes: 23 additions & 23 deletions src/Locker.cpp
Expand Up @@ -4,33 +4,11 @@

bool CLocker::s_preemption = false;

void CLocker::enter(void)
{
Py_BEGIN_ALLOW_THREADS

m_locker.reset(new v8::Locker(m_isolate->GetIsolate()));

Py_END_ALLOW_THREADS
}
void CLocker::leave(void)
{
Py_BEGIN_ALLOW_THREADS

m_locker.reset();

Py_END_ALLOW_THREADS
}

bool CLocker::IsLocked()
{
return v8::Locker::IsLocked(m_isolate->GetIsolate());
}

void CLocker::Expose(void)
{
py::class_<CLocker, boost::noncopyable>("JSLocker", py::no_init)
.def(py::init<>())
.def(py::init<CIsolatePtr>((py::arg("isolate"))))
.def(py::init<CIsolateWrapperPtr>((py::arg("isolate"))))

.add_static_property("active", &v8::Locker::IsActive,
"whether Locker is being used by this V8 instance.")
Expand All @@ -49,3 +27,25 @@ void CLocker::Expose(void)
.def("enter", &CUnlocker::enter)
.def("leave", &CUnlocker::leave);
}

void CLocker::enter(void)
{
Py_BEGIN_ALLOW_THREADS

m_locker.reset(new v8::Locker(m_isolate->GetIsolate()));

Py_END_ALLOW_THREADS
}
void CLocker::leave(void)
{
Py_BEGIN_ALLOW_THREADS

m_locker.reset();

Py_END_ALLOW_THREADS
}

bool CLocker::IsLocked()
{
return v8::Locker::IsLocked(m_isolate->GetIsolate());
}

0 comments on commit 3ab967e

Please sign in to comment.