Merge pull request #4061 from maxcom/SI-8924-list-iterator-oom

SI-8924 don't hold reference to list in iterator

2 files changed, 59 insertions, 4 deletions

diff --git a/src/library/scala/collection/LinearSeqLike.scala b/src/library/scala/collection/LinearSeqLike.scala
index 3288599221..96e2135fd1 100644
--- a/src/library/scala/collection/LinearSeqLike.scala
+++ b/src/library/scala/collection/LinearSeqLike.scala
@@ -46,12 +46,18 @@ trait LinearSeqLike[+A, +Repr <: LinearSeqLike[A, Repr]] extends SeqLike[A, Repr
         val result = these.head; these = these.tail; result
       } else Iterator.empty.next()
 
-    /** Have to clear `these` so the iterator is exhausted like
-     *  it would be without the optimization.
-     */
     override def toList: List[A] = {
+      /* Have to clear `these` so the iterator is exhausted like
+       * it would be without the optimization.
+       *
+       * Calling "newBuilder.result()" in toList method
+       * prevents original seq from garbage collection,
+       * so we use these.take(0) here.
+       *
+       * Check SI-8924 for details
+       */
       val xs = these.toList
-      these = newBuilder.result()
+      these = these.take(0)
       xs
     }
   }
diff --git a/test/junit/scala/collection/immutable/ListTest.scala b/test/junit/scala/collection/immutable/ListTest.scala
new file mode 100644
index 0000000000..1006801029
--- /dev/null
+++ b/test/junit/scala/collection/immutable/ListTest.scala
@@ -0,0 +1,49 @@
+package scala.collection.immutable
+
+import org.junit.{Assert, Test}
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+import scala.ref.WeakReference
+
+@RunWith(classOf[JUnit4])
+class ListTest {
+  /**
+   * Test that empty iterator does not hold reference
+   * to complete List
+   */
+  @Test
+  def testIteratorGC(): Unit = {
+    var num = 0
+    var emptyIterators = Seq.empty[(Iterator[Int], WeakReference[List[Int]])]
+
+    do {
+      val list = List.fill(10000)(num)
+      val ref = WeakReference(list)
+
+      val i = list.iterator
+
+      while (i.hasNext) i.next()
+
+      emptyIterators = (i, ref) +: emptyIterators
+
+      num+=1
+    } while (emptyIterators.forall(_._2.get.isDefined) && num<1000)
+
+    // check something is result to protect from JIT optimizations
+    for ((i, _) <- emptyIterators) {
+      Assert.assertTrue(i.isEmpty)
+    }
+
+    // await gc up to ~5 seconds
+    var forceLoops = 50
+    while (emptyIterators.forall(_._2.get.isDefined) && forceLoops>0) {
+      System.gc()
+      Thread.sleep(100)
+      forceLoops -= 1
+    }
+
+    // real assertion
+    Assert.assertTrue(emptyIterators.exists(_._2.get.isEmpty))
+  }
+}