Skip to content

Commit

Permalink
SI-6616 Check that unsafe operations are only called on the presentat…
Browse files Browse the repository at this point in the history
…ion compiler thread.

The method that checks the actual constraint is @elidable, expecting it to be used
for nightly builds but stripped-off in release builds. This way we don't lose any
performance, but 'fail-fast' in IDE nightlies.

This assumes that release builds will have at least `-Xelide-below ASSERTION`, but
this pull request does not do that.
  • Loading branch information
dragos committed Nov 6, 2012
1 parent 9999183 commit 03aa7fc
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 9 deletions.
41 changes: 32 additions & 9 deletions src/compiler/scala/tools/nsc/interactive/Global.scala
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -20,18 +20,23 @@ import scala.tools.nsc.io.Pickler._
import scala.tools.nsc.typechecker.DivergentImplicit import scala.tools.nsc.typechecker.DivergentImplicit
import scala.annotation.tailrec import scala.annotation.tailrec
import symtab.Flags.{ACCESSOR, PARAMACCESSOR} import symtab.Flags.{ACCESSOR, PARAMACCESSOR}
import scala.annotation.elidable
import scala.language.implicitConversions import scala.language.implicitConversions


/** The main class of the presentation compiler in an interactive environment such as an IDE /** The main class of the presentation compiler in an interactive environment such as an IDE
*/ */
class Global(settings: Settings, _reporter: Reporter, projectName: String = "") class Global(settings: Settings, _reporter: Reporter, projectName: String = "") extends {
extends scala.tools.nsc.Global(settings, _reporter) /* Is the compiler initializing? Early def, so that the field is true during the
with CompilerControl * execution of the super constructor.
with RangePositions */
with ContextTrees private var initializing = true
with RichCompilationUnits } with scala.tools.nsc.Global(settings, _reporter)
with ScratchPadMaker with CompilerControl
with Picklers { with RangePositions
with ContextTrees
with RichCompilationUnits
with ScratchPadMaker
with Picklers {


import definitions._ import definitions._


Expand All @@ -51,6 +56,7 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
import log.logreplay import log.logreplay
debugLog("logger: " + log.getClass + " writing to " + (new java.io.File(logName)).getAbsolutePath) debugLog("logger: " + log.getClass + " writing to " + (new java.io.File(logName)).getAbsolutePath)
debugLog("classpath: "+classPath) debugLog("classpath: "+classPath)
Console.err.println("\n ======= CHECK THREAD ACCESS compiler build ========\n")


private var curTime = System.nanoTime private var curTime = System.nanoTime
private def timeStep = { private def timeStep = {
Expand Down Expand Up @@ -433,7 +439,18 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
private var threadId = 0 private var threadId = 0


/** The current presentation compiler runner */ /** The current presentation compiler runner */
@volatile private[interactive] var compileRunner = newRunnerThread() @volatile private[interactive] var compileRunner: Thread = newRunnerThread()

/** Check that the currenyly executing thread is the presentation compiler thread.
*
* Compiler initialization may happen on a different thread (signalled by globalPhase being NoPhase)
*/
@elidable(elidable.WARNING)
override def assertCorrectThread() {
assert(initializing || (Thread.currentThread() eq compileRunner),
"Race condition detected: You are running a presentation compiler method outside the PC thread.[phase: %s]".format(globalPhase) +
" Please file a ticket with the current stack trace at https://www.assembla.com/spaces/scala-ide/support/tickets")
}


/** Create a new presentation compiler runner. /** Create a new presentation compiler runner.
*/ */
Expand Down Expand Up @@ -1110,6 +1127,12 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
alt alt
} }
} }

/** The compiler has been initialized. Constructors are evaluated in textual order,
* so this is set to true only after all super constructors and the primary constructor
* have been executed.
*/
initializing = false
} }


object CancelException extends Exception object CancelException extends Exception
Expand Down
6 changes: 6 additions & 0 deletions src/reflect/scala/reflect/internal/SymbolTable.scala
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package scala.reflect package scala.reflect
package internal package internal


import scala.annotation.elidable
import scala.collection.{ mutable, immutable } import scala.collection.{ mutable, immutable }
import util._ import util._


Expand Down Expand Up @@ -107,6 +108,11 @@ abstract class SymbolTable extends macros.Universe
val global: SymbolTable.this.type = SymbolTable.this val global: SymbolTable.this.type = SymbolTable.this
} with util.TraceSymbolActivity } with util.TraceSymbolActivity


/** Check that the executing thread is the compiler thread. No-op here,
* overridden in interactive.Global. */
@elidable(elidable.WARNING)
def assertCorrectThread() {}

/** Are we compiling for Java SE? */ /** Are we compiling for Java SE? */
// def forJVM: Boolean // def forJVM: Boolean


Expand Down
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/internal/Symbols.scala
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
} }
val current = phase val current = phase
try { try {
assertCorrectThread()
phase = phaseOf(infos.validFrom) phase = phaseOf(infos.validFrom)
tp.complete(this) tp.complete(this)
} finally { } finally {
Expand Down Expand Up @@ -1283,6 +1284,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
infos = infos.prev infos = infos.prev


if (validTo < curPeriod) { if (validTo < curPeriod) {
assertCorrectThread()
// adapt any infos that come from previous runs // adapt any infos that come from previous runs
val current = phase val current = phase
try { try {
Expand Down
1 change: 1 addition & 0 deletions src/reflect/scala/reflect/internal/Types.scala
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ trait Types extends api.Types { self: SymbolTable =>
/** Undo all changes to constraints to type variables upto `limit`. */ /** Undo all changes to constraints to type variables upto `limit`. */
//OPT this method is public so we can do `manual inlining` //OPT this method is public so we can do `manual inlining`
def undoTo(limit: UndoPairs) { def undoTo(limit: UndoPairs) {
assertCorrectThread()
while ((log ne limit) && log.nonEmpty) { while ((log ne limit) && log.nonEmpty) {
val (tv, constr) = log.head val (tv, constr) = log.head
tv.constr = constr tv.constr = constr
Expand Down

0 comments on commit 03aa7fc

Please sign in to comment.