Non-determinism in type macros

[Paul Butcher]

Is there anything in type macros which might behave non-deterministically? I've noticed a few spurious failures when working with typemacros which have mysteriously disappeared on rerunning the same code. Until now it's just been a suspicion that something's not quite right (because I've been changing other things when it's happened) but I've just caught it happening when I know that the code has been completely stable.

I did an "sbt clean" and then ran the ScalaMock tests. The first time (immediately after doing the clean) I got this failure:

[info] - cope with context bounds *** FAILED ***

[info]   Unexpected call: contextBound(foo, TypeTag[java.lang.String])

[info] 

[info] Expected:

[info] inAnyOrder {

[info]   contextBound(foo, TypeTag[java.lang.String]) once (never called - UNSATISFIED)

[info] }

[info] 

[info] Actual:

[info]   contextBound(foo, TypeTag[java.lang.String]) (Option.scala:120)

This is particularly odd, because the error says that it was expecting a call with the arguments ("foo", TypeTag[java.lang.String]), but it actually got a call with the arguments ("foo", TypeTag[String]). I guess that the most likely reason for this was that the first typeTag didn't compare as equal to the second for some reason?

The second time, I got this:

[info] - cope with context bounds *** FAILED ***

[info]   java.lang.NullPointerException:

[info]   at scala.reflect.internal.Types$TypeRef.computeHashCode(Types.scala:2332)

[info]   at scala.reflect.internal.Types$UniqueType.<init>(Types.scala:1274)

[info]   at scala.reflect.internal.Types$TypeRef.<init>(Types.scala:2315)

[info]   at scala.reflect.internal.Types$NoArgsTypeRef.<init>(Types.scala:2107)

[info]   at scala.reflect.internal.Types$ModuleTypeRef.<init>(Types.scala:2078)

[info]   at scala.reflect.internal.Types$PackageTypeRef.<init>(Types.scala:2095)

[info]   at scala.reflect.internal.Types$TypeRef$.apply(Types.scala:2516)

[info]   at scala.reflect.internal.Types$class.typeRef(Types.scala:3577)

[info]   at scala.reflect.internal.SymbolTable.typeRef(SymbolTable.scala:13)

[info]   at scala.reflect.internal.Symbols$TypeSymbol.newTypeRef(Symbols.scala:2754)

And the third time, everything ran just fine.

Since then, I've cleaned and rebuilt several times and each time all the tests have passed.

Clearly this is going to be a bitch to diagnose :-(

I'd be very grateful for pointers for where to start looking.

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Eugene Burmako]

Could you show more of the stack trace in the second case?

[Paul Butcher]

I wish I could. Unfortunately ScalaTest "helpfully" truncates the stack trace :-(

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Eugene Burmako]

>> contextBound(foo, TypeTag[java.lang.String]) once (never called - UNSATISFIED)

What does this imply? Incorrectly generated or absent mock method? Someone forgetting to call the test method?

[Eugene Burmako]

Also how many times does your type macro run per clean-compile-testall cycle? Maybe you generate multiple mocks, which somehow conflict with each other? Also, since c.freshName is currently broken as described in a nearby thread, maybe you reuse old mocks because of name clashes?

[Paul Butcher]

On 25 Dec 2012, at 21:34, Eugene Burmako <eugene....@epfl.ch> wrote:

>> contextBound(foo, TypeTag[java.lang.String]) once (never called - UNSATISFIED)
What does this imply? Incorrectly generated or absent mock method? Someone forgetting to call the test method?

The error says that the mock was expecting to receive a call of the form:

contextBound(foo, TypeTag[java.lang.String])

But this call wasn't made. Instead an unexpected call of the form:

contextBound(foo, TypeTag[java.lang.String])

was made. And no - you're not seeing things - those are identical.

As I said in my original mail, I guess that the most likely reason for this was that the first typeTag didn't compare as equal to the second for some reason?

[Paul Butcher]

On 25 Dec 2012, at 21:36, Eugene Burmako <eugene....@epfl.ch> wrote:

