# HG changeset patch # User jlahoda # Date 1470669764 -7200 # Node ID fe6c006184347976e45818ec0dbcac5cfa29346b # Parent 19c384f77e5125e18006626d78c6118759347f04 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 diff -r 19c384f77e51 -r fe6c00618434 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Scope.java --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Scope.java Thu Aug 04 17:48:48 2016 +0000 +++ b/langtools/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() { 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 iterator() { return new Iterator() { 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; diff -r 19c384f77e51 -r fe6c00618434 langtools/test/tools/javac/scope/IterateAndRemove.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/langtools/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 symbols = Arrays.asList(sym1, sym2, sym3); + + List>> getters = Arrays.asList( + scope -> scope.getSymbols(), + scope -> scope.getSymbolsByName(one) + ); + for (Function> 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 it = scope2Content.apply(s).iterator(); + List actual = new ArrayList<>(); + int count = 0; + + while (true) { + if (count++ == removeAt) + s.remove(removeWhat); + if (!it.hasNext()) + break; + actual.add((PackageSymbol) it.next()); + } + + List 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