Permalink
Browse files

Fixes SI-4929, with a test to verify.

Also fixes potential issue with Parsers.phrase not being reentrant; however, I was unable to actually reproduce this issue in practice. (The order in which lastNoSuccess was being set and compared seemed to guarantee that it would never actually be a problem).
  • Loading branch information...
stephenjudkins committed Jan 22, 2012
1 parent 8051740 commit dce6b34c38a6d774961ca6f9fd50b11300ecddd6
Showing with 58 additions and 14 deletions.
  1. +15 −14 src/library/scala/util/parsing/combinator/Parsers.scala
  2. +1 −0 test/files/run/t4929.check
  3. +42 −0 test/files/run/t4929.scala
@@ -12,6 +12,7 @@ import scala.util.parsing.input._
import scala.collection.mutable.ListBuffer
import scala.annotation.tailrec
import annotation.migration
import scala.util.DynamicVariable
// TODO: better error handling (labelling like parsec's <?>)
@@ -153,13 +154,14 @@ trait Parsers {
val successful = true
}
var lastNoSuccess: NoSuccess = null
private lazy val lastNoSuccess = 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 (!(lastNoSuccess != null && next.pos < lastNoSuccess.next.pos))
lastNoSuccess = this
if (lastNoSuccess.value map { v => !(next.pos < v.next.pos) } getOrElse true)
lastNoSuccess.value = Some(this)
def map[U](f: Nothing => U) = this
def mapPartial[U](f: PartialFunction[Nothing, U], error: Nothing => String): ParseResult[U] = this
@@ -487,7 +489,7 @@ trait Parsers {
}
/** Changes the error message produced by a parser.
*
*
* This doesn't change the behavior of a parser on neither
* success nor failure, just on error. The semantics are
* slightly different than those obtained by doing `| error(msg)`,
@@ -877,16 +879,15 @@ trait Parsers {
* if `p` consumed all the input.
*/
def phrase[T](p: Parser[T]) = new Parser[T] {
lastNoSuccess = null
def apply(in: Input) = p(in) match {
case s @ Success(out, in1) =>
if (in1.atEnd)
s
else if (lastNoSuccess == null || lastNoSuccess.next.pos < in1.pos)
Failure("end of input expected", in1)
else
lastNoSuccess
case _ => lastNoSuccess
def apply(in: Input) = lastNoSuccess.withValue(None) {
p(in) match {
case s @ Success(out, in1) =>
if (in1.atEnd)
s
else
lastNoSuccess.value filterNot { _.next.pos < in1.pos } getOrElse Failure("end of input expected", in1)
case ns => lastNoSuccess.value.getOrElse(ns)
}
}
}
@@ -0,0 +1 @@
success
View
@@ -0,0 +1,42 @@
import scala.util.parsing.json._
import java.util.concurrent._
import collection.JavaConversions._
object Test extends App {
val LIMIT = 2000
val THREAD_COUNT = 20
val count = new java.util.concurrent.atomic.AtomicInteger(0)
val begin = new CountDownLatch(THREAD_COUNT)
val finish = new CountDownLatch(THREAD_COUNT)
val errors = new ConcurrentLinkedQueue[Throwable]
(1 to THREAD_COUNT) foreach { i =>
val thread = new Thread {
override def run() {
begin.await(1, TimeUnit.SECONDS)
try {
while (count.getAndIncrement() < LIMIT && errors.isEmpty) {
JSON.parseFull("""{"foo": [1,2,3,4]}""")
}
} catch {
case t: Throwable => errors.add(t)
}
finish.await(10, TimeUnit.SECONDS)
}
}
thread.setDaemon(true)
thread.start()
}
errors foreach { throw(_) }
println("success")
}

0 comments on commit dce6b34

Please sign in to comment.