fix: JSONB ASM direct-write skips level tracking causing false 'level too large' on flat lists, for issue #3616#7635
Conversation
…acking, for issue alibaba#3616 When a Bean's field writers are split across direct-write and non-direct-write groups (e.g. a String field plus a BigDecimal field), the non-direct path emits the leading BC_OBJECT via startObject() and the direct path emits the trailing BC_OBJECT_END as a raw byte. startObject() bumps level but the raw-byte end never decrements it, so each serialized bean leaks 1 level. After ~2049 beans the JSONWriter throws "level too large : 2049" even though the structure has no real nesting. Mirror the level tracking on the direct-write path: increment level after writing BC_OBJECT raw, decrement after writing BC_OBJECT_END raw. New incrementLevel / decrementLevel helpers on JSONWriter expose just the level bookkeeping (no byte output), so the direct path keeps its raw-byte performance.
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
The fix correctly addresses the root cause: the ASM direct-write path for JSONB now mirrors the level tracking of startObject()/endObject() via the new incrementLevel()/decrementLevel() methods. The implementation is minimal, symmetric, and consistent with existing code patterns. All tests pass including regression tests for related issues.
— qwen3.7-max via Qwen Code /review
Local Verification ReportBug Reproduction (main branch)Confirmed the bug exists on The same reproducer on this PR branch: Root Cause AnalysisVerified the root cause described in the PR: when a Bean's field writers are split across direct-write (e.g. The fix adds Test Results (branch
|
| Test Suite | Tests | Result |
|---|---|---|
| Issue3616 (PR) | 2 | ✅ PASS |
| Issue3616RealWorld (maintainer) | 12 | ✅ PASS |
| Issue3657 (deep nesting guard) | 6 | ✅ PASS |
| Issue7616 (JSONB direct-write) | 3 | ✅ PASS |
com.alibaba.fastjson2.writer.*Test |
106 | ✅ PASS |
com.alibaba.fastjson2.jsonb.*Test |
97 | ✅ PASS |
| Total | 226 | ✅ ALL PASS |
Additional Real-World Scenarios Verified
Beyond the PR's Issue3616 tests, the following scenarios were tested:
- Scale — 10,000-element flat list of
(String, BigDecimal)beans with full round-trip (serialize → deserialize → assert field values) - Boundary — Exactly 2,048 and 2,049 elements (the precise failure threshold on
main) - Field type combinations —
String+BigInteger,String+LocalDate,String+LocalDateTime, multi-field beans with 2 direct + 2 non-direct fields (5,000 elements) - Real nesting still bounded — Deep chain of 2,500 nodes correctly triggers
level too large(level tracking fix does not weaken the overflow guard) - Moderate nesting — 10-level deep chain serialises and deserialises correctly
- Mixed nested + flat — Container object with a flat list of 5,000
(String, BigDecimal)beans inside - JSON text path unaffected —
JSON.toJSONString()on 3,000-element list works as before (fix is JSONB-specific) - Round-trip fidelity — Single bean serialise → deserialise produces identical field values
Review Summary
- Fix scope: Surgical — 2 new helper methods on
JSONWriter(no behaviour change for existing callers), plus 6 lines of bytecode emission in the JSONB ASM generator - No JSON-text serialiser changes, no API surface broken
- All 226 existing regression tests pass, including the deep-nesting guard (Issue3657) and the JSONB direct-write byte-frame fix (Issue7616)
Recommendation: LGTM, safe to merge.
Collapsible Detailed Test Log (click to expand)
Issue3616 (PR tests) — 2/2 PASS
Running com.alibaba.fastjson2.issues_3600.Issue3616
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.021 s
testFlatListBigDecimalAndString— 2,100 beans(String, BigDecimal), JSONB round-triptestFlatListWithDateObject— 2,100 beans(String, Date), JSONB serialisation
Issue3616RealWorld (maintainer tests) — 12/12 PASS
Running com.alibaba.fastjson2.issues_3600.Issue3616RealWorld
Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.084 s
| Test | Elements | Description |
|---|---|---|
testScale_10kElements |
10,000 | (String, BigDecimal) round-trip with field value assertions |
testBoundary_Exactly2048Elements |
2,048 | At the main failure boundary — passes with fix |
testBoundary_Exactly2049Elements |
2,049 | Exact trigger count on main — passes with fix |
testStringPlusBigInteger |
3,000 | (String, BigInteger) — different non-direct field type |
testStringPlusLocalDate |
3,000 | (String, LocalDate) round-trip |
testStringPlusLocalDateTime |
3,000 | (String, LocalDateTime) round-trip |
testMultipleDirectAndNonDirectFields |
5,000 | (String, String, BigDecimal, Date) mixed |
testRealNestingStillBounded |
depth 2,500 | Deep chain correctly triggers level too large |
testModerateNestingWorks |
depth 10 | Serialise + deserialise + walk chain |
testNestedStructureWithFlatLists |
5,000 items | Container wrapping flat list |
testJsonTextSerializationUnaffected |
3,000 | JSON.toJSONString() unchanged |
testRoundTripFidelity |
1 | Field-level equality after round-trip |
Issue3657 (deep nesting guard) — 6/6 PASS
Running com.alibaba.fastjson2.issues_3600.Issue3657
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.040 s
testJSONFactoryAPI,testDefaultMaxLevelLimit,testCustomMaxLevel,testBoundaryConditions,testRealWorldUsage,testRegression
Issue7616 (JSONB direct-write byte-frame) — 3/3 PASS
Running com.alibaba.fastjson2.issues_7000.Issue7616
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.253 s
testLargeUtf16MultiField,testSplitGroupWithDirectMiddle,testLargeUtf16StringInNestedBean
Writer test suite — 106/106 PASS
Tests run: 106, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
JSONB test suite — 97/97 PASS
Tests run: 97, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
Environment
- OS: macOS (darwin)
- JDK: OpenJDK (project default)
- Maven: via
mvn test -pl core - Build:
mvn validate(checkstyle) passes
wenshao
left a comment
There was a problem hiding this comment.
Clean, well-targeted fix. The level tracking is now symmetric between the direct-write and non-direct paths. One suggestion on test robustness (inline).
— qwen3-coder via Qwen Code /review
| } | ||
|
|
||
| @Test | ||
| public void testFlatListWithDateObject() { |
There was a problem hiding this comment.
[Suggestion] testFlatListWithDateObject only asserts that serialization does not throw; it never deserializes or verifies field content, unlike testFlatListBigDecimalAndString which does a full round-trip. — Concrete cost: a regression that produces structurally valid-but-wrong JSONB bytes for the Date+String split would pass this test silently.
| public void testFlatListWithDateObject() { | |
| public void testFlatListWithDateObject() { | |
| List<DateBean> list = new ArrayList<>(); | |
| for (int i = 0; i < 2100; i++) { | |
| list.add(new DateBean("name" + i, new java.util.Date(0))); | |
| } | |
| byte[] bytes = assertDoesNotThrow(() -> JSONB.toBytes(list)); | |
| List<DateBean> parsed = JSONB.parseArray(bytes, DateBean.class); | |
| assertEquals(list.size(), parsed.size()); | |
| assertEquals("name0", parsed.get(0).getName()); | |
| assertEquals(new java.util.Date(0), parsed.get(0).getTime()); | |
| } |
— qwen3-coder via Qwen Code /review
Problem
JSONB.toBytes(list)on a flat list of JavaBeans throwsJSONException: level too large : 2049after ~2049 elements, even though the structure has no actual nesting. Reported reproducer (issue #3616):TestObjecthas aStringfield and aBigDecimalfield — both standard, both unrelated to nesting. Stack trace:Root cause
ObjectWriterCreatorASM.genMethodWriteJSONBpartitions the bean's field writers into "direct" and "non-direct" groups. The two paths emit the BC_OBJECT / BC_OBJECT_END framing differently:group.start): callsJSONWriter.startObject()— writes BC_OBJECT and incrementslevel.group.end): callsJSONWriter.endObject()— writes BC_OBJECT_END and decrementslevel.group.start): writes BC_OBJECT raw to the byte buffer — does not touchlevel.group.end): writes BC_OBJECT_END raw to the byte buffer — does not touchlevel.For
TestObject, the field groups order out as[BigDecimal c (non-direct, start=true)] + [String d (direct, end=true)]. SostartObject()incrementslevelbut the matching end is the raw-byte direct path, which leaveslevelincremented. Each bean in the list leaks 1 level. At iteration 2049 we hitcontext.maxLevelandoverflowLevel()fires.The same imbalance occurs on any field-group split where start and end fall on different paths.
Fix
Mirror the level bookkeeping on the direct-write path so it stays consistent with
startObject()/endObject():JSONWriter.incrementLevel()— bumpsleveland runs the same overflow check asstartObject(). No byte output.JSONWriter.decrementLevel()— decrementslevel. No byte output.ObjectWriterCreatorASMcallsincrementLevel()after writing BC_OBJECT raw anddecrementLevel()after writing BC_OBJECT_END raw. The direct path keeps its raw-byte field-value writes, so the performance optimisation is preserved.Tests
core/src/test/java/com/alibaba/fastjson2/issues_3600/Issue3616.java:testFlatListBigDecimalAndString— 2100-element list of(String, BigDecimal)beans; serialise to JSONB and round-trip back, asserting size and field values. Reproduces the issue exactly and verifies it is gone.testFlatListWithDateObject— 2100-element list of(String, Date)beans, exercising another non-direct field type to guard against future regressions.Existing regression tests still pass:
Issue3657(intentionallevel too largefor genuinely deep nesting) — 6 testsIssue7616(related fix for direct-write byte-frame capacity) — 3 tests*Writer*test suite — 394 tests, all greenImpact
Surgical: 2 helper methods on
JSONWriter(no behaviour change for existing callers) plus 6 lines of bytecode emission in the JSONB ASM generator. No JSON-text serialiser changes, no API surface broken.Fixes #3616