diff --git a/include/StrType.hh b/include/StrType.hh index 5e1954c1..d8199bc4 100644 --- a/include/StrType.hh +++ b/include/StrType.hh @@ -36,8 +36,6 @@ public: */ static PyObject *getPyObject(JSContext *cx, JS::HandleValue str); - static const char *getValue(JSContext *cx, JS::HandleValue str); - static PyObject *proxifyString(JSContext *cx, JS::HandleValue str); }; diff --git a/python/pythonmonkey/require.py b/python/pythonmonkey/require.py index bbb4661f..696a4fbb 100644 --- a/python/pythonmonkey/require.py +++ b/python/pythonmonkey/require.py @@ -84,8 +84,8 @@ globalThis.python.stderr.read = lambda n: sys.stderr.read(n) # Python 3.13 dramatically changed how the namespace in `exec`/`eval` works # See https://docs.python.org/3.13/whatsnew/3.13.html#defined-mutation-semantics-for-locals -globalThis.python.eval = lambda x: eval(x, None, sys._getframe(1).f_locals) -globalThis.python.exec = lambda x: exec(x, None, sys._getframe(1).f_locals) +globalThis.python.eval = lambda x: eval(str(x)[:], None, sys._getframe(1).f_locals) +globalThis.python.exec = lambda x: exec(str(x)[:], None, sys._getframe(1).f_locals) globalThis.python.getenv = os.getenv globalThis.python.paths = sys.path diff --git a/src/ExceptionType.cc b/src/ExceptionType.cc index 053f18e1..620643f9 100644 --- a/src/ExceptionType.cc +++ b/src/ExceptionType.cc @@ -105,8 +105,8 @@ JSObject *ExceptionType::toJsError(JSContext *cx, PyObject *exceptionValue, PyOb if (stackObj.get()) { JS::RootedString stackStr(cx); JS::BuildStackString(cx, nullptr, stackObj, &stackStr, 2, js::StackFormat::SpiderMonkey); - JS::RootedValue stackStrVal(cx, JS::StringValue(stackStr)); - stackStream << "\nJS Stack Trace:\n" << StrType::getValue(cx, stackStrVal); + JS::UniqueChars stackStrUtf8 = JS_EncodeStringToUTF8(cx, stackStr); + stackStream << "\nJS Stack Trace:\n" << stackStrUtf8.get(); } diff --git a/src/JSObjectProxy.cc b/src/JSObjectProxy.cc index 9e755a17..e983335b 100644 --- a/src/JSObjectProxy.cc +++ b/src/JSObjectProxy.cc @@ -36,9 +36,10 @@ JSContext *GLOBAL_CX; /**< pointer to PythonMonkey's JSContext */ bool keyToId(PyObject *key, JS::MutableHandleId idp) { if (PyUnicode_Check(key)) { // key is str type JS::RootedString idString(GLOBAL_CX); - const char *keyStr = PyUnicode_AsUTF8(key); - JS::ConstUTF8CharsZ utf8Chars(keyStr, strlen(keyStr)); - idString.set(JS_NewStringCopyUTF8Z(GLOBAL_CX, utf8Chars)); + Py_ssize_t length; + const char *keyStr = PyUnicode_AsUTF8AndSize(key, &length); + JS::UTF8Chars utf8Chars(keyStr, length); + idString.set(JS_NewStringCopyUTF8N(GLOBAL_CX, utf8Chars)); return JS_StringToId(GLOBAL_CX, idString, idp); } else if (PyLong_Check(key)) { // key is int type uint32_t keyAsInt = PyLong_AsUnsignedLong(key); // TODO raise OverflowError if the value of pylong is out of range for a unsigned long diff --git a/src/JobQueue.cc b/src/JobQueue.cc index 0ee33b39..6dcb548f 100644 --- a/src/JobQueue.cc +++ b/src/JobQueue.cc @@ -121,6 +121,7 @@ bool sendJobToMainLoop(PyObject *pyFunc) { } loop.enqueue(pyFunc); + loop._loop = nullptr; // the `Py_XDECREF` Python API call in `PyEventLoop`'s destructor will not be accessible once we hand over the GIL by `PyGILState_Release` PyGILState_Release(gstate); return true; } diff --git a/src/PyBytesProxyHandler.cc b/src/PyBytesProxyHandler.cc index 525b0068..304c37c9 100644 --- a/src/PyBytesProxyHandler.cc +++ b/src/PyBytesProxyHandler.cc @@ -415,6 +415,9 @@ bool PyBytesProxyHandler::getOwnPropertyDescriptor( PyObject *attrName = idToKey(cx, id); PyObject *self = JS::GetMaybePtrFromReservedSlot(proxy, PyObjectSlot); PyObject *item = PyObject_GetAttr(self, attrName); + if (!item && PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); // clear error, we will be returning undefined in this case + } return handleGetOwnPropertyDescriptor(cx, id, desc, item); } diff --git a/src/PyDictProxyHandler.cc b/src/PyDictProxyHandler.cc index 72d2decf..4295d2c4 100644 --- a/src/PyDictProxyHandler.cc +++ b/src/PyDictProxyHandler.cc @@ -56,7 +56,7 @@ bool PyDictProxyHandler::getOwnPropertyDescriptor( ) const { PyObject *attrName = idToKey(cx, id); PyObject *self = JS::GetMaybePtrFromReservedSlot(proxy, PyObjectSlot); - PyObject *item = PyDict_GetItemWithError(self, attrName); + PyObject *item = PyDict_GetItemWithError(self, attrName); // returns NULL without an exception set if the key wasn’t present. return handleGetOwnPropertyDescriptor(cx, id, desc, item); } diff --git a/src/PyIterableProxyHandler.cc b/src/PyIterableProxyHandler.cc index 5de9ec3a..0a95603f 100644 --- a/src/PyIterableProxyHandler.cc +++ b/src/PyIterableProxyHandler.cc @@ -79,7 +79,7 @@ static bool toPrimitive(JSContext *cx, unsigned argc, JS::Value *vp) { _PyUnicodeWriter writer; _PyUnicodeWriter_Init(&writer); - + PyObject *s = PyObject_Repr(self); if (s == nullptr) { @@ -95,8 +95,8 @@ static bool toPrimitive(JSContext *cx, unsigned argc, JS::Value *vp) { return true; } - PyObject* repr = _PyUnicodeWriter_Finish(&writer); - + PyObject *repr = _PyUnicodeWriter_Finish(&writer); + args.rval().set(jsTypeFactory(cx, repr)); return true; } @@ -262,7 +262,7 @@ bool PyIterableProxyHandler::getOwnPropertyDescriptor( // symbol property if (id.isSymbol()) { JS::RootedSymbol rootedSymbol(cx, id.toSymbol()); - JS::SymbolCode symbolCode = JS::GetSymbolCode(rootedSymbol); + JS::SymbolCode symbolCode = JS::GetSymbolCode(rootedSymbol); if (symbolCode == JS::SymbolCode::iterator) { JSFunction *newFunction = JS_NewFunction(cx, iterable_values, 0, 0, NULL); @@ -275,7 +275,7 @@ bool PyIterableProxyHandler::getOwnPropertyDescriptor( ) )); return true; - } + } else if (symbolCode == JS::SymbolCode::toPrimitive) { JSFunction *newFunction = JS_NewFunction(cx, toPrimitive, 0, 0, nullptr); if (!newFunction) return false; @@ -293,6 +293,9 @@ bool PyIterableProxyHandler::getOwnPropertyDescriptor( PyObject *attrName = idToKey(cx, id); PyObject *self = JS::GetMaybePtrFromReservedSlot(proxy, PyObjectSlot); PyObject *item = PyObject_GetAttr(self, attrName); + if (!item && PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); // clear error, we will be returning undefined in this case + } return handleGetOwnPropertyDescriptor(cx, id, desc, item); } \ No newline at end of file diff --git a/src/PyListProxyHandler.cc b/src/PyListProxyHandler.cc index f03fb67d..72da8d4e 100644 --- a/src/PyListProxyHandler.cc +++ b/src/PyListProxyHandler.cc @@ -428,17 +428,16 @@ static bool array_fill(JSContext *cx, unsigned argc, JS::Value *vp) { JS::RootedValue fillValue(cx, args[0].get()); PyObject *fillValueItem = pyTypeFactory(cx, fillValue); - bool setItemCalled = false; for (int index = actualStart; index < actualEnd; index++) { - setItemCalled = true; + // Since each call of `PyList_SetItem` steals a reference (even if its to the same object), + // We need multiple references to it for it to steal. + Py_INCREF(fillValueItem); if (PyList_SetItem(self, index, fillValueItem) < 0) { return false; } } - if (!setItemCalled) { - Py_DECREF(fillValueItem); - } + Py_DECREF(fillValueItem); // return ref to self args.rval().set(jsTypeFactory(cx, self)); @@ -550,12 +549,7 @@ static bool array_concat(JSContext *cx, unsigned argc, JS::Value *vp) { PyObject *self = JS::GetMaybePtrFromReservedSlot(proxy, PyObjectSlot); Py_ssize_t selfSize = PyList_GET_SIZE(self); - - PyObject *result = PyList_New(selfSize); - - for (Py_ssize_t index = 0; index < selfSize; index++) { - PyList_SetItem(result, index, PyList_GetItem(self, index)); - } + PyObject *result = PyList_GetSlice(self, 0, selfSize); unsigned numArgs = args.length(); JS::RootedValue elementVal(cx); diff --git a/src/PyObjectProxyHandler.cc b/src/PyObjectProxyHandler.cc index 7f5e4b70..0671774c 100644 --- a/src/PyObjectProxyHandler.cc +++ b/src/PyObjectProxyHandler.cc @@ -143,8 +143,8 @@ bool PyObjectProxyHandler::getOwnPropertyDescriptor( PyObject *attrName = idToKey(cx, id); PyObject *self = JS::GetMaybePtrFromReservedSlot(proxy, PyObjectSlot); PyObject *item = PyObject_GetAttr(self, attrName); - if (!item) { // clear error, we will be returning undefined in this case - PyErr_Clear(); + if (!item && PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); // clear error, we will be returning undefined in this case } return handleGetOwnPropertyDescriptor(cx, id, desc, item); diff --git a/src/StrType.cc b/src/StrType.cc index 714bed7d..7df301bd 100644 --- a/src/StrType.cc +++ b/src/StrType.cc @@ -53,6 +53,16 @@ static bool containsSurrogatePair(const char16_t *chars, size_t length) { return false; } +/** + * @brief check if the Latin-1 encoded `chars` only contain ascii characters + */ +static bool containsOnlyAscii(const JS::Latin1Char *chars, size_t length) { + for (size_t i = 0; i < length; i++) { + if (chars[i] >= 128) return false; + } + return true; +} + /** * @brief creates new UCS4-encoded pyObject string. This must be called by the user if the original JSString contains any surrogate pairs * @@ -134,6 +144,16 @@ PyObject *StrType::proxifyString(JSContext *cx, JS::HandleValue strVal) { PY_UNICODE_OBJECT_WSTR_LENGTH(pyString) = 0; PY_UNICODE_OBJECT_READY(pyString) = 1; #endif + + #ifdef Py_DEBUG + // In a debug build of CPython, it needs to be a well-formed PyUnicodeObject, otherwise a `_PyObject_AssertFailed` error will be raised. + // See: `_PyUnicode_CheckConsistency` https://github.com/python/cpython/blob/v3.11.3/Objects/unicodeobject.c#L594-L600, #L552-L553 + if (containsOnlyAscii(chars, length)) { + PY_UNICODE_OBJECT_STATE(pyString).ascii = 1; + PY_UNICODE_OBJECT_UTF8(pyString) = (char *)chars; // XXX: most APIs (e.g. PyUnicode_AsUTF8) assume this is a \0 terminated string + PY_UNICODE_OBJECT_UTF8_LENGTH(pyString) = length; + } + #endif } else { // utf16 spidermonkey, ucs2 python const char16_t *chars = JS::GetTwoByteLinearStringChars(nogc, lstr); @@ -192,10 +212,3 @@ PyObject *StrType::getPyObject(JSContext *cx, JS::HandleValue str) { return proxifyString(cx, str); } - -const char *StrType::getValue(JSContext *cx, JS::HandleValue str) { - PyObject *pyString = proxifyString(cx, str); - const char *value = PyUnicode_AsUTF8(pyString); - Py_DECREF(pyString); - return value; -} \ No newline at end of file diff --git a/src/modules/pythonmonkey/pythonmonkey.cc b/src/modules/pythonmonkey/pythonmonkey.cc index 597f937b..8408b594 100644 --- a/src/modules/pythonmonkey/pythonmonkey.cc +++ b/src/modules/pythonmonkey/pythonmonkey.cc @@ -303,14 +303,23 @@ PyTypeObject JSObjectItemsProxyType = { }; static void cleanup() { + // Clean up the PythonMonkey module Py_XDECREF(PythonMonkey_Null); Py_XDECREF(PythonMonkey_BigInt); + + // Clean up SpiderMonkey delete autoRealm; delete global; - if (GLOBAL_CX) JS_DestroyContext(GLOBAL_CX); + if (GLOBAL_CX) { + JS_DestroyContext(GLOBAL_CX); + GLOBAL_CX = nullptr; + } delete JOB_QUEUE; JS_ShutDown(); } +static void cleanup(PyObject *) { + cleanup(); +} static PyObject *collect(PyObject *self, PyObject *args) { JS_GC(GLOBAL_CX); @@ -325,7 +334,7 @@ static bool getEvalOption(PyObject *evalOptions, const char *optionName, const c value = PyDict_GetItemString(evalOptions, optionName); } if (value && value != Py_None) { - *s_p = PyUnicode_AsUTF8(value); + *s_p = PyUnicode_AsUTF8(PyUnicode_FromObject(value)); } return value != NULL && value != Py_None; } @@ -443,7 +452,8 @@ static PyObject *eval(PyObject *self, PyObject *args) { #endif if (!getEvalOption(evalOptions, "filename", &s)) { if (filename && PyUnicode_Check(filename)) { - options.setFile(PyUnicode_AsUTF8(filename)); + PyObject *filenameStr = PyUnicode_FromObject(filename); // needs a strict Python str object (not a subtype) + options.setFile(PyUnicode_AsUTF8(filenameStr)); } } /* filename */ } /* fromPythonFrame */ @@ -454,8 +464,9 @@ static PyObject *eval(PyObject *self, PyObject *args) { JS::Rooted rval(GLOBAL_CX); if (code) { JS::SourceText source; - const char *codeChars = PyUnicode_AsUTF8(code); - if (!source.init(GLOBAL_CX, codeChars, strlen(codeChars), JS::SourceOwnership::Borrowed)) { + Py_ssize_t codeLength; + const char *codeChars = PyUnicode_AsUTF8AndSize(code, &codeLength); + if (!source.init(GLOBAL_CX, codeChars, codeLength, JS::SourceOwnership::Borrowed)) { setSpiderMonkeyException(GLOBAL_CX); return NULL; } @@ -514,9 +525,10 @@ static PyObject *isCompilableUnit(PyObject *self, PyObject *args) { return NULL; } - const char *bufferUtf8 = PyUnicode_AsUTF8(item); + Py_ssize_t bufferLength; + const char *bufferUtf8 = PyUnicode_AsUTF8AndSize(item, &bufferLength); - if (JS_Utf8BufferIsCompilableUnit(GLOBAL_CX, *global, bufferUtf8, strlen(bufferUtf8))) { + if (JS_Utf8BufferIsCompilableUnit(GLOBAL_CX, *global, bufferUtf8, bufferLength)) { Py_RETURN_TRUE; } else { Py_RETURN_FALSE; @@ -552,7 +564,6 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void) PyErr_SetString(SpiderMonkeyError, "Spidermonkey could not be initialized."); return NULL; } - Py_AtExit(cleanup); GLOBAL_CX = JS_NewContext(JS::DefaultHeapMaxBytes); if (!GLOBAL_CX) { @@ -645,6 +656,16 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void) if (pyModule == NULL) return NULL; + // Clean up SpiderMonkey when the PythonMonkey module gets destroyed (module.___cleanup is GCed) + // The `cleanup` function will be called automatically when this PyCapsule gets GCed + // We cannot use `Py_AtExit(cleanup);` because the GIL is unavailable after Python finalization, no more Python APIs can be called. + PyObject *autoDestructor = PyCapsule_New(&pythonmonkey, NULL, cleanup); + if (PyModule_AddObject(pyModule, "___cleanup", autoDestructor) < 0) { + Py_DECREF(autoDestructor); + Py_DECREF(pyModule); + return NULL; + } + Py_INCREF(&NullType); if (PyModule_AddObject(pyModule, "null", (PyObject *)&NullType) < 0) { Py_DECREF(&NullType); diff --git a/src/setSpiderMonkeyException.cc b/src/setSpiderMonkeyException.cc index d380d87c..6d9d10fa 100644 --- a/src/setSpiderMonkeyException.cc +++ b/src/setSpiderMonkeyException.cc @@ -66,8 +66,8 @@ PyObject *getExceptionString(JSContext *cx, const JS::ExceptionStack &exceptionS if (stackObj.get()) { JS::RootedString stackStr(cx); BuildStackString(cx, nullptr, stackObj, &stackStr, 2, js::StackFormat::SpiderMonkey); - JS::RootedValue stackStrVal(cx, JS::StringValue(stackStr)); - outStrStream << "Stack Trace:\n" << StrType::getValue(cx, stackStrVal); + JS::UniqueChars stackStrUtf8 = JS_EncodeStringToUTF8(cx, stackStr); + outStrStream << "Stack Trace:\n" << stackStrUtf8.get(); } }