Repository navigation
Avoid unchecked list indexing in JSON encoder - #9431
youknowone merged 1 commit into
Conversation
Clone checked live-list items under one read guard before encoder callbacks. Stop cleanly if concurrent shrink removes the next item, preserving dynamic callback deletion and marker cleanup. Assisted-by: OpenAI Codex: model version unavailable
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Handle optional sequence lookups crates/stdlib/src/json.rs |
List and tuple encoding use bounds-checked lookups. encode_list stops when a lookup returns no element. The comment now describes live sequence iteration as matching CPython’s behavior. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: youknowone
Merge Risk: 
7ac73
-
The change avoids unchecked indexing when lists shrink during JSON encoding while preserving normal output and cleanup paths. No actionable merge-blocking risk was identified; concurrency stress testing remains unverified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "Avoid unchecked list indexing in JSON encoder" matches the main change. The PR changes JsonEncoder.encode_list in crates/stdlib/src/json.rs to use bounds-checked lookups that return None in… Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "Avoid unchecked list indexing in JSON encoder" matches the main change. The PR changes JsonEncoder.encode_list in crates/stdlib/src/json.rs to use bounds-checked lookups that return None instead of direct indexing, so encoding stops when a list shrinks during iteration. The title is concise, specific, and does not use vague terms. It names the subsystem (JSON encoder) and the core change (checked indexing). It does not mention the race-condition motivation or the single-read-guard lookup, but the title does not need to cover every detail.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Summary
The native JSON list encoder checks the live length and fetches an item under separate list guards. A concurrent shrink can invalidate the index between those operations.
Use a checked, cloned item lookup under one read guard. If the item disappeared, stop before emitting a separator and finish through the existing marker-cleanup path. The guard is released before recursive encoding or Python callbacks. This preserves live callback-deletion semantics rather than taking a whole-list snapshot.
Only
crates/stdlib/src/json.rschanges. No Lib assertions, skips, dependency pins, or pending free-threading-package files are changed.Validation
cargo checkand strict Clippy (-D warnings) passed.Local validation used main
3b0dd785; the encoder source is unchanged in current main, and current-main integration awaits CI.The selected tests check the native encoder, ten-item callback deletion, small circular-reference/marker cleanup cases, and indentation. They are not a concurrent-race or general free-threading compatibility claim. Full workspace, C-API, concurrency stress, and GC/UAF-specific regressions were not run locally; hosted CI remains pending.
AI assistance
OpenAI Codex assisted with the implementation, CPython source comparison, validation, and this pull request description; the model version is unavailable.
Summary by CodeRabbit