Fix remaining memory bugs

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);
 };
 

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
 

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();
   }
 
 

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

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;
 }

src/PyBytesProxyHandler.cc

@@ -415,6 +415,9 @@ bool PyBytesProxyHandler::getOwnPropertyDescriptor(
   PyObject *attrName = idToKey(cx, id);
   PyObject *self = JS::GetMaybePtrFromReservedSlot<PyObject>(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);
 }

src/PyDictProxyHandler.cc

@@ -56,7 +56,7 @@ bool PyDictProxyHandler::getOwnPropertyDescriptor(
 ) const {
   PyObject *attrName = idToKey(cx, id);
   PyObject *self = JS::GetMaybePtrFromReservedSlot<PyObject>(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);
 }

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<PyObject>(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);
 }

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<PyObject>(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);

src/PyObjectProxyHandler.cc

@@ -143,8 +143,8 @@ bool PyObjectProxyHandler::getOwnPropertyDescriptor(
   PyObject *attrName = idToKey(cx, id);
   PyObject *self = JS::GetMaybePtrFromReservedSlot<PyObject>(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);

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;
-}

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<JS::Value> rval(GLOBAL_CX);
   if (code) {
     JS::SourceText<mozilla::Utf8Unit> 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);

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();
     }
   }