Skip to content

Check circular references introduced by custom JSON methods - #390

Merged
etrepum merged 2 commits into
simplejson:mainfrom
charan-rathore:fix-custom-method-cycles
Oct 1, 2026
Merged

etrepum merged 2 commits into
simplejson:mainfrom
charan-rathore:fix-custom-method-cycles

Conversation

@charan-rathore

Copy link
Copy Markdown
Contributor

With check_circular=True, an object whose for_json() returns itself exhausts recursion instead of raising the usual Circular reference detected error. A similar cycle through _asdict() is missed. The encoder documents circular checking for custom encoded objects.

Track the original object while encoding the method's return value in both Python and C. Reuse the existing C marker helpers and remove successful markers so repeated references remain valid. Add direct and indirect cycle tests, nested list/dict cases for both protocols, acyclic chains, repeated references and disabled circular checking.

Validation on macOS arm64:

  • On unchanged main, the final regression suite has 7 errors in 10 tests, all missed cycles. The minimal public API reproduction raises RecursionError in both encoders.
  • The repository's required two-encoder runner passes on Python 3.12.12: 492 tests, 74 skipped.
  • The same runner passes on Python 3.14.3: 492 tests, 62 skipped.
  • Both C extensions were compiled and their encoder/scanner wiring checked. Python compilation and git diff --check pass.

No Python 2.7, PyPy, Windows, free-threaded or debug builds were run. AGENTS.md requests a 3.14.4 spot-check; the local uv download catalog lacks that version, so 3.14.3 was used instead. Builds emit a linker warning about the pre-existing Modules/_hacl search path.

@charan-rathore

charan-rathore commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

The Pyodide wheel job crashed with "RangeError: Maximum call stack size exceeded" (a fatal Pyodide error, not a Python RecursionError). The test I added that recurses without bound on purpose is test_circular_checks_can_be_disabled, so I believe that is the cause, though the log does not name the test. I pushed 7667836 to skip that one test on emscripten, like test_free_threading and test_tool already do. I can't run Pyodide here, so the re-run will confirm. Thanks for re-running it.

@etrepum
etrepum added this pull request to the merge queue Oct 1, 2026
Merged via the queue into simplejson:main with commit 41f8017 Oct 1, 2026
26 checks passed
@charan-rathore
charan-rathore deleted the fix-custom-method-cycles branch October 2, 2026 04:29
etrepum pushed a commit that referenced this pull request Oct 2, 2026
Add CHANGES entries for #387, #388 and #390. Test Python 3.15 and
3.15t via allow-prereleases so the jobs follow 3.15.0 final, add
PyPy 3.12, move debug and TSan builds to CPython 3.14.8, and bump
checkout, setup-python, cache, setup-uv and cibuildwheel.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01FpcGq7FDACHqLiW7juM4SK
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 3, 2026
4.2.0 released 2026-10-02

* Start building wheels for Python 3.15 and drop Python 3.8 support
  via upgrade to cibuildwheel v4.2.0
  simplejson/simplejson#385
* Build and test riscv64 Linux wheels on native RISE runners
  simplejson/simplejson#389
* The C scanner now reports the same error message as the pure Python
  decoder for an invalid first key in an object
  simplejson/simplejson#388
* ``int_as_string_bitcount`` now supports thresholds of 64 bits and above
  (including 128-bit integers) in both the C and pure Python encoders;
  the C encoder previously rejected them
  simplejson/simplejson#387
* Circular references introduced through ``for_json()`` or ``_asdict()``
  now raise ``ValueError("Circular reference detected")`` instead of
  recursing without bound
  simplejson/simplejson#390
* CI now tests Python 3.15 (including free-threaded 3.15t), PyPy 3.12,
  CPython 3.14.8 debug builds, and uses current GitHub Actions and
  cibuildwheel v4.2.1
  simplejson/simplejson#386
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.

2 participants