SI-8976 MutableList.tail.iterator.size is length

The previous behavior was to iterate over the mutated
list of arbitrary length. The previous iteration of
the iterator would also iterate the terminal element
of the list without halting.

This is fixed by capping the length of iterator.

That is OK because mutating the list by adding to it during
iteration is not recommended.

For good measure, the exhausted iterator does not hold
a reference to any remaining tail.

A test is added that will no doubt be superseded by the QCC tests.
(Quasi-Comprehensive Collections.)

The test just checks that the extra tail is not strongly
reachable from the iterator. If the garbage collector happens
to kick in and determine that the object is weakly reachable,
then the check terminates early.

4 files changed, 113 insertions, 6 deletions

diff --git a/test/junit/scala/collection/mutable/MutableListTest.scala b/test/junit/scala/collection/mutable/MutableListTest.scala
new file mode 100644
index 0000000000..ac6d30def0
--- /dev/null
+++ b/test/junit/scala/collection/mutable/MutableListTest.scala
@@ -0,0 +1,37 @@
+package scala.collection.mutable
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import org.junit.Assert._
+
+import scala.tools.testing.AssertUtil._
+
+@RunWith(classOf[JUnit4])
+class MutableListTest {
+  
+  // Tests SI-8976
+  @Test def tailIteratorMustTerminateAtLength(): Unit = {
+    val is = MutableList(1,2,3)
+    val tl = is.tail
+    assertEquals(tl.length, tl.iterator.length)
+    is += 5
+    assertEquals(tl.length, tl.iterator.length)
+    assertSameElements(tl, tl.iterator)
+  }
+  @Test def iteratorMustFailEventually(): Unit = assertThrows[NoSuchElementException] {
+    MutableList[Unit]().iterator.next()
+  }
+  // was: Root empty iterator held reference
+  @Test def iteratorMustNotHoldOntoLast(): Unit = {
+    val is = MutableList(Some(1), Some(2))
+    val it = is.iterator
+    val x  = Some(3)
+    is += x
+    assertNotReachable(x, it) {
+      it.next()
+      it.next()
+    }
+    assertTrue(it.isEmpty)
+  }
+}
diff --git a/test/junit/scala/tools/testing/AssertThrowsTest.scala b/test/junit/scala/tools/testing/AssertThrowsTest.scala
index d91e450bac..76758f51d2 100644
--- a/test/junit/scala/tools/testing/AssertThrowsTest.scala
+++ b/test/junit/scala/tools/testing/AssertThrowsTest.scala
@@ -38,6 +38,6 @@ class AssertThrowsTest {
     } catch {
       case e: AssertionError => return
     }
-    assert(false, "assertThrows should error if the tested expression does not throw anything")
+    fail("assertThrows should error if the tested expression does not throw anything")
   }
 }
diff --git a/test/junit/scala/tools/testing/AssertUtil.scala b/test/junit/scala/tools/testing/AssertUtil.scala
index 83a637783f..d29f9a473f 100644
--- a/test/junit/scala/tools/testing/AssertUtil.scala
+++ b/test/junit/scala/tools/testing/AssertUtil.scala
@@ -2,18 +2,42 @@ package scala.tools
 package testing
 
 import org.junit.Assert
-import Assert.fail
+import Assert._
 import scala.runtime.ScalaRunTime.stringOf
 import scala.collection.{ GenIterable, IterableLike }
+import scala.collection.JavaConverters._
+import scala.collection.mutable
+import java.lang.ref._
+import java.lang.reflect._
+import java.util.IdentityHashMap
 
 /** This module contains additional higher-level assert statements
  *  that are ultimately based on junit.Assert primitives.
  */
 object AssertUtil {
-  /**
-   * Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message
-   * satisfies the `checkMessage` predicate.
-   * If any other exception will be re-thrown.
+  private final val timeout = 60 * 1000L                 // wait a minute
+
+  private implicit class `ref helper`[A](val r: Reference[A]) extends AnyVal {
+    def isEmpty: Boolean  = r.get == null
+    def nonEmpty: Boolean = !isEmpty
+  }
+  private implicit class `class helper`(val clazz: Class[_]) extends AnyVal {
+    def allFields: List[Field] = {
+      def loop(k: Class[_]): List[Field] =
+        if (k == null) Nil
+        else k.getDeclaredFields.toList ::: loop(k.getSuperclass)
+      loop(clazz)
+    }
+  }
+  private implicit class `field helper`(val f: Field) extends AnyVal {
+    def follow(o: AnyRef): AnyRef = {
+      f setAccessible true
+      f get o
+    }
+  }
+
+  /** Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message
+   *  satisfies the `checkMessage` predicate. If any other exception will be re-thrown.
    */
   def assertThrows[T <: Throwable](f: => Any,
                                    checkMessage: String => Boolean = s => true)
@@ -41,4 +65,29 @@ object AssertUtil {
    */
   def assertSameElements[A, B >: A](expected: IterableLike[A, _], actual: Iterator[B]): Unit =
     assertSameElements(expected, actual.toList, "")
+
+  /** Value is not strongly reachable from roots after body is evaluated.
+   */
+  def assertNotReachable[A <: AnyRef](a: => A, roots: AnyRef*)(body: => Unit): Unit = {
+    val wkref = new WeakReference(a)
+    def refs(root: AnyRef): mutable.Set[AnyRef] = {
+      val seen = new IdentityHashMap[AnyRef, Unit]
+      def loop(o: AnyRef): Unit =
+        if (wkref.nonEmpty && o != null && !seen.containsKey(o)) {
+          seen.put(o, ())
+          for {
+            f <- o.getClass.allFields
+            if !Modifier.isStatic(f.getModifiers)
+            if !f.getType.isPrimitive
+            if !classOf[Reference[_]].isAssignableFrom(f.getType)
+          } loop(f follow o)
+        }
+      loop(root)
+      seen.keySet.asScala
+    }
+    body
+    for (r <- roots if wkref.nonEmpty) {
+      assertFalse(s"Root $r held reference", refs(r) contains wkref.get)
+    }
+  }
 }
diff --git a/test/junit/scala/tools/testing/AssertUtilTest.scala b/test/junit/scala/tools/testing/AssertUtilTest.scala
new file mode 100644
index 0000000000..03d8815ab2
--- /dev/null
+++ b/test/junit/scala/tools/testing/AssertUtilTest.scala
@@ -0,0 +1,21 @@
+package scala.tools
+package testing
+
+import org.junit.Assert._
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import AssertUtil._
+
+import java.lang.ref._
+
+@RunWith(classOf[JUnit4])
+class AssertUtilTest {
+
+  @Test def reachableIgnoresReferences(): Unit = {
+    class Holder[A](val ref: SoftReference[A])
+    val o = new Object
+    val r = new SoftReference(o)
+    assertNotReachable(o, new Holder(r)) { }
+  }
+}