Skip to content

BUG: fix memory leak in JSON datetime serialization #62217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 3, 2025
+38 −10
Merged

BUG: fix memory leak in JSON datetime serialization #62217

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
791bb41
fix: memory leak in JSON datetime serialization
Alvaro-Kothe Aug 29, 2025
f7b2f5b
fix: flag to free `cStr` in `Object_getBigNumStringValue`
Alvaro-Kothe Aug 29, 2025
f324bd8
fix: use dynamic allocated memory to store strings
Alvaro-Kothe Sep 2, 2025
15c19c0
fix: replace PyObject_Malloc with PyMem_Malloc
Alvaro-Kothe Sep 2, 2025
1ea0283
fix: drop `const` qualifier in `cStr`
Alvaro-Kothe Sep 2, 2025
0925e6b
fix: remove void pointer case in PyMem_Free
Alvaro-Kothe Sep 2, 2025
62b0e26
fix: create const for default `cStr` size
Alvaro-Kothe Sep 2, 2025
0f20b7d
try to fix ci error
Alvaro-Kothe Sep 2, 2025
05f4926
hack: go back to use `PyObject_*` because of CI errors
Alvaro-Kothe Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 35 additions & 9 deletions pandas/_libs/src/vendored/ujson/python/objToJSON.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ static const char *PyDateTimeToIsoCallback(JSOBJ obj, JSONTypeContext *tc,
}

NPY_DATETIMEUNIT base = ((PyObjectEncoder *)tc->encoder)->datetimeUnit;
return PyDateTimeToIso(obj, base, len);
GET_TC(tc)->cStr = PyDateTimeToIso(obj, base, len);
return GET_TC(tc)->cStr;
}

