From d3b5db7fee66092c063d3306c383660ae83e2fca Mon Sep 17 00:00:00 2001
From: Sami Nurminen <snurmine@gmail.com>
Date: Mon, 6 Jul 2015 23:49:14 +0300
Subject: [PATCH] Fix for the issue
 https://issues.scala-lang.org/browse/SI-9010. This solution takes some ideas
 / inspiration from the discussion in
 https://issues.scala-lang.org/browse/SI-4929.

Added LastNoSuccessHelper object into Parsers trait to
encapsulate tracking of the last NoSuccess result when
tracking is actually needed. Idea is that the last NoSuccess
result is only needed inside phrase parsers
to give better error messages i.e. it is not needed for "correctness".

Null is used as initial value for context:DynamicValue variable in
LastNoSuccessHelper object as a marker if computation is outside
"trackLastNoSuccessWithInitialValue" and therefore tracking
of NoSuccess results is unnecessary.

Tracking of NoSuccess results is done only iff
computation goes trough "trackLastNoSuccessWithInitialValue",
because then context variable inside LastNoSuccess helper
gets None(non-null) value which enables tracking.
---
 .../util/parsing/combinator/Parsers.scala     | 40 +++++++++++++++----
 .../combinator/SI9010MemoryLeakTest.scala     | 40 +++++++++++++++++++
 2 files changed, 73 insertions(+), 7 deletions(-)
 create mode 100644 src/test/scala/scala/util/parsing/combinator/SI9010MemoryLeakTest.scala

diff --git a/src/main/scala/scala/util/parsing/combinator/Parsers.scala b/src/main/scala/scala/util/parsing/combinator/Parsers.scala
index 9fd60b3b..347ea2c2 100644
--- a/src/main/scala/scala/util/parsing/combinator/Parsers.scala
+++ b/src/main/scala/scala/util/parsing/combinator/Parsers.scala
@@ -156,14 +156,13 @@ trait Parsers {
     val successful = true
   }
 
-  private lazy val lastNoSuccessVar = new DynamicVariable[Option[NoSuccess]](None)
 
   /** A common super-class for unsuccessful parse results. */
   sealed abstract class NoSuccess(val msg: String, override val next: Input) extends ParseResult[Nothing] { // when we don't care about the difference between Failure and Error
     val successful = false
 
-    if (lastNoSuccessVar.value forall (v => !(next.pos < v.next.pos)))
-      lastNoSuccessVar.value = Some(this)
+    if (LastNoSuccessHelper.isLastNoSuccessTrackingEnabled && (LastNoSuccessHelper.value forall (v => !(next.pos < v.next.pos))))
+      LastNoSuccessHelper.value = Some(this)
 
     def map[U](f: Nothing => U) = this
     def mapPartial[U](f: PartialFunction[Nothing, U], error: Nothing => String): ParseResult[U] = this
@@ -908,14 +907,14 @@ trait Parsers {
    *           if `p` consumed all the input.
    */
   def phrase[T](p: Parser[T]) = new Parser[T] {
-    def apply(in: Input) = lastNoSuccessVar.withValue(None) {
+    def apply(in: Input) = LastNoSuccessHelper.trackLastNoSuccessWithInitialValue(None) {
       p(in) match {
       case s @ Success(out, in1) =>
         if (in1.atEnd)
           s
         else
-          lastNoSuccessVar.value filterNot { _.next.pos < in1.pos } getOrElse Failure("end of input expected", in1)
-        case ns => lastNoSuccessVar.value.getOrElse(ns)
+          LastNoSuccessHelper.value filterNot { _.next.pos < in1.pos } getOrElse Failure("end of input expected", in1)
+        case ns => LastNoSuccessHelper.value.getOrElse(ns)
       }
     }
   }
@@ -946,4 +945,31 @@ trait Parsers {
     override def ~ [U](p: => Parser[U]): Parser[~[T, U]]
       = OnceParser{ (for(a <- this; b <- commit(p)) yield new ~(a,b)).named("~") }
   }
-}
+
+  /**
+   * Utility to handle thread-local binding of
+   * last NoSuccess.
+   */
+  object LastNoSuccessHelper {
+    lazy val context = new DynamicVariable[Option[NoSuccess]](null)
+
+    def value : Option[NoSuccess] = {
+      val threadValue = context.value
+      if (threadValue == null) None else threadValue
+    }
+
+    def value_= (newValue: Option[NoSuccess]) = {
+        context.value = newValue
+    }
+
+    def isLastNoSuccessTrackingEnabled = {
+       context.value != null
+    }
+
+    def trackLastNoSuccessWithInitialValue[S](initialValue: Option[NoSuccess])(body: => S): S = {
+      context.withValue(initialValue) {
+        body
+      }
+    }
+  }
+}
\ No newline at end of file
diff --git a/src/test/scala/scala/util/parsing/combinator/SI9010MemoryLeakTest.scala b/src/test/scala/scala/util/parsing/combinator/SI9010MemoryLeakTest.scala
new file mode 100644
index 00000000..8f651bc1
--- /dev/null
+++ b/src/test/scala/scala/util/parsing/combinator/SI9010MemoryLeakTest.scala
@@ -0,0 +1,40 @@
+package scala.util.parsing.combinator
+
+import org.junit.{Test}
+
+/**
+ * Test that no references are left in LastNoSuccessHelper's DynamicVariable
+ * after parsing.
+ */
+class SI9010MemoryLeakTest {
+  
+  class TestParser extends JavaTokenParsers {
+    val token: Parser[String] = "a"
+  }
+
+  @Test
+  def shouldNotLeaveReferencesAfterFailedParse(): Unit = {
+    val testParser = new TestParser
+
+    val parseResult = testParser.parse(testParser.token, "b")
+    assert(parseResult.successful == false)
+    assert(testParser.LastNoSuccessHelper.context.value == null)
+
+    val parseAllResult = testParser.parseAll(testParser.token, "b")
+    assert(parseAllResult.successful == false)
+    assert(testParser.LastNoSuccessHelper.context.value == null)
+  }
+
+  @Test
+  def shouldNotLeaveReferencesAfterSuccesfullParse(): Unit = {
+    val testParser = new TestParser
+
+    val parseResult = testParser.parse(testParser.token, "a")
+    assert(parseResult.successful)
+    assert(testParser.LastNoSuccessHelper.context.value == null)
+
+    val parseAllResult = testParser.parseAll(testParser.token, "a")
+    assert(parseAllResult.successful)
+    assert(testParser.LastNoSuccessHelper.context.value == null)
+  }
+}