Skip to content

Conversation

@davvid
Copy link
Member

@davvid davvid commented Dec 7, 2025

Even though the unpickler goes out of scope immediately after decode() completes, the Python garbage collector is not able to determine that garbage collection can occur despite the fact that no references are being held. The observed behavior is that Python is not deleting the ephemeral unpickler instance.

Make this easier for the GC to handle by explicitly resetting the ephemeral Unpickler instance at the end of decode(). This guarantees that no external references are being held and allows them to be cleaned up in a slightly more predictable fashion.

Closes: #579

Even though the unpickler goes out of scope immediately after decode()
completes, the Python garbage collector is not able to determine that
garbage collection can occur despite the fact that no references are
being held. The observed behavior is that Python is not deleting
the ephemeral unpickler instance.

Make this easier for the GC to handle by explicitly resetting the
ephemeral Unpickler instance at the end of decode(). This guarantees
that no external references are being held and allows them to
be cleaned up in a slightly more predictable fashion.

The test case was added to the numpy test suite because triggering
this behavior currently requires the numpy extension to be used.

Closes: jsonpickle#579
@davvid
Copy link
Member Author

davvid commented Dec 7, 2025

The most recent push added a test to ensure that this behaves as expected. I was only able to make the test fail when using numpy (as reported in #579) so I ended up adding the test to the numpy extension tests in numpy_test.py.

@davvid davvid requested a review from Theelx December 7, 2025 23:32
Copy link
Contributor

@Theelx Theelx left a comment

Choose a reason for hiding this comment

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

This is an surprisingly simple fix, thanks for this! I added my approval, but can you add a CHANGES.rst entry before merging it?

@davvid davvid merged commit 2bc5cc6 into jsonpickle:main Dec 9, 2025
5 checks passed
davvid added a commit to davvid/jsonpickle that referenced this pull request Dec 9, 2025
* davvid/decode-gc:
  CHANGES: update release notes draft
  unpickler: avoid holding on to external references
@davvid
Copy link
Member Author

davvid commented Dec 9, 2025

Thanks for reviewing! CHANGES.rst has been updated and I pushed a fixup commit to apply the latest code style to this commit.

@davvid davvid deleted the decode-gc branch December 9, 2025 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deallocation Of Memory After Decode

2 participants