Skip to content

Commit

Permalink
fix(java): fix polymorphic array serialization (apache#1324)
Browse files Browse the repository at this point in the history 
`Object[]` can be assigned from `String[]` and other arrays, we take it
as a final type before, so we skip writing classinfo for such types and
lost object type too.

This PR fix this polymorphic array serialization bug by checking array
type polymorphism by component type.

Closes apache#1323
  • Loading branch information
@chaokunyang
chaokunyang authored Jan 10, 2024
1 parent 8230837 commit 18527e3
Show file tree
Hide file tree
Showing 19 changed files with 262 additions and 112 deletions.
35 changes: 17 additions & 18 deletions java/fury-core/src/main/java/org/apache/fury/builder/BaseObjectCodecBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@

import com.google.common.collect.ImmutableSet;
import com.google.common.reflect.TypeToken;
import java.lang.reflect.Modifier;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -418,15 +417,15 @@ protected boolean useMapSerialization(TypeToken<?> typeToken) {
* the method can still return false. For example, we return false in meta share mode to write
* class defs for the non-inner final types.
*/
protected abstract boolean isFinal(Class<?> clz);
protected abstract boolean isMonomorphic(Class<?> clz);

protected Expression serializeForNotNullObject(
Expression inputObject, Expression buffer, TypeToken<?> typeToken, Expression serializer) {
Class<?> clz = getRawType(typeToken);
if (serializer != null) {
return new Invoke(serializer, "write", buffer, inputObject);
}
if (isFinal(clz)) {
if (isMonomorphic(clz)) {
serializer = getOrCreateSerializer(clz);
return new Invoke(serializer, "write", buffer, inputObject);
} else {
Expand Down Expand Up @@ -473,7 +472,7 @@ protected Expression writeForNotNullNonFinalObject(
*/
protected Expression getOrCreateSerializer(Class<?> cls) {
// Not need to check cls final, take collection writeSameTypeElements as an example.
// Preconditions.checkArgument(isFinal(cls), cls);
// Preconditions.checkArgument(isMonomorphic(cls), cls);
Reference serializerRef = serializerMap.get(cls);
if (serializerRef == null) {
// potential recursive call for seq codec generation is handled in `getSerializerClass`.
Expand Down Expand Up @@ -552,7 +551,7 @@ protected Expression getClassExpr(Class<?> cls) {
*/
protected Tuple2<Reference, Boolean> addClassInfoField(Class<?> cls) {
Expression classInfoExpr;
boolean needUpdate = !ReflectionUtils.isFinal(cls);
boolean needUpdate = !ReflectionUtils.isMonomorphic(cls);
String key;
if (!needUpdate) {
key = "classInfo:" + cls;
Expand All @@ -572,7 +571,7 @@ protected Tuple2<Reference, Boolean> addClassInfoField(Class<?> cls) {
classInfoRef = Tuple2.of(fieldRef(name, classInfoTypeToken), false);
} else {
classInfoExpr = inlineInvoke(classResolverRef, "nilClassInfo", classInfoTypeToken);
String name = ctx.newName(StringUtils.uncapitalize(cls.getSimpleName()) + "ClassInfo");
String name = ctx.newName(cls, "ClassInfo");
ctx.addField(false, ctx.type(ClassInfo.class), name, classInfoExpr);
// Can't use fieldRef, since the field is not final.
classInfoRef = Tuple2.of(new Reference(name, classInfoTypeToken), true);
Expand All @@ -584,7 +583,7 @@ protected Tuple2<Reference, Boolean> addClassInfoField(Class<?> cls) {
protected Reference addClassInfoHolderField(Class<?> cls) {
// Final type need to write classinfo when meta share enabled.
String key;
if (Modifier.isFinal(cls.getModifiers())) {
if (ReflectionUtils.isMonomorphic(cls)) {
key = "classInfoHolder:" + cls;
} else {
key = "classInfoHolder:" + cls + walkPath;
Expand All @@ -608,7 +607,7 @@ protected Expression readClassInfo(Class<?> cls, Expression buffer) {
}

protected Expression readClassInfo(Class<?> cls, Expression buffer, boolean inlineReadClassInfo) {
if (Modifier.isFinal(cls.getModifiers())) {
if (ReflectionUtils.isMonomorphic(cls)) {
Reference classInfoRef = addClassInfoField(cls).f0;
if (inlineReadClassInfo) {
return inlineInvoke(
Expand Down Expand Up @@ -665,7 +664,7 @@ protected Expression serializeForCollection(
// get serializer, write class info if necessary.
if (serializer == null) {
Class<?> clz = getRawType(typeToken);
if (isFinal(clz)) {
if (isMonomorphic(clz)) {
serializer = getOrCreateSerializer(clz);
} else {
ListExpression writeClassAction = new ListExpression();
Expand Down Expand Up @@ -730,7 +729,7 @@ protected Expression writeCollectionData(
writeElementsHeader(elemClass, trackingRef, serializer, buffer, collection);
Expression flags = writeElementsHeader.f0;
builder.add(flags);
boolean finalType = isFinal(elemClass);
boolean finalType = isMonomorphic(elemClass);
if (finalType) {
if (trackingRef) {
builder.add(
Expand Down Expand Up @@ -816,7 +815,7 @@ private Tuple2<Expression, Invoke> writeElementsHeader(
Expression collectionSerializer,
Expression buffer,
Expression value) {
if (isFinal(elementType)) {
if (isMonomorphic(elementType)) {
Expression bitmap;
if (trackingRef) {
bitmap =
Expand Down Expand Up @@ -922,7 +921,7 @@ private Expression writeContainerElement(
boolean generateNewMethod =
useCollectionSerialization(elementType) || useMapSerialization(elementType);
Class<?> rawType = getRawType(elementType);
boolean finalType = isFinal(rawType);
boolean finalType = isMonomorphic(rawType);
elem = tryCastIfPublic(elem, elementType);
Expression write;
if (finalType) {
Expand Down Expand Up @@ -968,7 +967,7 @@ protected Expression serializeForMap(
boolean generateNewMethod) {
if (serializer == null) {
Class<?> clz = getRawType(typeToken);
if (isFinal(clz)) {
if (isMonomorphic(clz)) {
serializer = getOrCreateSerializer(clz);
} else {
ListExpression writeClassAction = new ListExpression();
Expand Down Expand Up @@ -1163,7 +1162,7 @@ protected Expression deserializeForNotNull(
} else if (useMapSerialization(typeToken)) {
obj = deserializeForMap(buffer, typeToken, serializer, cutPoint);
} else {
if (isFinal(cls)) {
if (isMonomorphic(cls)) {
Preconditions.checkState(serializer == null);
serializer = getOrCreateSerializer(cls);
Class<?> returnType =
Expand Down Expand Up @@ -1195,7 +1194,7 @@ protected Expression deserializeForCollection(
TypeToken<?> elementType = getElementType(typeToken);
if (serializer == null) {
Class<?> cls = getRawType(typeToken);
if (isFinal(cls)) {
if (isMonomorphic(cls)) {
serializer = getOrCreateSerializer(cls);
} else {
Expression classInfo = readClassInfo(cls, buffer);
Expand Down Expand Up @@ -1244,7 +1243,7 @@ protected Expression readCollectionCodegen(
builder.add(flags);
Class<?> elemClass = TypeUtils.getRawType(elementType);
walkPath.add(elementType.toString());
boolean finalType = isFinal(elemClass);
boolean finalType = isMonomorphic(elemClass);
boolean trackingRef = visitFury(fury -> fury.getClassResolver().needToWriteRef(elemClass));
if (finalType) {
if (trackingRef) {
Expand Down Expand Up @@ -1376,7 +1375,7 @@ private Expression readContainerElement(
useCollectionSerialization(elementType) || useMapSerialization(elementType);
CutPoint cutPoint = new CutPoint(genNewMethod, buffer);
Class<?> rawType = getRawType(elementType);
boolean finalType = isFinal(rawType);
boolean finalType = isMonomorphic(rawType);
Expression read;
if (finalType) {
if (trackingRef) {
Expand Down Expand Up @@ -1426,7 +1425,7 @@ protected Expression deserializeForMap(
TypeToken<?> valueType = keyValueType.f1;
if (serializer == null) {
Class<?> cls = getRawType(typeToken);
if (isFinal(cls)) {
if (isMonomorphic(cls)) {
serializer = getOrCreateSerializer(cls);
} else {
Expression classInfo = readClassInfo(cls, buffer);
Expand Down
4 changes: 2 additions & 2 deletions java/fury-core/src/main/java/org/apache/fury/builder/CodecBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,12 @@ protected Expression setFieldValue(Expression bean, Descriptor d, Expression val
if (duplicatedFields.contains(fieldName) || !sourcePublicAccessible(beanClass)) {
return unsafeSetField(bean, d, value);
}
if (!Modifier.isFinal(d.getModifiers()) && Modifier.isPublic(d.getModifiers())) {
if (!d.isFinalField() && Modifier.isPublic(d.getModifiers())) {
return new Expression.SetField(bean, fieldName, value);
} else if (d.getWriteMethod() != null && Modifier.isPublic(d.getWriteMethod().getModifiers())) {
return new Invoke(bean, d.getWriteMethod().getName(), value);
} else {
if (!Modifier.isFinal(d.getModifiers()) && !Modifier.isPrivate(d.getModifiers())) {
if (!d.isFinalField() && !Modifier.isPrivate(d.getModifiers())) {
if (AccessorHelper.defineSetter(d.getField())) {
Class<?> accessorClass = AccessorHelper.getAccessorClass(d.getField());
if (!value.type().equals(d.getTypeToken())) {
Expand Down
16 changes: 8 additions & 8 deletions java/fury-core/src/main/java/org/apache/fury/builder/CompatibleCodecBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import com.google.common.reflect.TypeToken;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -78,6 +77,7 @@
import org.apache.fury.type.TypeUtils;
import org.apache.fury.util.Platform;
import org.apache.fury.util.Preconditions;
import org.apache.fury.util.ReflectionUtils;
import org.apache.fury.util.function.SerializableSupplier;
import org.apache.fury.util.record.RecordUtils;

Expand Down Expand Up @@ -135,8 +135,8 @@ protected void addCommonImports() {
}

@Override
protected boolean isFinal(Class<?> clz) {
return Modifier.isFinal(clz.getModifiers());
protected boolean isMonomorphic(Class<?> clz) {
return ReflectionUtils.isMonomorphic(clz);
}

private Descriptor createDescriptor(FieldInfo fieldInfo) {
Expand Down Expand Up @@ -315,7 +315,7 @@ public Expression buildEncodeExpression() {
buffer, mapFieldInfo.getValueType()));
}
Class<?> clz = descriptor.getRawType();
if (Modifier.isFinal(clz.getModifiers())) {
if (ReflectionUtils.isMonomorphic(clz)) {
// serializeForNotNull won't write field type if it's final,
// but the type is useful if peer doesn't have this field.
writeFieldValue.add(writeFinalClassInfo(buffer, clz));
Expand Down Expand Up @@ -837,7 +837,7 @@ private Expression readObjectField(
skipFinalClassInfo(((MapFieldInfo) fieldInfo).getValueType(), buffer));
}
Class<?> clz = getRawType(typeToken);
if (Modifier.isFinal(clz.getModifiers())) {
if (ReflectionUtils.isMonomorphic(clz)) {
// deserializeForNotNull won't read field type if it's final
deserializedValue.add(skipFinalClassInfo(clz, buffer));
}
Expand Down Expand Up @@ -870,14 +870,14 @@ private Expression readObjectField(
}

protected Expression getFinalClassInfo(Class<?> cls) {
Preconditions.checkArgument(Modifier.isFinal(cls.getModifiers()));
Preconditions.checkArgument(ReflectionUtils.isMonomorphic(cls));
Tuple2<Reference, Boolean> classInfoRef = addClassInfoField(cls);
Preconditions.checkArgument(!classInfoRef.f1);
return classInfoRef.f0;
}

protected Expression writeFinalClassInfo(Expression buffer, Class<?> cls) {
Preconditions.checkArgument(Modifier.isFinal(cls.getModifiers()));
Preconditions.checkArgument(ReflectionUtils.isMonomorphic(cls));
ClassInfo classInfo = visitFury(f -> f.getClassResolver().getClassInfo(cls, false));
if (classInfo != null && classInfo.getClassId() != ClassResolver.NO_CLASS_ID) {
return fury.getClassResolver().writeClassExpr(buffer, classInfo.getClassId());
Expand All @@ -887,7 +887,7 @@ protected Expression writeFinalClassInfo(Expression buffer, Class<?> cls) {
}

protected Expression skipFinalClassInfo(Class<?> cls, Expression buffer) {
Preconditions.checkArgument(Modifier.isFinal(cls.getModifiers()));
Preconditions.checkArgument(ReflectionUtils.isMonomorphic(cls));
ClassInfo classInfo = visitFury(f -> f.getClassResolver().getClassInfo(cls, false));
if (classInfo != null && classInfo.getClassId() != ClassResolver.NO_CLASS_ID) {
return fury.getClassResolver().skipRegisteredClassExpr(buffer);
Expand Down
4 changes: 2 additions & 2 deletions java/fury-core/src/main/java/org/apache/fury/builder/ObjectCodecBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ protected void addCommonImports() {

/** Mark non-inner registered final types as non-final to write class def for those types. */
@Override
protected boolean isFinal(Class<?> clz) {
return visitFury(f -> f.getClassResolver().isFinal(clz));
protected boolean isMonomorphic(Class<?> clz) {
return visitFury(f -> f.getClassResolver().isMonomorphic(clz));
}

/**
Expand Down
4 changes: 2 additions & 2 deletions java/fury-core/src/main/java/org/apache/fury/builder/ObjectCodecOptimizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ private void buildGroups() {
// Note get field value also took some byte code if not public.
List<Descriptor> primitiveDescriptorsList =
new ArrayList<>(descriptorGrouper.getPrimitiveDescriptors());
while (primitiveDescriptorsList.size() > 0) {
int endIndex = Math.min(24, primitiveDescriptorsList.size());
while (!primitiveDescriptorsList.isEmpty()) {
int endIndex = Math.min(20, primitiveDescriptorsList.size());
primitiveGroups.add(primitiveDescriptorsList.subList(0, endIndex));
primitiveDescriptorsList =
primitiveDescriptorsList.subList(endIndex, primitiveDescriptorsList.size());
Expand Down
32 changes: 12 additions & 20 deletions java/fury-core/src/main/java/org/apache/fury/resolver/ClassResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.io.IOException;
import java.io.Serializable;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.lang.reflect.Type;
import java.math.BigDecimal;
import java.math.BigInteger;
Expand Down Expand Up @@ -514,22 +513,15 @@ public List<Class<?>> getRegisteredClasses() {
* a class is registered but not an inner class with inner serializer, it will still be taken as
* non-final to write class def, so that it can be deserialized by the peer still..
*/
public boolean isFinal(Class<?> clz) {
if (Modifier.isFinal(clz.getModifiers())) {
if (fury.getConfig().shareMetaContext()) {
boolean isInnerClass = isInnerClass(clz);
if (!isInnerClass) {
return false;
} else {
// can't create final map/collection type using TypeUtils.mapOf(TypeToken<K>,
// TypeToken<V>)
return !Map.class.isAssignableFrom(clz) && !Collection.class.isAssignableFrom(clz);
}
} else {
return true;
}
public boolean isMonomorphic(Class<?> clz) {
if (fury.getConfig().shareMetaContext()) {
// can't create final map/collection type using TypeUtils.mapOf(TypeToken<K>,
// TypeToken<V>)
return ReflectionUtils.isMonomorphic(clz)
&& isInnerClass(clz)
&& (!Map.class.isAssignableFrom(clz) && !Collection.class.isAssignableFrom(clz));
}
return false;
return ReflectionUtils.isMonomorphic(clz);
}

/** Returns true if <code>cls</code> is fury inner registered class. */
Expand Down Expand Up @@ -1752,9 +1744,9 @@ public GenericType buildGenericType(TypeToken<?> typeToken) {
typeToken.getType(),
t -> {
if (t.getClass() == Class.class) {
return isFinal((Class<?>) t);
return isMonomorphic((Class<?>) t);
} else {
return isFinal(getRawType(t));
return isMonomorphic(getRawType(t));
}
});
}
Expand All @@ -1764,9 +1756,9 @@ public GenericType buildGenericType(Type type) {
type,
t -> {
if (t.getClass() == Class.class) {
return isFinal((Class<?>) t);
return isMonomorphic((Class<?>) t);
} else {
return isFinal(getRawType(t));
return isMonomorphic(getRawType(t));
}
});
}
Expand Down
19 changes: 10 additions & 9 deletions java/fury-core/src/main/java/org/apache/fury/resolver/FieldResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

import com.google.common.reflect.TypeToken;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -261,7 +260,9 @@ public FieldResolver(
Short classId = classResolver.getRegisteredClassId(fieldType);
// try to encode 6 bit for a char if field name is ascii.
// then 7 byte can encode 9 char, remains 2 bits can be used as flag bits or just left.
if (ReflectionUtils.isFinal(fieldType) && classId != null && classId < MAX_EMBED_CLASS_ID) {
if (ReflectionUtils.isMonomorphic(fieldType)
&& classId != null
&& classId < MAX_EMBED_CLASS_ID) {
if (fieldNameLen <= 3 && classId <= 63) { // at most 4 chars
// little-endian reversed bits: 24 bits field name + 6 bits class id + bit `1 0`.
int encodedFieldInfo = (int) encodeFieldNameAsLong(fieldName);
Expand Down Expand Up @@ -744,7 +745,7 @@ public static FieldInfo of(
TypeToken<?> elementTypeToken =
TypeUtils.getElementType(TypeToken.of(field.getGenericType()));
byte fieldType =
Modifier.isFinal(getRawType(elementTypeToken).getModifiers())
ReflectionUtils.isMonomorphic(getRawType(elementTypeToken))
? FieldTypes.COLLECTION_ELEMENT_FINAL
: FieldTypes.OBJECT;
return new CollectionFieldInfo(
Expand All @@ -755,12 +756,12 @@ public static FieldInfo of(
TypeToken<?> keyTypeToken = kvType.f0;
TypeToken<?> valueTypeToken = kvType.f1;
byte fieldType;
if (Modifier.isFinal(getRawType(keyTypeToken).getModifiers())
&& Modifier.isFinal(getRawType(valueTypeToken).getModifiers())) {
if (ReflectionUtils.isMonomorphic(getRawType(keyTypeToken))
&& ReflectionUtils.isMonomorphic(getRawType(valueTypeToken))) {
fieldType = FieldTypes.MAP_KV_FINAL;
} else if (Modifier.isFinal(getRawType(keyTypeToken).getModifiers())) {
} else if (ReflectionUtils.isMonomorphic(getRawType(keyTypeToken))) {
fieldType = FieldTypes.MAP_KEY_FINAL;
} else if (Modifier.isFinal(getRawType(valueTypeToken).getModifiers())) {
} else if (ReflectionUtils.isMonomorphic(getRawType(valueTypeToken))) {
fieldType = FieldTypes.MAP_VALUE_FINAL;
} else {
fieldType = FieldTypes.OBJECT;
Expand Down Expand Up @@ -930,10 +931,10 @@ public MapFieldInfo(
this.keyTypeToken = keyTypeToken;
this.valueTypeToken = valueTypeToken;
keyType = getRawType(keyTypeToken);
isKeyTypeFinal = Modifier.isFinal(keyType.getModifiers());
isKeyTypeFinal = ReflectionUtils.isMonomorphic(keyType);
keyClassInfoHolder = classResolver.nilClassInfoHolder();
valueType = getRawType(valueTypeToken);
isValueTypeFinal = Modifier.isFinal(valueType.getModifiers());
isValueTypeFinal = ReflectionUtils.isMonomorphic(valueType);
valueClassInfoHolder = classResolver.nilClassInfoHolder();
}

Expand Down
Loading

0 comments on commit 18527e3

Please sign in to comment.