chaokunyang commented on PR #3122: URL: https://github.com/apache/fory/pull/3122#issuecomment-4087552274
Hi @sagiruthvik @nealeu , the current design is fragile: 1. The hash is now mutable, but it is consumed before registration is finished. `ClassResolver` registers itself under the initial hash during construction in `ClassResolver.java:234`, and later GraalVM lookups read whatever the current hash is in `TypeResolver.java:1480`. `registerSerializer(...)` then mutates the hash afterward in `Fory.java:230`. That means one `Fory` can end up split across multiple hash buckets. In native-image mode, later lookups can miss the resolver or serializer metadata entirely. 2. The fix is incomplete across public APIs. Only two overloads update `configHash` in `Fory.java:230` and `Fory.java:236`. `registerSerializer(Class, Function<...>)`, all `registerSerializerAndType(...)`, and both `registerUnion(...)` paths still bypass the update in `Fory.java:218` and `Fory.java:241`. `TypeResolver.registerSerializerAndType(...)` goes straight to resolver-side registration in `TypeResolver.java:235`, so the original cache-sharing bug can still happen through those APIs. 3. There is also a secondary fragility here: the new namespace starts from raw `Config.hashCode()` in `Fory.java:133` and `Config.java:329`. That makes the global cache key collision-prone and order-sensitive, which is weaker than the previous collision-free ID mapping. -- 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]
