-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
gh-146152: Fix memory leak in _json encoder error paths #146164
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
0597100
9ec9348
cd9cc6f
5428b5e
b68b52b
cafe9ba
a4be61f
f7bdb6f
f09ce9a
5359ecd
753a88e
6498f26
6b69fea
81332c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,12 @@ | ||
| import weakref | ||
| import sys | ||
| from test import support | ||
| from test.test_json import PyTest, CTest | ||
|
|
||
|
|
||
| class JSONTestObject: | ||
| pass | ||
|
|
||
okiemute04 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is still deleted. |
||
| class TestRecursion: | ||
| def test_listrecursion(self): | ||
| x = [] | ||
|
|
@@ -115,6 +116,36 @@ def default(self, o): | |
| with support.infinite_recursion(1000): | ||
| EndlessJSONEncoder(check_circular=False).encode(5j) | ||
|
|
||
| @support.skip_if_unlimited_stack_size | ||
| @support.skip_emscripten_stack_overflow() | ||
| @support.skip_wasi_stack_overflow() | ||
| def test_memory_leak_on_recursion_error(self): | ||
| # Test that no memory leak occurs when a RecursionError is raised. | ||
| class LeakTestObj: | ||
| pass | ||
|
|
||
| weak_refs = [] | ||
| def default(obj): | ||
| if isinstance(obj, LeakTestObj): | ||
| new_obj = LeakTestObj() | ||
| weak_refs.append(weakref.ref(new_obj)) | ||
| return [new_obj] | ||
| raise TypeError | ||
|
|
||
| depth = min(500, sys.getrecursionlimit() - 10) | ||
| obj = LeakTestObj() | ||
| for _ in range(depth): | ||
| obj = [obj] | ||
|
|
||
| with support.infinite_recursion(): | ||
| with self.assertRaises(RecursionError): | ||
| self.dumps(obj, default=default) | ||
|
|
||
| support.gc_collect() | ||
| self.assertTrue(weak_refs, "No objects were created to track") | ||
| for i, ref in enumerate(weak_refs): | ||
| self.assertIsNone(ref(), f"object {i} still alive") | ||
picnixz marked this conversation as resolved.
Show resolved
Hide resolved
okiemute04 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| class TestPyRecursion(TestRecursion, PyTest): pass | ||
| class TestCRecursion(TestRecursion, CTest): pass | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| Fix a memory leak in the :mod:`json` module when a RecursionError occurs | ||
| during encoding. Previously, objects created in the ``default()`` function | ||
| while recursively encoding JSON could remain alive due to being tracked | ||
| indefinitely in the encoder's internal circular-reference dictionary. This | ||
| change ensures such objects are properly freed after the exception. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1632,8 +1632,17 @@ encoder_listencode_obj(PyEncoderObject *s, PyUnicodeWriter *writer, | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (_Py_EnterRecursiveCall(" while encoding a JSON object")) { | ||||||||||||||||||||||||||
| if (ident != NULL) { | ||||||||||||||||||||||||||
| int del_rv = PyDict_DelItem(s->markers, ident); | ||||||||||||||||||||||||||
| Py_DECREF(ident); | ||||||||||||||||||||||||||
| if (del_rv < 0) { | ||||||||||||||||||||||||||
| Py_DECREF(newobj); | ||||||||||||||||||||||||||
| return -1; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+1635
to
+1643
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Actually since we are anyway falling through the return -1 case we can simplify this as follows. Then keep the XDECREF for indent. However explicitly suppress the error code of DelItem. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Py_DECREF(newobj); | ||||||||||||||||||||||||||
| Py_XDECREF(ident); | ||||||||||||||||||||||||||
| return -1; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| rv = encoder_listencode_obj(s, writer, newobj, indent_level, indent_cache); | ||||||||||||||||||||||||||
|
|
@@ -1642,15 +1651,21 @@ encoder_listencode_obj(PyEncoderObject *s, PyUnicodeWriter *writer, | |||||||||||||||||||||||||
| Py_DECREF(newobj); | ||||||||||||||||||||||||||
| if (rv) { | ||||||||||||||||||||||||||
| _PyErr_FormatNote("when serializing %T object", obj); | ||||||||||||||||||||||||||
okiemute04 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||
| Py_XDECREF(ident); | ||||||||||||||||||||||||||
| if (ident != NULL) { | ||||||||||||||||||||||||||
| int del_rv = PyDict_DelItem(s->markers, ident); | ||||||||||||||||||||||||||
| Py_DECREF(ident); | ||||||||||||||||||||||||||
| if (del_rv < 0) { | ||||||||||||||||||||||||||
| return -1; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+1654
to
+1660
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one can benefit from the same optimization as my other comment as we anyway return -1. |
||||||||||||||||||||||||||
| return -1; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (ident != NULL) { | ||||||||||||||||||||||||||
| if (PyDict_DelItem(s->markers, ident)) { | ||||||||||||||||||||||||||
| Py_XDECREF(ident); | ||||||||||||||||||||||||||
| int del_rv = PyDict_DelItem(s->markers, ident); | ||||||||||||||||||||||||||
| Py_DECREF(ident); | ||||||||||||||||||||||||||
| if (del_rv < 0) { | ||||||||||||||||||||||||||
| return -1; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| Py_XDECREF(ident); | ||||||||||||||||||||||||||
|
Comment on lines
-1649
to
-1653
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part does not need modifications..It was already properly doing its job. |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return rv; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave this blank line.