Fix a bug where GWT-RPC would not generate some covariant array types, possibly causing inconsistent serialization policies. Includes a test.
Fixes issue 7791
Review at http://gwt-code-reviews.appspot.com/1869804
Review by: cromwellian@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11379 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java b/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java
index 3b74b3a..8c8f90f 100644
--- a/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java
+++ b/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java
@@ -134,10 +134,10 @@
private boolean instantiableSubtypes;
/**
- * All instantiable types found when this type was quaried, including the
- * type itself.
+ * All instantiable types found when this type was queried, including the
+ * type itself. (Null until calculated.)
*/
- private Set<JClassType> instantiableTypes = new HashSet<JClassType>();
+ private Set<JClassType> instantiableTypes;
/**
* Custom field serializer or <code>null</code> if there isn't one.
@@ -178,17 +178,6 @@
}
}
- /**
- * Returns the internal set of instantiable types for this TIC.
- * Modifications to this set are immediately recorded into the TIC as well.
- * TODO(spoon) maybe pass the TIC around instead of the set? then there
- * could be addInstantiableType(JClassType) instead of speccing this to be
- * mutable.
- */
- public Set<JClassType> getInstantiableTypes() {
- return instantiableTypes;
- }
-
public TypePath getPath() {
return path;
}
@@ -819,29 +808,40 @@
if (!entrySucceeded) {
problems.report(logger, TreeLogger.ERROR, TreeLogger.INFO);
} else {
- if (problems.hasFatalProblems()) {
- entrySucceeded = false;
- problems.reportFatalProblems(logger, TreeLogger.ERROR);
- }
- // if entrySucceeded, there may still be "warning" problems, but too
- // often they're expected (e.g. non-instantiable subtypes of List), so
- // we log them at DEBUG.
- // TODO(fabbott): we could blacklist or graylist those types here, so
- // instantiation during code generation would flag them for us.
- problems.report(logger, TreeLogger.DEBUG, TreeLogger.DEBUG);
+ maybeReport(logger, problems);
}
-
- allSucceeded &= entrySucceeded;
+ allSucceeded &= entrySucceeded & !problems.hasFatalProblems();
}
if (!allSucceeded) {
throw new UnableToCompleteException();
}
+ assertNothingPending();
- for (TypeInfoComputed tic : typeToTypeInfoComputed.values()) {
- assert (!tic.isPendingInstantiable());
+ // Add covariant arrays in a separate pass. We want to ensure that nothing is pending
+ // so that the leaf type's instantiableTypes variable is ready (if it's computed at all)
+ // and all of the leaf's subtypes are ready.
+ // (Copy values to avoid concurrent modification.)
+ List<TypeInfoComputed> ticsToCheck = new ArrayList<TypeInfoComputed>();
+ ticsToCheck.addAll(typeToTypeInfoComputed.values());
+ for (TypeInfoComputed tic : ticsToCheck) {
+ JArrayType type = tic.getType().isArray();
+ if (type != null && tic.instantiable) {
+ ProblemReport problems = new ProblemReport();
+ problems.setContextType(type);
+
+ markArrayTypes(logger, type, tic.getPath(), problems);
+
+ maybeReport(logger, problems);
+ allSucceeded &= !problems.hasFatalProblems();
+ }
}
+ if (!allSucceeded) {
+ throw new UnableToCompleteException();
+ }
+ assertNothingPending();
+
pruneUnreachableTypes();
logReachableTypes(logger);
@@ -998,12 +998,15 @@
// TreeLogger subtypesLogger = localLogger.branch(TreeLogger.DEBUG,
// "Analyzing subclasses:", null);
tic = ensureTypeInfoComputed(classType, path);
+ Set<JClassType> instantiableTypes = new HashSet<JClassType>();
boolean anySubtypes =
- checkSubtypes(localLogger, originalType, tic.getInstantiableTypes(), path, problems);
+ checkSubtypes(localLogger, originalType, instantiableTypes, path, problems);
if (!tic.isDone()) {
tic.setInstantiableSubtypes(anySubtypes);
tic.setInstantiable(false);
}
+ // Don't publish this until complete to ensure nobody depends on partial results.
+ tic.instantiableTypes = instantiableTypes;
return tic;
}
@@ -1021,6 +1024,14 @@
return shouldConsiderFieldsForSerialization(type, typeFilter, problems);
}
+ private void assertNothingPending() {
+ if (getClass().desiredAssertionStatus()) {
+ for (TypeInfoComputed tic : typeToTypeInfoComputed.values()) {
+ assert (!tic.isPendingInstantiable());
+ }
+ }
+ }
+
/**
* Consider any subtype of java.lang.Object which qualifies for serialization.
*/
@@ -1058,12 +1069,16 @@
return checkArrayInstantiable(logger, arrayType, path, problems);
}
- JClassType leafClass = leafType.isClassOrInterface();
+ TypeInfoComputed tic = ensureTypeInfoComputed(array, path);
+ if (tic.isDone() || tic.isPendingInstantiable()) {
+ return tic;
+ }
+ tic.setPendingInstantiable();
+
JTypeParameter isLeafTypeParameter = leafType.isTypeParameter();
if (isLeafTypeParameter != null && !typeParametersInRootTypes.contains(isLeafTypeParameter)) {
// Don't deal with non root type parameters, but make a TIC entry to
// save time if it recurs. We assume they're indirectly instantiable.
- TypeInfoComputed tic = ensureTypeInfoComputed(array, path);
tic.setInstantiableSubtypes(true);
tic.setInstantiable(false);
return tic;
@@ -1072,18 +1087,12 @@
if (!isAllowedByFilter(array, problems)) {
// Don't deal with filtered out types either, but make a TIC entry to
// save time if it recurs. We assume they're not instantiable.
- TypeInfoComputed tic = ensureTypeInfoComputed(array, path);
tic.setInstantiable(false);
return tic;
}
- TypeInfoComputed tic = ensureTypeInfoComputed(array, path);
- if (tic.isDone()) {
- return tic;
- } else if (tic.isPendingInstantiable()) {
- return tic;
- }
- tic.setPendingInstantiable();
+ // An array is instantiable provided that any leaf subtype is instantiable.
+ // (Ignores the possibility of empty arrays of non-instantiable types.)
TreeLogger branch = logger.branch(TreeLogger.DEBUG, "Analyzing component type:", null);
@@ -1091,32 +1100,6 @@
computeTypeInstantiability(branch, leafType, TypePaths
.createArrayComponentPath(array, path), problems);
boolean succeeded = leafTic.hasInstantiableSubtypes();
- if (succeeded) {
- if (leafClass == null) {
- assert leafType.isPrimitive() != null;
- markArrayTypesInstantiable(leafType, array.getRank(), path);
- } else {
- TreeLogger covariantArrayLogger = logger.branch(TreeLogger.DEBUG, "Covariant array types");
-
- /*
- * Compute covariant arrays for arrays of reference types.
- */
- for (JClassType instantiableType : TypeHierarchyUtils.getAllTypesBetweenRootTypeAndLeaves(
- leafClass, leafTic.getInstantiableTypes())) {
- if (!isAccessibleToSerializer(instantiableType)) {
- // Skip types that are not accessible from a serializer
- continue;
- }
-
- if (covariantArrayLogger.isLoggable(TreeLogger.DEBUG)) {
- covariantArrayLogger.branch(TreeLogger.DEBUG, getArrayType(typeOracle, array.getRank(),
- instantiableType).getParameterizedQualifiedSourceName());
- }
-
- markArrayTypesInstantiable(instantiableType, array.getRank(), path);
- }
- }
- }
tic.setInstantiable(succeeded);
return tic;
@@ -1553,6 +1536,60 @@
}
}
+ /**
+ * Marks all covariant and lesser-ranked arrays as instantiable for all leaf types between
+ * the given array's leaf type and its instantiable subtypes. (Note: this adds O(S * R)
+ * array types to the output where S is the number of subtypes and R is the rank.)
+ * Prerequisite: The leaf type's tic and its subtypes must already be created.
+ */
+ private void markArrayTypes(TreeLogger logger, JArrayType array, TypePath path,
+ ProblemReport problems) {
+ logger = logger.branch(TreeLogger.DEBUG, "Adding array types for " + array);
+
+ JType leafType = array.getLeafType();
+ TypeInfoComputed leafTic = typeToTypeInfoComputed.get(leafType);
+ assert leafTic != null : "not computed: " + leafType;
+
+ JClassType leafClass = leafType.isClassOrInterface();
+ if (leafClass == null) {
+ // Simple case: no covariance, just lower ranks.
+ assert leafType.isPrimitive() != null;
+ markArrayTypesInstantiable(leafType, array.getRank(), path);
+ return;
+ }
+
+ TreeLogger covariantArrayLogger =
+ logger.branch(TreeLogger.DEBUG, "Covariant array types:");
+
+ Set<JClassType> instantiableTypes = leafTic.instantiableTypes;
+ if (instantiableTypes == null) {
+ // The types are there (due to a supertype) but the Set wasn't computed, so compute it now.
+ instantiableTypes = new HashSet<JClassType>();
+ List<JClassType> candidates =
+ getPossiblyInstantiableSubtypes(logger, leafClass, problems);
+ for (JClassType candidate : candidates) {
+ TypeInfoComputed tic = typeToTypeInfoComputed.get(candidate);
+ if (tic != null && tic.instantiable) {
+ instantiableTypes.add(candidate);
+ }
+ }
+ }
+ for (JClassType instantiableType : TypeHierarchyUtils.getAllTypesBetweenRootTypeAndLeaves(
+ leafClass, instantiableTypes)) {
+ if (!isAccessibleToSerializer(instantiableType)) {
+ // Skip types that are not accessible from a serializer
+ continue;
+ }
+
+ if (covariantArrayLogger.isLoggable(TreeLogger.DEBUG)) {
+ covariantArrayLogger.branch(TreeLogger.DEBUG, getArrayType(typeOracle, array.getRank(),
+ instantiableType).getParameterizedQualifiedSourceName());
+ }
+
+ markArrayTypesInstantiable(instantiableType, array.getRank(), path);
+ }
+ }
+
private boolean maybeInstantiable(TreeLogger logger, JClassType type, ProblemReport problems) {
boolean success =
canBeInstantiated(type, problems) && shouldConsiderFieldsForSerialization(type, problems);
@@ -1565,6 +1602,21 @@
return success;
}
+ /**
+ * Report problems if they are fatal or we're debugging.
+ */
+ private void maybeReport(TreeLogger logger, ProblemReport problems) {
+ if (problems.hasFatalProblems()) {
+ problems.reportFatalProblems(logger, TreeLogger.ERROR);
+ }
+ // if entrySucceeded, there may still be "warning" problems, but too
+ // often they're expected (e.g. non-instantiable subtypes of List), so
+ // we log them at DEBUG.
+ // TODO(fabbott): we could blacklist or graylist those types here, so
+ // instantiation during code generation would flag them for us.
+ problems.report(logger, TreeLogger.DEBUG, TreeLogger.DEBUG);
+ }
+
private boolean mightNotBeExposed(JGenericType baseType, int paramIndex) {
TypeParameterFlowInfo flowInfo = getFlowInfo(baseType, paramIndex);
return flowInfo.getMightNotBeExposed() || isManuallySerializable(baseType);
diff --git a/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java b/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java
index d5f01a0..ee3fcd1 100644
--- a/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java
+++ b/user/test/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilderTest.java
@@ -45,11 +45,13 @@
import com.google.gwt.user.rebind.rpc.testcases.client.ManualSerialization;
import com.google.gwt.user.rebind.rpc.testcases.client.NoSerializableTypes;
import com.google.gwt.user.rebind.rpc.testcases.client.NotAllSubtypesAreSerializable;
+import com.google.gwt.user.rebind.rpc.testcases.client.SubclassUsedInArray;
import junit.framework.TestCase;
import java.io.PrintWriter;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
@@ -1925,6 +1927,15 @@
}
/**
+ * Tests a case where a subtype is visited and then later used to find covariant array types.
+ * (Reproduces a caching issue that depends on the order in which types are visited.)
+ */
+ public void testSubclassUsedInArray() throws NotFoundException, UnableToCompleteException {
+ JClassType expected = arrayType(SubclassUsedInArray.LeafType.class);
+ checkSerializable(expected, SubclassUsedInArray.Base.class, SubclassUsedInArray.HasArray.class);
+ }
+
+ /**
* Tests subtypes that introduce new instantiable type parameters.
*
* @throws UnableToCompleteException
@@ -2366,6 +2377,72 @@
assertNotInstantiableOrFieldSerializable(so, a.getRawType());
}
+ /**
+ * Checks that a type is serializable when searching from the given roots.
+ * Also, check that the root order doesn't matter.
+ */
+ private void checkSerializable(JClassType expected, Class... roots)
+ throws UnableToCompleteException {
+
+ // find serializable types in forward and reverse order
+ SerializableTypeOracle forwardResult = findSerializable(roots);
+ roots = Arrays.copyOf(roots, roots.length);
+ Collections.reverse(Arrays.asList(roots));
+ SerializableTypeOracle reverseResult = findSerializable(roots);
+
+ // check that the expected type is serializable
+ boolean forwardOk = forwardResult.isSerializable(expected);
+ boolean reverseOk = reverseResult.isSerializable(expected);
+ if (!forwardOk && !reverseOk) {
+ fail(expected + " is not serializable from " + join(", ", roots) + " in either order");
+ }
+ if (!forwardOk || !reverseOk) {
+ fail(expected + " is not serializable from " + join(", ", roots) + " in both orders");
+ }
+
+ // also check that other serializable types are stable
+ checkSameSerializables(forwardResult, reverseResult);
+ }
+
+ /**
+ * Finds serializable types from the given roots (in order).
+ */
+ private SerializableTypeOracle findSerializable(Class... roots)
+ throws UnableToCompleteException {
+ TreeLogger logger = createLogger();
+ TypeOracle oracle = getTestTypeOracle();
+ SerializableTypeOracleBuilder builder =
+ createSerializableTypeOracleBuilder(logger, oracle);
+ for (Class root : roots) {
+ builder.addRootType(logger, oracle.findType(root.getCanonicalName()));
+ }
+ return builder.build(logger);
+ }
+
+ private void checkSameSerializables(SerializableTypeOracle first, SerializableTypeOracle second) {
+ String firstTypes = join("\n", first.getSerializableTypes());
+ String secondTypes = join("\n", second.getSerializableTypes());
+ assertEquals("type oracles differ", firstTypes, secondTypes);
+ }
+
+ private JClassType arrayType(Class<?> itemType)
+ throws UnableToCompleteException, NotFoundException {
+ TypeOracle typeOracle = getTestTypeOracle();
+ JClassType leaf = typeOracle.getType(itemType.getCanonicalName());
+ return typeOracle.getArrayType(leaf);
+ }
+
+ private <T> String join(String delimiter, T... items) {
+ StringBuilder result = new StringBuilder();
+ for (int i = 0; i < items.length; i++) {
+ if (i > 0) {
+ result.append(delimiter);
+ }
+ result.append(items[i]);
+ }
+ return result.toString();
+ }
+
private JClassType[] makeArray(JClassType... elements) {
return elements;
}
diff --git a/user/test/com/google/gwt/user/rebind/rpc/testcases/client/SubclassUsedInArray.java b/user/test/com/google/gwt/user/rebind/rpc/testcases/client/SubclassUsedInArray.java
new file mode 100644
index 0000000..903e8d9
--- /dev/null
+++ b/user/test/com/google/gwt/user/rebind/rpc/testcases/client/SubclassUsedInArray.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.user.rebind.rpc.testcases.client;
+
+import java.io.Serializable;
+
+/**
+ * Sets up a situation where an array type has a covariant type but its leaf's subtypes aren't
+ * cached.
+ */
+public class SubclassUsedInArray {
+
+ /** Root type. */
+ public interface Base extends Serializable {
+ }
+
+ /** Array's leaf type. Not a root so its subtypes aren't cached. */
+ public static class Subtype implements Base {
+ // Has a field so it's not trivially serializable and full analysis will be done.
+ private FieldType fieldType;
+ }
+
+ /** A subtype to trigger a covariant array type. */
+ public static class LeafType extends Subtype {
+ }
+
+ /** Root type to trigger the array. */
+ public static class HasArray implements Serializable {
+ private Subtype[] array;
+ }
+
+ /** Just a field. */
+ public static class FieldType implements Serializable {
+ }
+}
