Merge pull request #69 from liskin/lastnosuccess-memleak

Fix lastNoSuccessVar memory leak

Author: gourlaysama

src/main/scala/scala/util/parsing/combinator/Parsers.scala

@@ -156,14 +156,19 @@ trait Parsers {
     val successful = true
   }
 
-  private lazy val lastNoSuccessVar = new DynamicVariable[Option[NoSuccess]](None)
+  /* two layers of Option:
+   *  outer Option is None if lastNoSuccess tracking is disabled (outside of
+   *               phrase) and Some if tracking is enabled
+   *  inner Option is None if NoSuccess hasn't been seen yet, Some otherwise
+   * this is necessary to avoid leaking NoSuccesses in thread locals */
+  private lazy val lastNoSuccessVar = new DynamicVariable[Option[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 (lastNoSuccessVar.value exists (_ forall (v => !(next.pos < v.next.pos))))
+      lastNoSuccessVar.value = Some(Some(this))
 
     def map[U](f: Nothing => U) = this
     def mapPartial[U](f: PartialFunction[Nothing, U], error: Nothing => String): ParseResult[U] = this
@@ -878,14 +883,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) = lastNoSuccessVar.withValue(Some(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)
+        case s @ Success(out, in1) =>
+          if (in1.atEnd)
+            s
+          else
+            lastNoSuccessVar.value flatMap (_ filterNot { _.next.pos < in1.pos }) getOrElse Failure("end of input expected", in1)
+        case ns => lastNoSuccessVar.value.flatten.getOrElse(ns)
       }
     }
   }

src/test/scala/scala/util/parsing/combinator/t9010.scala

@@ -0,0 +1,51 @@
+import scala.util.parsing.combinator._
+import scala.util.DynamicVariable
+
+import org.junit.Test
+
+class t9010 {
+  @Test
+  def test: Unit = {
+    val p = new grammar
+    val lastNoSuccessVar = getLastNoSuccessVar(p)
+    import p._
+
+    val res1 = parse(x, "x")
+    assert(res1.successful)
+    assert(lastNoSuccessVar.value == None)
+
+    val res2 = parse(x, "y")
+    assert(!res2.successful)
+    assert(lastNoSuccessVar.value == None)
+
+    val res3 = parseAll(x, "x")
+    assert(res3.successful)
+    assert(lastNoSuccessVar.value == None)
+
+    val res4 = parseAll(x, "y")
+    assert(!res4.successful)
+    assert(lastNoSuccessVar.value == None)
+  }
+
+  private def getLastNoSuccessVar(p: Parsers): DynamicVariable[Option[_]] = {
+    // use java reflection instead of scala (see below) because of
+    // https://issues.scala-lang.org/browse/SI-9306
+    val fn = "scala$util$parsing$combinator$Parsers$$lastNoSuccessVar"
+    val f = p.getClass.getDeclaredMethod(fn)
+    f.setAccessible(true)
+    f.invoke(p).asInstanceOf[DynamicVariable[Option[_]]]
+
+    /*
+    val ru = scala.reflect.runtime.universe
+    val mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val lastNoSuccessVarField =
+      ru.typeOf[Parsers].decl(ru.TermName("lastNoSuccessVar")).asTerm.accessed.asTerm
+    mirror.reflect(p).reflectField(lastNoSuccessVarField).get.
+      asInstanceOf[DynamicVariable[Option[_]]]
+    */
+  }
+
+  private final class grammar extends RegexParsers {
+    val x: Parser[String] = "x"
+  }
+}