Skip to content

Quasiquotes use Symbol.exists, which isn't (currently) threadsafe in runtime reflection #409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
retronym opened this issue Jul 25, 2017 · 5 comments
Assignees
Milestone

Comments

@retronym
Copy link
Member

retronym commented Jul 25, 2017

[info] ! liftable.lift tuple: Exception raised on property evaluation.
[info] > Exception: java.lang.IllegalArgumentException: requirement failed: Tuples with 16 arity aren't supported

https://scala-ci.typesafe.com/job/scala-2.12.x-integrate-bootstrap/1040/consoleFull

@retronym retronym changed the title test failure in the scalacheck suite for quasiquotes (intermittent? test failure in the scalacheck suite for quasiquotes (intermittent)? Jul 25, 2017
@retronym
Copy link
Member Author

This failed in the nightly release build for 2.12.x last night. Right before I was planning to cut 2.12.3. Hmmmm.

@retronym retronym self-assigned this Jul 25, 2017
@retronym retronym added this to the 2.12.4 milestone Jul 25, 2017
@retronym retronym mentioned this issue Jul 25, 2017
37 tasks
@retronym
Copy link
Member Author

    val MaxTupleArity, MaxProductArity, MaxFunctionArity = 22

    lazy val ProductClass          = new VarArityClass("Product", MaxProductArity, countFrom = 1, init = Some(UnitClass))
    lazy val TupleClass            = new VarArityClass("Tuple", MaxTupleArity, countFrom = 1)

...

    class VarArityClass(name: String, maxArity: Int, countFrom: Int = 0, init: Option[ClassSymbol] = None) extends VarArityClassApi {
      private val offset = countFrom - init.size
      private def isDefinedAt(i: Int) = i < seq.length + offset && i >= offset
      val seq: IndexedSeq[ClassSymbol] = (init ++: countFrom.to(maxArity).map { i => getRequiredClass("scala." + name + i) }).toVector
      def apply(i: Int) = if (isDefinedAt(i)) seq(i - offset) else NoSymbol
   }

... 
    object SyntacticTuple extends SyntacticTupleExtractor {
      def apply(args: List[Tree]): Tree = {
        require(args.isEmpty || TupleClass(args.length).exists, s"Tuples with ${args.length} arity aren't supported")
        gen.mkTuple(args)
      }

@retronym
Copy link
Member Author

Maybe: a new or latent race condition in runtime reflection?

@retronym
Copy link
Member Author

No sign of that exception with a arity < 22 on google

@retronym
Copy link
Member Author

I can repro with 2.12.2.

⚡ scala
Welcome to Scala 2.12.2 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_112).
Type in expressions for evaluation. Or try :help.

scala> import reflect.runtime.universe._; val threads = collection.mutable.Buffer[Thread](); for (i <- 1 to 22; j <- 1 to 8) { val t = new Thread { override def run(): Unit = internal.reificationSupport.SyntacticTuple.apply(List.fill(i)(EmptyTree)) }; threads += t; t.start(); }; threads.foreach(_.join())
Exception in thread "Thread-6" java.lang.IllegalArgumentException: requirement failed: Tuples with 1 arity aren't supported
	at scala.reflect.internal.ReificationSupport$ReificationSupportImpl$SyntacticTuple$.apply(ReificationSupport.scala:430)
	at scala.reflect.internal.ReificationSupport$ReificationSupportImpl$SyntacticTuple$.apply(ReificationSupport.scala:428)
	at $line3.$read$$iw$$iw$$anon$1.run(<console>:11)
Exception in thread "Thread-11" java.lang.ClassCastException: scala.reflect.internal.Types$NoPrefix$ cannot be cast to scala.reflect.internal.Symbols$Symbol
	at scala.reflect.internal.pickling.UnPickler$Scan.readSymbolRef(UnPickler.scala:634)
	at scala.reflect.

I won't hold up the 2.12.3 release on account of this.

The fix will be one or more of:

  • GIL synchronize Symbol#exists in SynchronizedSymbol
  • simplify Symbol.exists if !isCompilerUniverse (can runtime reflection symbol loaders ever assign NoType? If not, the check collapses to != NoSymbol)
  • modify callers of Symbol#exists in ReificationSupport to use != NoSymbol.

/cc @densh @xeno-by in case they have ever run into this, or something that might have the same root cause.

@retronym retronym changed the title test failure in the scalacheck suite for quasiquotes (intermittent)? Quasiquotes use Symbol.exists, which isn't (currently) threadsafe in runtime reflection Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant