changeset 3564:45241cff9d3a

8144733: Iterating over elements of a Scope can return spurious inner class elements Summary: When a Symbol is removed from a Scope while iterating over it, update the iterator as well to reflect the change. Reviewed-by: mcimadamore
author jlahoda
date Mon, 08 Aug 2016 17:22:44 +0200
parents 4c85d752a985
children aebfafc43714
files src/jdk.compiler/share/classes/com/sun/tools/javac/code/Scope.java test/tools/javac/scope/IterateAndRemove.java
diffstat 2 files changed, 140 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Scope.java	Thu Aug 04 17:48:48 2016 +0000
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Scope.java	Mon Aug 08 17:22:44 2016 +0200
@@ -296,6 +296,8 @@
          */
         int nelems = 0;
 
+        int removeCount = 0;
+
         /** Use as a "not-found" result for lookup.
          * Also used to mark deleted entries in the table.
          */
@@ -474,6 +476,8 @@
                 te = te.sibling;
             }
 
+            removeCount++;
+
             //notify listeners
             listeners.symbolRemoved(sym, this);
         }
@@ -569,15 +573,29 @@
                     return new Iterator<Symbol>() {
                         private ScopeImpl currScope = ScopeImpl.this;
                         private Scope.Entry currEntry = elems;
+                        private int seenRemoveCount = currScope.removeCount;
                         {
                             update();
                         }
 
                         public boolean hasNext() {
+                            if (seenRemoveCount != currScope.removeCount &&
+                                currEntry != null &&
+                                !currEntry.scope.includes(currEntry.sym)) {
+                                doNext(); //skip entry that is no longer in the Scope
+                                seenRemoveCount = currScope.removeCount;
+                            }
                             return currEntry != null;
                         }
 
                         public Symbol next() {
+                            if (!hasNext()) {
+                                throw new NoSuchElementException();
+                            }
+
+                            return doNext();
+                        }
+                        private Symbol doNext() {
                             Symbol sym = (currEntry == null ? null : currEntry.sym);
                             if (currEntry != null) {
                                 currEntry = currEntry.sibling;
@@ -596,6 +614,7 @@
                                 while (currEntry == null && currScope.next != null) {
                                     currScope = currScope.next;
                                     currEntry = currScope.elems;
+                                    seenRemoveCount = currScope.removeCount;
                                     skipToNextMatchingEntry();
                                 }
                             }
@@ -618,13 +637,26 @@
                 public Iterator<Symbol> iterator() {
                      return new Iterator<Symbol>() {
                         Scope.Entry currentEntry = lookup(name, sf);
+                        int seenRemoveCount = currentEntry.scope != null ?
+                                currentEntry.scope.removeCount : -1;
 
                         public boolean hasNext() {
+                            if (currentEntry.scope != null &&
+                                seenRemoveCount != currentEntry.scope.removeCount &&
+                                !currentEntry.scope.includes(currentEntry.sym)) {
+                                doNext(); //skip entry that is no longer in the Scope
+                            }
                             return currentEntry.scope != null &&
                                     (lookupKind == RECURSIVE ||
                                      currentEntry.scope == ScopeImpl.this);
                         }
                         public Symbol next() {
+                            if (!hasNext()) {
+                                throw new NoSuchElementException();
+                            }
+                            return doNext();
+                        }
+                        private Symbol doNext() {
                             Scope.Entry prevEntry = currentEntry;
                             currentEntry = currentEntry.next(sf);
                             return prevEntry.sym;
@@ -686,9 +718,9 @@
         /** The entry's scope.
          *  scope == null   iff   this == sentinel
          */
-        public Scope scope;
+        public ScopeImpl scope;
 
-        public Entry(Symbol sym, Entry shadowed, Entry sibling, Scope scope) {
+        public Entry(Symbol sym, Entry shadowed, Entry sibling, ScopeImpl scope) {
             this.sym = sym;
             this.shadowed = shadowed;
             this.sibling = sibling;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/scope/IterateAndRemove.java	Mon Aug 08 17:22:44 2016 +0200
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2016, 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 8144733
+ * @summary Verify that Scope.remove removes the Symbol also from already running iterations.
+ * @modules jdk.compiler/com.sun.tools.javac.code
+ *          jdk.compiler/com.sun.tools.javac.util
+ */
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.function.Function;
+
+import com.sun.tools.javac.code.Scope;
+import com.sun.tools.javac.code.Scope.WriteableScope;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.PackageSymbol;
+import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.Name;
+import com.sun.tools.javac.util.Names;
+
+public class IterateAndRemove {
+    public static void main(String... args) {
+        new IterateAndRemove().run();
+    }
+
+    void run() {
+        Context ctx = new Context();
+        Names names = Names.instance(ctx);
+        Symbol root = new PackageSymbol(names.empty, null);
+        Name one = names.fromString("1");
+        PackageSymbol sym1 = new PackageSymbol(one, new PackageSymbol(names.fromString("a"), root));
+        PackageSymbol sym2 = new PackageSymbol(one, new PackageSymbol(names.fromString("b"), root));
+        PackageSymbol sym3 = new PackageSymbol(one, new PackageSymbol(names.fromString("c"), root));
+        List<Symbol> symbols = Arrays.asList(sym1, sym2, sym3);
+
+        List<Function<Scope, Iterable<Symbol>>> getters = Arrays.asList(
+                scope -> scope.getSymbols(),
+                scope -> scope.getSymbolsByName(one)
+        );
+        for (Function<Scope, Iterable<Symbol>> scope2Content : getters) {
+            for (int removeAt : new int[] {0, 1, 2, 3}) {
+                for (Symbol removeWhat : new Symbol[] {sym1, sym2, sym3}) {
+                    WriteableScope s = WriteableScope.create(root);
+
+                    symbols.forEach(s :: enter);
+
+                    Iterator<Symbol> it = scope2Content.apply(s).iterator();
+                    List<PackageSymbol> actual = new ArrayList<>();
+                    int count = 0;
+
+                    while (true) {
+                        if (count++ == removeAt)
+                            s.remove(removeWhat);
+                        if (!it.hasNext())
+                            break;
+                        actual.add((PackageSymbol) it.next());
+                    }
+
+                    List<Symbol> copy = new ArrayList<>(symbols);
+
+                    Collections.reverse(copy);
+
+                    count = 0;
+
+                    while (true) {
+                        if (count == removeAt && copy.indexOf(removeWhat) >= count)
+                            copy.remove(removeWhat);
+                        count++;
+                        if (count >= copy.size())
+                            break;
+                    }
+
+                    if (!copy.equals(actual)) {
+                        throw new AssertionError("differs: actual: " + actual + "; expected: " + copy);
+                    }
+                }
+            }
+        }
+    }
+}
\ No newline at end of file