static const char *PyTimeToJSON(JSOBJ _obj, JSONTypeContext *tc,
Expand Down Expand Up @@ -1007,16 +1008,24 @@ static const char *List_iterGetName(JSOBJ Py_UNUSED(obj),
//=============================================================================
static void Index_iterBegin(JSOBJ Py_UNUSED(obj), JSONTypeContext *tc) {
GET_TC(tc)->index = 0;
GET_TC(tc)->cStr = PyObject_Malloc(20);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its probably better to create a static const variable for a size within the module, rather than repeating the hard-coded 20 multiple times

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created the variable CSTR_SIZE

if (!GET_TC(tc)->cStr) {
PyErr_NoMemory();
}
}

static int Index_iterNext(JSOBJ obj, JSONTypeContext *tc) {
const Py_ssize_t index = GET_TC(tc)->index;
Py_XDECREF(GET_TC(tc)->itemValue);
if (!GET_TC(tc)->cStr) {
return 0;
}

if (index == 0) {
GET_TC(tc)->cStr = "name";
strcpy((char *)GET_TC(tc)->cStr, "name");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you want to use strcpy here you will need to drop the const qualifier on the struct member; casting away const-ness like this is going to trigger undefined behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the const qualifier.

GET_TC(tc)->itemValue = PyObject_GetAttrString(obj, "name");
} else if (index == 1) {
GET_TC(tc)->cStr = "data";
strcpy((char *)GET_TC(tc)->cStr, "data");
GET_TC(tc)->itemValue = get_values(obj);
if (!GET_TC(tc)->itemValue) {
return 0;
Expand Down Expand Up @@ -1049,19 +1058,27 @@ static void Series_iterBegin(JSOBJ Py_UNUSED(obj), JSONTypeContext *tc) {
PyObjectEncoder *enc = (PyObjectEncoder *)tc->encoder;
GET_TC(tc)->index = 0;
enc->outputFormat = VALUES; // for contained series
GET_TC(tc)->cStr = PyObject_Malloc(20);
if (!GET_TC(tc)->cStr) {
PyErr_NoMemory();
}
}

static int Series_iterNext(JSOBJ obj, JSONTypeContext *tc) {
const Py_ssize_t index = GET_TC(tc)->index;
Py_XDECREF(GET_TC(tc)->itemValue);
if (!GET_TC(tc)->cStr) {
return 0;
}

if (index == 0) {
GET_TC(tc)->cStr = "name";
strcpy((char *)GET_TC(tc)->cStr, "name");
GET_TC(tc)->itemValue = PyObject_GetAttrString(obj, "name");
} else if (index == 1) {
GET_TC(tc)->cStr = "index";
strcpy((char *)GET_TC(tc)->cStr, "index");
GET_TC(tc)->itemValue = PyObject_GetAttrString(obj, "index");
} else if (index == 2) {
GET_TC(tc)->cStr = "data";
strcpy((char *)GET_TC(tc)->cStr, "data");
GET_TC(tc)->itemValue = get_values(obj);
if (!GET_TC(tc)->itemValue) {
return 0;
Expand Down Expand Up @@ -1096,19 +1113,27 @@ static void DataFrame_iterBegin(JSOBJ Py_UNUSED(obj), JSONTypeContext *tc) {
PyObjectEncoder *enc = (PyObjectEncoder *)tc->encoder;
GET_TC(tc)->index = 0;
enc->outputFormat = VALUES; // for contained series & index
GET_TC(tc)->cStr = PyObject_Malloc(20);
if (!GET_TC(tc)->cStr) {
PyErr_NoMemory();
}
}

static int DataFrame_iterNext(JSOBJ obj, JSONTypeContext *tc) {
const Py_ssize_t index = GET_TC(tc)->index;
Py_XDECREF(GET_TC(tc)->itemValue);
if (!GET_TC(tc)->cStr) {
return 0;
}

if (index == 0) {
GET_TC(tc)->cStr = "columns";
strcpy((char *)GET_TC(tc)->cStr, "columns");
GET_TC(tc)->itemValue = PyObject_GetAttrString(obj, "columns");
} else if (index == 1) {
GET_TC(tc)->cStr = "index";
strcpy((char *)GET_TC(tc)->cStr, "index");
GET_TC(tc)->itemValue = PyObject_GetAttrString(obj, "index");
} else if (index == 2) {
GET_TC(tc)->cStr = "data";
strcpy((char *)GET_TC(tc)->cStr, "data");
Py_INCREF(obj);
GET_TC(tc)->itemValue = obj;
} else {
Expand Down Expand Up @@ -1880,6 +1905,7 @@ static void Object_endTypeContext(JSOBJ Py_UNUSED(obj), JSONTypeContext *tc) {
GET_TC(tc)->rowLabels = NULL;
NpyArr_freeLabels(GET_TC(tc)->columnLabels, GET_TC(tc)->columnLabelsLen);
GET_TC(tc)->columnLabels = NULL;
PyObject_Free((void *)GET_TC(tc)->cStr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too sure about this approach - generally the CPython API only wants you to call this function when you've allocated with the corresponding function(s)

https://docs.python.org/3/c-api/memory.html#c.PyObject_Free

Frees the memory block pointed to by p, which must have been returned by a previous call to PyObject_Malloc(), PyObject_Realloc() or PyObject_Calloc(). Otherwise, or if PyObject_Free(p) has been called before, undefined behavior occurs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry I misread your PR. I see you allocate 20 bytes to cStr, so this is clean in that regards. However, I think you want to use the PyMem_ functions since we are dealing with raw C constructs and not Python objects, so maybe replace this with PyMem_Free, any PyObject_Malloc calls with PyMem_Malloc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm missing something, PyObject_Free() is being called on memory that's been allocated with PyObject_Malloc()...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why cast to a void pointer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was because of the const qualifier in cStr, but since I removed this qualifier, I am no longer casting.

GET_TC(tc)->cStr = NULL;
PyObject_Free(tc->prv);
tc->prv = NULL;
Expand Down
Loading