From 647a80bb487597d45759f4c98e15442623e66917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E8=91=89=20Scarlet?= <93977077+mukjepscarlet@users.noreply.github.com> Date: Tue, 25 Nov 2025 22:56:39 +0800 Subject: [PATCH 1/3] refactor: slightly optimize ConstructorConstructor --- .../gson/internal/ConstructorConstructor.java | 112 ++++++++++-------- 1 file changed, 64 insertions(+), 48 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java index 5588367366..0a02d2acab 100644 --- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java +++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java @@ -109,14 +109,14 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) @SuppressWarnings("unchecked") // types must agree InstanceCreator typeCreator = (InstanceCreator) instanceCreators.get(type); if (typeCreator != null) { - return () -> typeCreator.createInstance(type); + return new InstanceCreatorConstructor<>(typeCreator, type); } // Next try raw type match for instance creators @SuppressWarnings("unchecked") // types must agree InstanceCreator rawTypeCreator = (InstanceCreator) instanceCreators.get(rawType); if (rawTypeCreator != null) { - return () -> rawTypeCreator.createInstance(type); + return new InstanceCreatorConstructor<>(rawTypeCreator, type); } // First consider special constructors before checking for no-args constructors @@ -143,9 +143,7 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) // of adjusting filter suggested below is irrelevant since it would not solve the problem String exceptionMessage = checkInstantiable(rawType); if (exceptionMessage != null) { - return () -> { - throw new JsonIOException(exceptionMessage); - }; + return new ExceptionObjectConstructor<>(exceptionMessage); } if (!allowUnsafe) { @@ -153,9 +151,7 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) "Unable to create instance of " + rawType + "; Register an InstanceCreator or a TypeAdapter for this type."; - return () -> { - throw new JsonIOException(message); - }; + return new ExceptionObjectConstructor<>(message); } // Consider usage of Unsafe as reflection, so don't use if BLOCK_ALL @@ -167,9 +163,7 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) + "; ReflectionAccessFilter does not permit using reflection or Unsafe. Register an" + " InstanceCreator or a TypeAdapter for this type or adjust the access filter to" + " allow using reflection."; - return () -> { - throw new JsonIOException(message); - }; + return new ExceptionObjectConstructor<>(message); } // finally try unsafe @@ -191,10 +185,10 @@ private static ObjectConstructor newSpecialCollectionConstructor( T set = (T) EnumSet.noneOf((Class) elementType); return set; } else { - throw new JsonIOException("Invalid EnumSet type: " + type.toString()); + throw new JsonIOException("Invalid EnumSet type: " + type); } } else { - throw new JsonIOException("Invalid EnumSet type: " + type.toString()); + throw new JsonIOException("Invalid EnumSet type: " + type); } }; } @@ -209,10 +203,10 @@ else if (rawType == EnumMap.class) { T map = (T) new EnumMap((Class) elementType); return map; } else { - throw new JsonIOException("Invalid EnumMap type: " + type.toString()); + throw new JsonIOException("Invalid EnumMap type: " + type); } } else { - throw new JsonIOException("Invalid EnumMap type: " + type.toString()); + throw new JsonIOException("Invalid EnumMap type: " + type); } }; } @@ -250,9 +244,7 @@ private static ObjectConstructor newDefaultConstructor( + " constructor is not accessible and ReflectionAccessFilter does not permit making" + " it accessible. Register an InstanceCreator or a TypeAdapter for this type, change" + " the visibility of the constructor or adjust the access filter."; - return () -> { - throw new JsonIOException(message); - }; + return new ExceptionObjectConstructor<>(message); } // Only try to make accessible if allowed; in all other cases checks above should @@ -260,20 +252,7 @@ private static ObjectConstructor newDefaultConstructor( if (filterResult == FilterResult.ALLOW) { String exceptionMessage = ReflectionHelper.tryMakeAccessible(constructor); if (exceptionMessage != null) { - /* - * Create ObjectConstructor which throws exception. - * This keeps backward compatibility (compared to returning `null` which - * would then choose another way of creating object). - * And it supports types which are only serialized but not deserialized - * (compared to directly throwing exception here), e.g. when runtime type - * of object is inaccessible, but compile-time type is accessible. - */ - return () -> { - // New exception is created every time to avoid keeping reference - // to exception with potentially long stack trace, causing a - // memory leak - throw new JsonIOException(exceptionMessage); - }; + return new ExceptionObjectConstructor<>(exceptionMessage); } } @@ -333,24 +312,24 @@ private static ObjectConstructor newDefaultImplementationConstructor( return null; } - private static ObjectConstructor> newCollectionConstructor( + private static ObjectConstructor> newCollectionConstructor( Class rawType) { // First try List implementation if (rawType.isAssignableFrom(ArrayList.class)) { - return () -> new ArrayList<>(); + return ArrayList::new; } // Then try Set implementation else if (rawType.isAssignableFrom(LinkedHashSet.class)) { - return () -> new LinkedHashSet<>(); + return LinkedHashSet::new; } // Then try SortedSet / NavigableSet implementation else if (rawType.isAssignableFrom(TreeSet.class)) { - return () -> new TreeSet<>(); + return TreeSet::new; } // Then try Queue implementation else if (rawType.isAssignableFrom(ArrayDeque.class)) { - return () -> new ArrayDeque<>(); + return ArrayDeque::new; } // Was unable to create matching Collection constructor @@ -370,7 +349,7 @@ private static boolean hasStringKeyType(Type mapType) { return GsonTypes.getRawType(typeArguments[0]) == String.class; } - private static ObjectConstructor> newMapConstructor( + private static ObjectConstructor> newMapConstructor( Type type, Class rawType) { // First try Map implementation /* @@ -378,21 +357,24 @@ private static ObjectConstructor> newMap * values for older JDKs; use own LinkedTreeMap instead */ if (rawType.isAssignableFrom(LinkedTreeMap.class) && hasStringKeyType(type)) { + // Must use lambda instead of method reference (`LinkedTreeMap::new`) here, otherwise this + // causes an exception when Gson is used by a custom system class loader, see + // https://github.com/google/gson/pull/2864#issuecomment-3528623716 return () -> new LinkedTreeMap<>(); } else if (rawType.isAssignableFrom(LinkedHashMap.class)) { - return () -> new LinkedHashMap<>(); + return LinkedHashMap::new; } // Then try SortedMap / NavigableMap implementation else if (rawType.isAssignableFrom(TreeMap.class)) { - return () -> new TreeMap<>(); + return TreeMap::new; } // Then try ConcurrentMap implementation else if (rawType.isAssignableFrom(ConcurrentHashMap.class)) { - return () -> new ConcurrentHashMap<>(); + return ConcurrentHashMap::new; } // Then try ConcurrentNavigableMap implementation else if (rawType.isAssignableFrom(ConcurrentSkipListMap.class)) { - return () -> new ConcurrentSkipListMap<>(); + return ConcurrentSkipListMap::new; } // Was unable to create matching Map constructor @@ -431,12 +413,7 @@ private ObjectConstructor newUnsafeAllocator(Class rawType) { " Or adjust your R8 configuration to keep the no-args constructor of the class."; } - // Separate effectively final variable to allow usage in the lambda below - String exceptionMessageF = exceptionMessage; - - return () -> { - throw new JsonIOException(exceptionMessageF); - }; + return new ExceptionObjectConstructor<>(exceptionMessage); } } @@ -444,4 +421,43 @@ private ObjectConstructor newUnsafeAllocator(Class rawType) { public String toString() { return instanceCreators.toString(); } + + /* + * Create ObjectConstructor which throws exception. + * This keeps backward compatibility (compared to returning `null` which + * would then choose another way of creating object). + * And it supports types which are only serialized but not deserialized + * (compared to directly throwing exception here), e.g. when runtime type + * of object is inaccessible, but compile-time type is accessible. + */ + private static final class ExceptionObjectConstructor implements ObjectConstructor { + private final String exceptionMessage; + + ExceptionObjectConstructor(String exceptionMessage) { + this.exceptionMessage = exceptionMessage; + } + + @Override + public T construct() { + // New exception is created every time to avoid keeping reference + // to exception with potentially long stack trace, causing a + // memory leak + throw new JsonIOException(exceptionMessage); + } + } + + private static final class InstanceCreatorConstructor implements ObjectConstructor { + private final InstanceCreator instanceCreator; + private final Type type; + + InstanceCreatorConstructor(InstanceCreator instanceCreator, Type type) { + this.instanceCreator = instanceCreator; + this.type = type; + } + + @Override + public T construct() { + return instanceCreator.createInstance(type); + } + } } From 76cdf98d7a139cbc44e46f70bc5017c40171e995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=A8=E8=91=89=20Scarlet?= <93977077+MukjepScarlet@users.noreply.github.com> Date: Wed, 26 Nov 2025 11:25:21 +0800 Subject: [PATCH 2/3] Update comments Co-authored-by: Marcono1234 --- .../com/google/gson/internal/ConstructorConstructor.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java index 0a02d2acab..f4b57b0957 100644 --- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java +++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java @@ -439,9 +439,8 @@ private static final class ExceptionObjectConstructor implements ObjectConstr @Override public T construct() { - // New exception is created every time to avoid keeping reference - // to exception with potentially long stack trace, causing a - // memory leak + // New exception is created every time to avoid keeping a reference to an exception with potentially long stack trace, causing a memory leak + // (which would happen if the exception was already created when the `ExceptionObjectConstructor` is created) throw new JsonIOException(exceptionMessage); } } From f112c2b8c087aa075bc660949925a4354f6107e8 Mon Sep 17 00:00:00 2001 From: MukjepScarlet <93977077+mukjepscarlet@users.noreply.github.com> Date: Wed, 26 Nov 2025 11:27:52 +0800 Subject: [PATCH 3/3] spotless apply Co-authored-by: Marcono1234 --- .../gson/internal/ConstructorConstructor.java | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java index f4b57b0957..a76a0014e4 100644 --- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java +++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java @@ -143,7 +143,7 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) // of adjusting filter suggested below is irrelevant since it would not solve the problem String exceptionMessage = checkInstantiable(rawType); if (exceptionMessage != null) { - return new ExceptionObjectConstructor<>(exceptionMessage); + return new ThrowingObjectConstructor<>(exceptionMessage); } if (!allowUnsafe) { @@ -151,7 +151,7 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) "Unable to create instance of " + rawType + "; Register an InstanceCreator or a TypeAdapter for this type."; - return new ExceptionObjectConstructor<>(message); + return new ThrowingObjectConstructor<>(message); } // Consider usage of Unsafe as reflection, so don't use if BLOCK_ALL @@ -163,7 +163,7 @@ public ObjectConstructor get(TypeToken typeToken, boolean allowUnsafe) + "; ReflectionAccessFilter does not permit using reflection or Unsafe. Register an" + " InstanceCreator or a TypeAdapter for this type or adjust the access filter to" + " allow using reflection."; - return new ExceptionObjectConstructor<>(message); + return new ThrowingObjectConstructor<>(message); } // finally try unsafe @@ -244,7 +244,7 @@ private static ObjectConstructor newDefaultConstructor( + " constructor is not accessible and ReflectionAccessFilter does not permit making" + " it accessible. Register an InstanceCreator or a TypeAdapter for this type, change" + " the visibility of the constructor or adjust the access filter."; - return new ExceptionObjectConstructor<>(message); + return new ThrowingObjectConstructor<>(message); } // Only try to make accessible if allowed; in all other cases checks above should @@ -252,7 +252,7 @@ private static ObjectConstructor newDefaultConstructor( if (filterResult == FilterResult.ALLOW) { String exceptionMessage = ReflectionHelper.tryMakeAccessible(constructor); if (exceptionMessage != null) { - return new ExceptionObjectConstructor<>(exceptionMessage); + return new ThrowingObjectConstructor<>(exceptionMessage); } } @@ -413,7 +413,7 @@ private ObjectConstructor newUnsafeAllocator(Class rawType) { " Or adjust your R8 configuration to keep the no-args constructor of the class."; } - return new ExceptionObjectConstructor<>(exceptionMessage); + return new ThrowingObjectConstructor<>(exceptionMessage); } } @@ -422,25 +422,28 @@ public String toString() { return instanceCreators.toString(); } - /* - * Create ObjectConstructor which throws exception. - * This keeps backward compatibility (compared to returning `null` which - * would then choose another way of creating object). - * And it supports types which are only serialized but not deserialized - * (compared to directly throwing exception here), e.g. when runtime type - * of object is inaccessible, but compile-time type is accessible. + /** + * {@link ObjectConstructor} which always throws an exception. + * + *

This keeps backward compatibility, compared to using a {@code null} {@code + * ObjectConstructor}, which would then choose another way of creating the object. And it supports + * types which are only serialized but not deserialized (compared to directly throwing an + * exception when the {@code ObjectConstructor} is requested), e.g. when the runtime type of an + * object is inaccessible, but the compile-time type is accessible. */ - private static final class ExceptionObjectConstructor implements ObjectConstructor { + private static final class ThrowingObjectConstructor implements ObjectConstructor { private final String exceptionMessage; - ExceptionObjectConstructor(String exceptionMessage) { + ThrowingObjectConstructor(String exceptionMessage) { this.exceptionMessage = exceptionMessage; } @Override public T construct() { - // New exception is created every time to avoid keeping a reference to an exception with potentially long stack trace, causing a memory leak - // (which would happen if the exception was already created when the `ExceptionObjectConstructor` is created) + // New exception is created every time to avoid keeping a reference to an exception with + // potentially long stack trace, causing a memory leak + // (which would happen if the exception was already created when the + // `ExceptionObjectConstructor` is created) throw new JsonIOException(exceptionMessage); } }