8186961: Class.getFields() does not return fields of previously visited super interfaces/classes

Reviewed-by: mchung, redestad

src/java.base/share/classes/java/lang/Class.java

@@ -53,12 +53,11 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.StringJoiner;
 
 import jdk.internal.HotSpotIntrinsicCandidate;
@@ -1771,7 +1770,7 @@ public final class Class<T> implements java.io.Serializable,
         if (sm != null) {
             checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true);
         }
-        return copyFields(privateGetPublicFields(null));
+        return copyFields(privateGetPublicFields());
     }
 
 
@@ -3026,7 +3025,7 @@ public final class Class<T> implements java.io.Serializable,
     // Returns an array of "root" fields. These Field objects must NOT
     // be propagated to the outside world, but must instead be copied
     // via ReflectionFactory.copyField.
-    private Field[] privateGetPublicFields(Set<Class<?>> traversedInterfaces) {
+    private Field[] privateGetPublicFields() {
         Field[] res;
         ReflectionData<T> rd = reflectionData();
         if (rd != null) {
@@ -3034,35 +3033,25 @@ public final class Class<T> implements java.io.Serializable,
             if (res != null) return res;
         }
 
-        // No cached value available; compute value recursively.
-        // Traverse in correct order for getField().
-        List<Field> fields = new ArrayList<>();
-        if (traversedInterfaces == null) {
-            traversedInterfaces = new HashSet<>();
-        }
+        // Use a linked hash set to ensure order is preserved and
+        // fields from common super interfaces are not duplicated
+        LinkedHashSet<Field> fields = new LinkedHashSet<>();
 
         // Local fields
-        Field[] tmp = privateGetDeclaredFields(true);
-        addAll(fields, tmp);
+        addAll(fields, privateGetDeclaredFields(true));
 
         // Direct superinterfaces, recursively
-        for (Class<?> c : getInterfaces()) {
-            if (!traversedInterfaces.contains(c)) {
-                traversedInterfaces.add(c);
-                addAll(fields, c.privateGetPublicFields(traversedInterfaces));
-            }
+        for (Class<?> si : getInterfaces()) {
+            addAll(fields, si.privateGetPublicFields());
         }
 
         // Direct superclass, recursively
-        if (!isInterface()) {
-            Class<?> c = getSuperclass();
-            if (c != null) {
-                addAll(fields, c.privateGetPublicFields(traversedInterfaces));
-            }
+        Class<?> sc = getSuperclass();
+        if (sc != null) {
+            addAll(fields, sc.privateGetPublicFields());
         }
 
-        res = new Field[fields.size()];
-        fields.toArray(res);
+        res = fields.toArray(new Field[0]);
         if (rd != null) {
             rd.publicFields = res;
         }

test/jdk/java/lang/reflect/StaticFieldsOnInterface.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8186961
+ * @run main/othervm StaticFieldsOnInterface C
+ * @run main/othervm StaticFieldsOnInterface D
+ * @run main/othervm StaticFieldsOnInterface Y
+ */
+
+public class StaticFieldsOnInterface {
+    /*
+            A
+           / \
+          B  C
+          \  /
+           D
+
+        Interface A has a public field
+        Ensure B, C, D only report exactly one public field
+
+          A
+         /
+        X A
+        |/
+        Y
+
+        Ensure class Y, extending class X, reports exactly one public field
+     */
+
+    public interface A {
+        public static final int CONSTANT = 42;
+    }
+
+    public interface B extends A {
+    }
+
+    public interface C extends A {
+    }
+
+    public interface D extends B, C {
+    }
+
+    static class X implements A {}
+    static class Y extends X implements A {}
+
+    public static void main(String[] args) throws Exception {
+        char first = 'C';
+        if (args.length > 0) {
+            first = args[0].charAt(0);
+        }
+
+        assertOneField(A.class);
+        // D first
+        if (first == 'D') {
+            assertOneField(D.class);
+            assertOneField(C.class);
+        }
+        // C first
+        else if (first == 'C') {
+            assertOneField(C.class);
+            assertOneField(D.class);
+        }
+        else {
+            assertOneField(Y.class);
+        }
+    }
+
+    static void assertOneField(Class<?> c) {
+        int nfs = c.getFields().length;
+        if (nfs != 1) {
+            throw new AssertionError(String.format(
+                    "Class %s does not have exactly one field: %d", c.getName(), nfs));
+        }
+    }
+}