gh-146152: Fix memory leak in _json encoder error paths by okiemute04 · Pull Request #146164 · python/cpython

okiemute04 · 2026-03-19T11:16:38Z

Add PyDict_DelItem calls to remove objects from the markers dict
on all error paths in encoder_listencode_obj. Previously, objects
were only removed on the success path, causing memory leaks when:

default() raises an exception
RecursionError occurs
Nested encoding fails

Issue: _json: stale markers entries on error paths in encoder_listencode_obj #146152

@raminfp

Only clean up markers on the RecursionError path (PATH B), where objects accumulate during stack unwinding. Other error paths are safe because the markers dict is local and will be freed. Based on review feedback from @raminfp and @serhiy-storchaka.

serhiy-storchaka

I am not sure that this works. Please add a test.

Define default() which creates a new object, adds a weak reference to it to the list, and returns that object.
Call JSON encoding, and after catching an exception, call test.support.gc_collect(), then test that all weak references in the list return None.

okiemute04 · 2026-03-20T09:49:03Z

@serhiy-storchaka Thanks for the review! I've added a test to test_recursion.py that creates objects tracked with weak references, triggers a RecursionError, and verifies all objects are freed after garbage collection.

serhiy-storchaka

Thank you for your update, @okiemute04.

The test does not fail with the current code. I am not sure it tests what it should test.

serhiy-storchaka · 2026-03-22T11:13:50Z

Lib/test/test_json/test_recursion.py

+        with self.assertRaises(RecursionError):
+            self.dumps(obj, default=default)
+
+        gc.collect()


Use test.support.gc_collect() -- it calls gc.collect() several times.

@serhiy-storchaka thanks for your response, i will update it to use test.support.gc_collect() for more thorough garbage collection and test again

@serhiy-storchaka updated the test to use support.gc_collect(). I did test locally, and it works fine.

picnixz · 2026-03-22T17:16:28Z

Lib/test/test_json/test_recursion.py

+import weakref
+



Can you put this import before the from imports (and only leave two blank lines after the from imports?)

picnixz · 2026-03-22T17:17:12Z

Lib/test/test_json/test_recursion.py

+            self.assertIsNone(ref(),
+                f"Object {i} still alive - memory leak detected!")


Suggested change

self.assertIsNone(ref(),

f"Object {i} still alive - memory leak detected!")

self.assertIsNone(ref(), f"object {i} is still alive")

picnixz · 2026-03-22T17:17:38Z

Modules/_json.c

        Py_DECREF(newobj);
        if (rv) {
            _PyErr_FormatNote("when serializing %T object", obj);
+


Please revert this

picnixz · 2026-03-22T17:17:53Z

Modules/_json.c

+            }
            Py_DECREF(newobj);
-            Py_XDECREF(ident);
+


Revert blank lines additions

picnixz · 2026-03-22T17:18:35Z

Modules/_json.c


        if (_Py_EnterRecursiveCall(" while encoding a JSON object")) {
+            if (ident != NULL) {
+                PyDict_DelItem(s->markers, ident);


This may fail so we need to check the return type.

picnixz · 2026-03-22T17:19:46Z

Lib/test/test_json/test_recursion.py

+    @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."""


Make it a regular comment as docstrings are shown

picnixz · 2026-03-22T17:21:54Z

Misc/NEWS.d/next/Library/2026-03-19-04-18-27.gh-issue-146152.Ifrd7O.rst

@@ -0,0 +1,3 @@
+Fix a memory leak in the :mod:`json` module when encoding objects with a


This looks like three separate bugs but the test is testing all of them at the same time. Please reformulate this so that it convenes the issue more properly.

bedevere-app · 2026-03-22T17:22:24Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

bedevere-app bot mentioned this pull request Mar 19, 2026

_json: stale markers entries on error paths in encoder_listencode_obj #146152

Open

bedevere-app bot added the awaiting review label Mar 19, 2026

okiemute04 force-pushed the fix-json-encoder-leak branch 3 times, most recently from 3eddfdf to 51c39f9 Compare March 19, 2026 13:53

okiemute04 force-pushed the fix-json-encoder-leak branch from 51c39f9 to 0597100 Compare March 19, 2026 14:00

serhiy-storchaka reviewed Mar 20, 2026

View reviewed changes

okiemute04 added 2 commits March 20, 2026 02:39

Add test for JSON encoder memory leak on recursion error

9ec9348

Add test for JSON encoder memory leak on recursion error

cd9cc6f

serhiy-storchaka reviewed Mar 22, 2026

View reviewed changes

okiemute04 added 2 commits March 22, 2026 06:42

Use support.gc_collect() in JSON encoder memory leak test

5428b5e

Trigger CI rebuild

b68b52b

picnixz requested changes Mar 22, 2026

View reviewed changes

bedevere-app bot removed the awaiting review label Mar 22, 2026

bedevere-app bot added the awaiting changes label Mar 22, 2026

		self.assertIsNone(ref(),
		f"Object {i} still alive - memory leak detected!")

	self.assertIsNone(ref(),
	f"Object {i} still alive - memory leak detected!")
	self.assertIsNone(ref(), f"object {i} is still alive")

		@@ -0,0 +1,3 @@
		Fix a memory leak in the :mod:`json` module when encoding objects with a

Uh oh!

Conversation

okiemute04 commented Mar 19, 2026 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

okiemute04 commented Mar 20, 2026

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

okiemute04 commented Mar 19, 2026 •

edited by bedevere-app bot

Loading