Also how many times does your type macro run per clean-compile-testall cycle? Maybe you generate multiple mocks, which somehow conflict with each other? Also, since c.freshName is currently broken as described in a nearby thread, maybe you reuse old mocks because of name clashes?

It runs many times (once per test in MockTest.scala - around 25 of them). I don't see how they could conflict with each other though? AIUI freshName is only broken if it's used in more than one compilation unit? Right now I've only got a single compilation unit which generates any macro type-based mocks.

[Eugene Burmako]

That might be possible. Could you make your debug printing more detailed, so that we get more information when it fails the next time?

scala> typeTag[String]
res0: reflect.runtime.universe.TypeTag[String] = TypeTag[String]

scala> showRaw(typeTag[String], printIds = true)
res1: String =
TypeTag([1])
[1] TypeRef(SingleType(ThisType(scala#2267), scala.Predef#2269), TypeName("String")#2276, List())

[Eugene Burmako]

Ah, okay. If it's a single compilation unit, then we're fine.

[Eugene Burmako]

The more I think, the more I get the impression that type macros are unrelated.

Is that possible that you run multiple tests in parallel? Runtime reflection isn't thread-safe, and the NPE might well be a result of multiple threads trying to use the singleton scala.reflect.runtime.universe.

The typetag thing might be explained by the fact that TypeTag.== behaves exactly as Type.== in a sense that equal TypeRefs can be reported as non-equal because different mirrors might have different package symbols participating in TypeRef.pre. Therefore you can't reliably compare TypeTags using ==. Just as with types, you should always go for =:=.

[Paul Butcher]

The test actually has nothing to do with TypeTags specifically - it's really checking that ScalaMock can mock methods with context bounds correctly - I just happened to choose TypeTag as that context bound. I'll switch to using something else.

Regarding thread safety - this only matters at runtime presumably, not compile time? I'm not sure to what extent sbt runs the compiler in parallel when compiling multiple files and/or projects? I get the impression that it's somewhat parallel (at least, more than one core seems to be active during a lengthy compilation). Could there be problems with macros if more than one compiler instance is active simultaneously?

As far as running tests is concerned, I believe that the ScalaTest runner in sbt runs suites in parallel, but not tests within a suite. Specs2 runs tests in parallel by default, however. And this is the only test that makes use of TypeTags so it's hard to see how parallelism could be causing the problem.

I wasn't aware that reflection wasn't threadsafe. Can I suggest that you add a prominent note to that effect in the documentation. Preferably in 10 metre high flaming capital letters :-)

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Paul Butcher]

On 25 Dec 2012, at 22:04, Paul Butcher <pa...@paulbutcher.com> wrote:

... this is the only test that makes use of TypeTags so it's hard to see how parallelism could be causing the problem.

Actually - I take that back. I have one test of this form for the old non-type-macro-based version of mocks and one for the typemacro-based version. And they're in different suites, so they might be running parallel with each other.

But if this is the problem, then I suspect we really need to reconsider the thread safety of reflection. If just adding a TypeTag context bound to a method is enough to make it non-threadsafe, then it's bound to catch people out.

[Eugene Burmako]

Where do you think flaming letters would be appropriate? It would be an overkill to put them on every doc page. Maybe just the overview page of the upcoming reflection guide at docs.scala-lang.org would be enough?

On 25 December 2012 23:04, Paul Butcher <pa...@paulbutcher.com> wrote:

[Eugene Burmako]

You raise a very important problem. Indeed, we don't provide any guarantees that calling typetag-enabled methods from different threads is going to end well.

Could you please somehow ask ScalaTest to display more lines in stack traces? Maybe it's possible to log failures? A precise stack trace would be very helpful in determining the whereabouts of the problem.

[Paul Butcher]

On 25 Dec 2012, at 22:24, Eugene Burmako <eugene....@epfl.ch> wrote:

You raise a very important problem. Indeed, we don't provide any guarantees that calling typetag-enabled methods from different threads is going to end well.

What about my question regarding parallel compiler instances? I'm concerned that this means that all macros are inherently problematic as they tend to make heavy use of reflection. The SBT documentation says that by default SBT executes tasks in parallel:

http://www.scala-sbt.org/release/docs/Detailed-Topics/Parallel-Execution

Could you please somehow ask ScalaTest to display more lines in stack traces? Maybe it's possible to log failures? A precise stack trace would be very helpful in determining the whereabouts of the problem.

I'll see if I can work out how :-)

[Eugene Burmako]

When macros call reflective APIs, they are calling into the compiler itself, without any intermediaries. That's why macros are in this regard no different from the functions declared inside the compiler.

To the best of my knowledge, the compiler has never been thread-safe, so I doubt that SBT would reuse the same compiler instance for parallel compilation. And if the instances are different, then we don't have to worry about clashes.

[Paul Butcher]

On 25 Dec 2012, at 22:21, Eugene Burmako <eugene....@epfl.ch> wrote:

Where do you think flaming letters would be appropriate? It would be an overkill to put them on every doc page. Maybe just the overview page of the upcoming reflection guide at docs.scala-lang.org would be enough?

That would certainly be a start. But TBH I suspect that this is a banana skin that's bound to catch people out no matter how prominently the message is displayed. It simply never occurred to me that reflection wouldn't be threadsafe, and I doubt that I'll be the only one to make this assumption.

[Paul Butcher]

Ah cool - fair enough.

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Roland Kuhn]

