Merge pull request #428 from Distributive-Network/Xmader/fix/string-c…

…orruption Fix the string corruption bug
Distributive-Network · Sep 6, 2024 · 2d19e25 · 2d19e25
2 parents c140da7 + 935e9e9
commit 2d19e25
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 16 deletions.
diff --git a/src/PyObjectProxyHandler.cc b/src/PyObjectProxyHandler.cc
@@ -44,8 +44,9 @@ bool PyObjectProxyHandler::handleGetOwnPropertyDescriptor(JSContext *cx, JS::Han
   JS::MutableHandle<mozilla::Maybe<JS::PropertyDescriptor>> desc, PyObject *item) {
   // see if we're calling a function
   if (id.isString()) {
-    JS::RootedString idString(cx, id.toString());
-    const char *methodName = JS_EncodeStringToUTF8(cx, idString).get();
+    JS::UniqueChars idString = JS_EncodeStringToUTF8(cx, JS::RootedString(cx, id.toString()));
+    const char *methodName = idString.get();
+
     if (!strcmp(methodName, "toString") || !strcmp(methodName, "toLocaleString") || !strcmp(methodName, "valueOf")) {
       JS::RootedObject objectPrototype(cx);
       if (!JS_GetClassPrototype(cx, JSProto_Object, &objectPrototype)) {

diff --git a/src/modules/pythonmonkey/pythonmonkey.cc b/src/modules/pythonmonkey/pythonmonkey.cc
@@ -48,25 +48,39 @@
 
 JS::PersistentRootedObject jsFunctionRegistry;
 
+/**
+ * @brief During a GC, string buffers may have moved, so we need to re-point our JSStringProxies
+ * The char buffer pointer obtained by previous `JS::Get{Latin1,TwoByte}LinearStringChars` calls remains valid only as long as no GC occurs.
+ */
+void updateCharBufferPointers() {
+  if (_Py_IsFinalizing()) {
+    return; // do not move char pointers around if python is finalizing
+  }
+
+  JS::AutoCheckCannotGC nogc;
+  for (const JSStringProxy *jsStringProxy: jsStringProxies) {
+    JSLinearString *str = JS_ASSERT_STRING_IS_LINEAR(jsStringProxy->jsString->toString());
+    void *updatedCharBufPtr; // pointer to the moved char buffer after a GC
+    if (JS::LinearStringHasLatin1Chars(str)) {
+      updatedCharBufPtr = (void *)JS::GetLatin1LinearStringChars(nogc, str);
+    } else { // utf16 / ucs2 string
+      updatedCharBufPtr = (void *)JS::GetTwoByteLinearStringChars(nogc, str);
+    }
+    ((PyUnicodeObject *)(jsStringProxy))->data.any = updatedCharBufPtr;
+  }
+}
+
 void pythonmonkeyGCCallback(JSContext *cx, JSGCStatus status, JS::GCReason reason, void *data) {
   if (status == JSGCStatus::JSGC_END) {
     JS::ClearKeptObjects(GLOBAL_CX);
     while (JOB_QUEUE->runFinalizationRegistryCallbacks(GLOBAL_CX));
+    updateCharBufferPointers();
+  }
+}
 
-    if (_Py_IsFinalizing()) {
-      return; // do not move char pointers around if python is finalizing
-    }
-
-    JS::AutoCheckCannotGC nogc;
-    for (const JSStringProxy *jsStringProxy: jsStringProxies) { // char buffers may have moved, so we need to re-point our JSStringProxies
-      JSLinearString *str = (JSLinearString *)(jsStringProxy->jsString->toString()); // jsString is guaranteed to be linear
-      if (JS::LinearStringHasLatin1Chars(str)) {
-        (((PyUnicodeObject *)(jsStringProxy))->data.any) = (void *)JS::GetLatin1LinearStringChars(nogc, str);
-      }
-      else { // utf16 / ucs2 string
-        (((PyUnicodeObject *)(jsStringProxy))->data.any) = (void *)JS::GetTwoByteLinearStringChars(nogc, str);
-      }
-    }
+void nurseryCollectionCallback(JSContext *cx, JS::GCNurseryProgress progress, JS::GCReason reason, void *data) {
+  if (progress == JS::GCNurseryProgress::GC_NURSERY_COLLECTION_END) {
+    updateCharBufferPointers();
   }
 }
 
@@ -565,6 +579,7 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void)
   JS_SetGCParameter(GLOBAL_CX, JSGC_MAX_BYTES, (uint32_t)-1);
 
   JS_SetGCCallback(GLOBAL_CX, pythonmonkeyGCCallback, NULL);
+  JS::AddGCNurseryCollectionCallback(GLOBAL_CX, nurseryCollectionCallback, NULL);
 
   JS::RealmCreationOptions creationOptions = JS::RealmCreationOptions();
   JS::RealmBehaviors behaviours = JS::RealmBehaviors();