Skip to content

Commit 935e9e9

Browse files
committed
fix: the string corruption bug
String buffers memory would be moved around during a minor GC (nursery collection). The string buffer pointer obtained by `JS::Get{Latin1,TwoByte}LinearStringChars` remains valid only as long as no GC happens. Even though `JS::PersistentRootedValue` does keep the string buffer from being garbage-collected, it does not guarantee the buffer would not be moved elsewhere during a GC, and so invalidates the string buffer pointer.
1 parent 0f3039b commit 935e9e9

File tree

1 file changed

+29
-14
lines changed

1 file changed

+29
-14
lines changed

src/modules/pythonmonkey/pythonmonkey.cc

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,39 @@
4848

4949
JS::PersistentRootedObject jsFunctionRegistry;
5050

51+
/**
52+
* @brief During a GC, string buffers may have moved, so we need to re-point our JSStringProxies
53+
* The char buffer pointer obtained by previous `JS::Get{Latin1,TwoByte}LinearStringChars` calls remains valid only as long as no GC occurs.
54+
*/
55+
void updateCharBufferPointers() {
56+
if (_Py_IsFinalizing()) {
57+
return; // do not move char pointers around if python is finalizing
58+
}
59+
60+
JS::AutoCheckCannotGC nogc;
61+
for (const JSStringProxy *jsStringProxy: jsStringProxies) {
62+
JSLinearString *str = JS_ASSERT_STRING_IS_LINEAR(jsStringProxy->jsString->toString());
63+
void *updatedCharBufPtr; // pointer to the moved char buffer after a GC
64+
if (JS::LinearStringHasLatin1Chars(str)) {
65+
updatedCharBufPtr = (void *)JS::GetLatin1LinearStringChars(nogc, str);
66+
} else { // utf16 / ucs2 string
67+
updatedCharBufPtr = (void *)JS::GetTwoByteLinearStringChars(nogc, str);
68+
}
69+
((PyUnicodeObject *)(jsStringProxy))->data.any = updatedCharBufPtr;
70+
}
71+
}
72+
5173
void pythonmonkeyGCCallback(JSContext *cx, JSGCStatus status, JS::GCReason reason, void *data) {
5274
if (status == JSGCStatus::JSGC_END) {
5375
JS::ClearKeptObjects(GLOBAL_CX);
5476
while (JOB_QUEUE->runFinalizationRegistryCallbacks(GLOBAL_CX));
77+
updateCharBufferPointers();
78+
}
79+
}
5580

56-
if (_Py_IsFinalizing()) {
57-
return; // do not move char pointers around if python is finalizing
58-
}
59-
60-
JS::AutoCheckCannotGC nogc;
61-
for (const JSStringProxy *jsStringProxy: jsStringProxies) { // char buffers may have moved, so we need to re-point our JSStringProxies
62-
JSLinearString *str = (JSLinearString *)(jsStringProxy->jsString->toString()); // jsString is guaranteed to be linear
63-
if (JS::LinearStringHasLatin1Chars(str)) {
64-
(((PyUnicodeObject *)(jsStringProxy))->data.any) = (void *)JS::GetLatin1LinearStringChars(nogc, str);
65-
}
66-
else { // utf16 / ucs2 string
67-
(((PyUnicodeObject *)(jsStringProxy))->data.any) = (void *)JS::GetTwoByteLinearStringChars(nogc, str);
68-
}
69-
}
81+
void nurseryCollectionCallback(JSContext *cx, JS::GCNurseryProgress progress, JS::GCReason reason, void *data) {
82+
if (progress == JS::GCNurseryProgress::GC_NURSERY_COLLECTION_END) {
83+
updateCharBufferPointers();
7084
}
7185
}
7286

@@ -565,6 +579,7 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void)
565579
JS_SetGCParameter(GLOBAL_CX, JSGC_MAX_BYTES, (uint32_t)-1);
566580

567581
JS_SetGCCallback(GLOBAL_CX, pythonmonkeyGCCallback, NULL);
582+
JS::AddGCNurseryCollectionCallback(GLOBAL_CX, nurseryCollectionCallback, NULL);
568583

569584
JS::RealmCreationOptions creationOptions = JS::RealmCreationOptions();
570585
JS::RealmBehaviors behaviours = JS::RealmBehaviors();

0 commit comments

Comments
 (0)