Hi Eugene,

yes, I think that a big fat warning about thread-safety should be present at each method which is not safe, precisely because reflection is not something which people suspect to be problematic. Without knowing any details, may I ask why it is not safe and what it would take to change that? This sounds like a giant “WTF” for potentially lots of users … (and it makes it virtually impossible—possibly due to lack of knowledge—to use reflection from within actors, for example)

Regards,

Dr. Roland Kuhn

Akka Tech Lead

Typesafe – The software stack for applications that scale.

twitter: @rolandkuhn

[Paul Phillips]

Unless I've been asleep for too long, reflection of the kind you mean is supposed to be threadsafe, though there are undoubtedly bugs. It is from macros at compile time, which are not going through scala.reflect.runtime where all the synchronization takes place, that the problem arises. 

[Josh Marcus]

For what it's worth, I was scratching my head about scala reflection's non-deterministic behavior on Monday -- a single test I built that uses a TypeTag would fail with an illegal cyclic reference error occasionally but not every time.  This involved actors but there was only a single test -- my personal experience would lead to me to believe there's a runtime bug that leads to non-deterministic behavior with basic reflection.  Anyway, I'll share a minimal test case when I can. 

--j

[Paul Phillips]

On Tue, Dec 25, 2012 at 6:04 PM, Josh Marcus <josh....@gmail.com> wrote:

my personal experience would lead to me to believe there's a runtime bug that leads to non-deterministic behavior with basic reflection

My experience would lead me to believe there are a bunch of them; I'm only saying that it's *supposed* to be thread-safe (which does not imply deterministic.) Or, if that is not the case, someone should clue me in on what is the purpose of

  https://github.com/scala/scala/blob/master/src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala

and the other files like it. I'd forgotten about

  https://issues.scala-lang.org/browse/SI-6240

but as you can see in the comments, thread-safe reflection is already a doomed endeavor for 2.10.0.

As to determinism, you can find plenty of tickets I'm sure: here are a couple of mine, and I haven't made any effort to find reflection bugs, and I especially haven't sought out non-determinism.

  https://issues.scala-lang.org/browse/SI-6573

  https://issues.scala-lang.org/browse/SI-6826

[Eugene Burmako]

The purpose of SynchronizedXXX is to prevent races, but because of SI-6240 it's not working for symbols. When we fix that bug, the situation will improve.

[Daniel Sobral]

On Tue, Dec 25, 2012 at 8:52 PM, Paul Butcher <pa...@paulbutcher.com> wrote:

On 25 Dec 2012, at 22:21, Eugene Burmako <eugene....@epfl.ch> wrote:

Where do you think flaming letters would be appropriate? It would be an overkill to put them on every doc page. Maybe just the overview page of the upcoming reflection guide at docs.scala-lang.org would be enough?

