mandrean opened a new pull request, #3342:
URL: https://github.com/apache/fory/pull/3342
## Why?
Deserializing nested HashMap subclasses (e.g. `class MyMap extends
HashMap<String, MyOtherMap>`) throws NPE in both SCHEMA_CONSISTENT and
COMPATIBLE modes. Two distinct bugs cause this.
## What does this PR do?
**Bug 1: Write/read generics push asymmetry in
`MapLikeSerializer.readJavaChunkGeneric`**
The write path in `writeJavaChunkGeneric` always pushes generic types onto
the `Generics` stack, but the read path conditionally pushed them based on
`hasGenericParameters()`. For nested HashMap subclasses without own type
parameters (only inherited ones), `hasGenericParameters()` returns false,
causing inner maps
to fall into the non-generic read path while bytes were written by the
generic path. This results in a null serializer when
`KEY_DECL_TYPE`/`VALUE_DECL_TYPE` chunk flags are set.
Fix: remove the conditional branch and always push/pop generics
symmetrically, matching the write path.
**Bug 2: Meta context index desync in
`ChildContainerSerializers.readAndSkipLayerClassMeta`**
In COMPATIBLE mode, `writeLayerClassMeta` adds layer marker classes to
`metaContext.classMap`, which shares the same index space as
`writeSharedClassMeta`. However, `readAndSkipLayerClassMeta` skipped the bytes
without adding entries to `metaContext.readTypeInfos`. This caused subsequent
`readSharedClassMeta`
reference lookups to use wrong indices.
Fix: add a `null` placeholder to `metaContext.readTypeInfos` after skipping
a new layer meta entry.
## Related issues
https://github.com/apache/fory/issues/3337
## Does this PR introduce any user-facing change?
- [ ] Does this PR introduce any public API change? No
- [ ] Does this PR introduce any binary protocol compatibility change? No
Binary compatibility is safe. The fix only changes the read path, it doesn't
change what bytes are written. The write path was already always pushing
generics and writing with KEY_DECL_TYPE/VALUE_DECL_TYPE flags set. The read
path now correctly consumes those same bytes.
For bug 2, the fix only adds a bookkeeping placeholder on the read side,
writing is untouched.
Old writers + new readers: works. New writers + old readers: unchanged
(write path is identical)
## Benchmark
Bug 1 fix removes a conditional branch (one fewer branch per chunk
iteration), so performance should be equal or marginally better.
Bug 2 fix adds a single `ArrayList.add(null)` call per new layer meta entry,
which is negligible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