That would certainly be a start. But TBH I suspect that this is a banana skin that's bound to catch people out no matter how prominently the message is displayed. It simply never occurred to me that reflection wouldn't be threadsafe, and I doubt that I'll be the only one to make this assumption.

+1 here.

For the macro side of things, I wouldn't presume thread safety. I had presumed, however, that normal reflection was thread safe -- I'd put a big warning about it in my blogs if I thought otherwise. In fact, I think I'll edit them to put that warning.

 

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

--
Daniel C. Sobral

I travel to the future all the time.

[Paul Phillips]

On Wed, Dec 26, 2012 at 1:01 AM, Eugene Burmako <eugene....@epfl.ch> wrote:

The purpose of SynchronizedXXX is to prevent races, but because of SI-6240 it's not working for symbols. When we fix that bug, the situation will improve.

Okay, but is reflection supposed to be thread-safe or not? If not, why bother preventing races? What races are taking place if not between threads? What will it mean for the situation to improve? Forget the implementation, I'm trying to understand what it is that we're *trying* to do.

[Eugene Burmako]

I think, reflection ought to be thread-safe, or otherwise invocations of typetag-bearing methods will pose danger.

The races I was referring to happen when different threads try to simultaneously: 1) initialize scala.reflect.runtime.universe (e.g. by calling into Definitions.init()) or 2) invoke completer of some symbol. Maybe there are other races, which is hinted by Paul's NPE in typeRef, but I'm not aware of those.

SynchronizedXXX traits are meant to fix the 2nd type of races mentioned above. To fix them we need to come up with a solution that wouldn't impose a performance penalty as the solutions explored in https://github.com/scala/scala/pull/1380. An obvious way to fix the 1st type would be to carefully arrange locks around scala.reflect.runtime.universe. I hope there are other ways. To fix other types of races we first need to uncover them.

[√iktor Ҡlang]

Worst possible idea to introduce a single global lock for the reflection. Just sayin'

--
Viktor Klang

Director of Engineering

Typesafe - The software stack for applications that scale

Twitter: @viktorklang

[Paul Phillips]

On Wed, Dec 26, 2012 at 6:43 AM, √iktor Ҡlang <viktor...@gmail.com> wrote:

Worst possible idea to introduce a single global lock for the reflection. Just sayin'

You have something against GRLs?

GRL POWER!

[Paul Butcher]

I assume that this thread safety issue does not exist with manifests? In which case, I think that I'm right in saying that this is thread safe:

def myMethod[T: ClassManifest] = ???

whereas this is not:

def myMethod[T: ClassTag] = ???

Have I got that right? (Eugene - please correct me if I'm wrong).

Unfortunately, the former results in a deprecation warning suggesting that it should be rewritten as the latter:

Welcome to Scala version 2.10.0 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_09).

Type in expressions to have them evaluated.

Type :help for more information.

scala> def myMethod[T: ClassManifest] = ???

<console>:7: warning: type ClassManifest in object Predef is deprecated: Use scala.reflect.ClassTag instead

       def myMethod[T: ClassManifest] = ???

                     ^

myMethod: [T](implicit evidence$1: ClassManifest[T])Nothing

So we're suggesting that people should rewrite their thread safe code to become unsafe.

Oddly, ClassManifest is the only manifest that has this deprecation warning. The other types of manifest have their deprecation warnings commented out with:

// TODO undeprecated until Scala reflection becomes non-experimental

Is it just an oversight that ClassManifest is still deprecated? Or is ClassTag not subject to thread safety issues?

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

[Eugene Burmako]

ClassTag doesn't have problems with multi-threading, only TypeTags do

On 26 December 2012 21:42, Paul Butcher <pa...@swiftkey.net> wrote:

I assume that this thread safety issue does not exist with manifests? In which case, I think that I'm right in saying that this is thread safe:

def myMethod[T: ClassManifest] = ???

whereas this is not:

def myMethod[T: ClassTag] = ???

Have I got that right? (Eugene - please correct me if I'm wrong).

Unfortunately, the former results in a deprecation warning suggesting that it should be rewritten as the latter:

Welcome to Scala version 2.10.0 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_09).

Type in expressions to have them evaluated.

Type :help for more information.

scala> def myMethod[T: ClassManifest] = ???

<console>:7: warning: type ClassManifest in object Predef is deprecated: Use scala.reflect.ClassTag instead

       def myMethod[T: ClassManifest] = ???

                     ^

myMethod: [T](implicit evidence$1: ClassManifest[T])Nothing

So we're suggesting that people should rewrite their thread safe code to become unsafe.

Oddly, ClassManifest is the only manifest that has this deprecation warning. The other types of manifest have their deprecation warnings commented out with:

// TODO undeprecated until Scala reflection becomes non-experimental

Is it just an oversight that ClassManifest is still deprecated? Or is ClassTag not subject to thread safety issues?

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

On 26 Dec 2012, at 14:39, Eugene Burmako <eugene....@epfl.ch> wrote:

[Eugene Burmako]

And all manifests - both ClassManifest and Manifest are thread-safe

On 26 December 2012 21:42, Paul Butcher <pa...@swiftkey.net> wrote:

I assume that this thread safety issue does not exist with manifests? In which case, I think that I'm right in saying that this is thread safe:

def myMethod[T: ClassManifest] = ???

whereas this is not:

def myMethod[T: ClassTag] = ???

Have I got that right? (Eugene - please correct me if I'm wrong).

Unfortunately, the former results in a deprecation warning suggesting that it should be rewritten as the latter:

Welcome to Scala version 2.10.0 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_09).

Type in expressions to have them evaluated.

Type :help for more information.

scala> def myMethod[T: ClassManifest] = ???

<console>:7: warning: type ClassManifest in object Predef is deprecated: Use scala.reflect.ClassTag instead

       def myMethod[T: ClassManifest] = ???

                     ^

myMethod: [T](implicit evidence$1: ClassManifest[T])Nothing

So we're suggesting that people should rewrite their thread safe code to become unsafe.

Oddly, ClassManifest is the only manifest that has this deprecation warning. The other types of manifest have their deprecation warnings commented out with:

// TODO undeprecated until Scala reflection becomes non-experimental

Is it just an oversight that ClassManifest is still deprecated? Or is ClassTag not subject to thread safety issues?

--
paul.butcher->msgCount++

Snetterton, Castle Combe, Cadwell Park...
Who says I have a one track mind?

http://www.paulbutcher.com/
LinkedIn: http://www.linkedin.com/in/paulbutcher
MSN: pa...@paulbutcher.com
AIM: paulrabutcher
Skype: paulrabutcher

On 26 Dec 2012, at 14:39, Eugene Burmako <eugene....@epfl.ch> wrote:

[Paul Butcher]

On 26 Dec 2012, at 21:19, Eugene Burmako <eugene....@epfl.ch> wrote:

ClassTag doesn't have problems with multi-threading, only TypeTags do

Cool - I'll stop worrying :-)

I would find it extremely helpful (and I suspect I'm not alone) if we could put together a concise statement of exactly what is and is not thread safe right now. Together with, as Paul asked, a statement of intent regarding where thread safety is heading.

[Erik Engbrecht]

It's also worth noting that the lack of thread safety extends beyond the program itself being multithreaded.  I'm in the process of updating a small, old project to use Scala reflection and I encountered non-deterministic test failures because by default SBT executes tasks in parallel.  This was easy enough to disable once I realized it, but if it wasn't for this thread I would have really been scratching me head.  The amount of implicit parallelism leveraged by the Scala community, sometimes without realizing it, is pretty high.

[Eugene Burmako]

Could you please describe these failures? Stack traces would be very helpful.

[Erik Engbrecht]

Here's several test executions:

https://gist.github.com/4390558

[Eugene Burmako]

Thanks a lot! That's very helpful.

[Eugene Burmako]

Updated the docs: http://docs.scala-lang.org/overviews/reflection/thread-safety.html

On Dec 27 2012, 9:21 pm, Eugene Burmako <eugene.burm...@epfl.ch>
wrote:

> Thanks a lot! That's very helpful.
>

> >>>> LinkedIn:http://www.linkedin.****com/in/paulbutcher<http://www.linkedin.com/in/paulbutcher>