Skip to content

Commit

Permalink
Fix potential concurrency issue on Scala codegen. (#100)
Browse files Browse the repository at this point in the history
  • Loading branch information
miguelbranco80 authored Aug 18, 2023
1 parent b1ae53c commit aacd249
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
17 changes: 12 additions & 5 deletions raw-compiler/src/main/scala/raw/compiler/jvm/JvmCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ abstract class JvmCompiler(implicit settings: RawSettings) extends StrictLogging
*/
def compile(id: String, code: => JvmCode): Unit = {
// Check if class already compiled and loaded.
// In that case, do not even add it to the compile queue, where it would be wanting behind other compilations,
// which could be for unrelated queries.
if (isClassCompiled(id)) {
logger.debug(s"Class $id already compiled and loaded. Reusing.")
return
Expand Down Expand Up @@ -86,11 +88,16 @@ abstract class JvmCompiler(implicit settings: RawSettings) extends StrictLogging

private class CompileTask(id: String, jvmCode: JvmCode) extends Callable[Unit] {
override def call(): Unit = {
// Compile code and obtain jar file.
doCompile(id, jvmCode)

// Now that the jar has been loaded successfully, the class is available for others to use.
addClassCompiled(id)
if (isClassCompiled(id)) {
// Another task in this queue compiled the class already just before.
logger.debug(s"Class $id already compiled and loaded. Reusing.")
} else {
// Compile code and obtain jar file.
doCompile(id, jvmCode)

// Now that the jar has been loaded successfully, the class is available for others to use.
addClassCompiled(id)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,13 @@ class Scala2JvmCompiler(
val programSourcePath = sourceDir.resolve(idPart1).resolve(idPart2).resolve(id)
val programClassPath = classDir.resolve(idPart1).resolve(idPart2).resolve(id)

// Just before emitting the code, check if it already exists.
// This could happen if another process concurrently generated this class and we don't know about it.
// Just before emitting the code, check if it already exists on DISK.
// This could happen if:
// a) in the past, a previous run left files on disk that generated this class.
// b) another process concurrently generated this class and we don't know about it (?)
// If this happens, then reload all classes once again - just as we do during startup -, which means the one
// we needed is now available. Since we are under a directory lock, this is a safe operation.
// we needed is now available. We load all classes because this class may itself depend on other classes.
// Since we are under a directory lock, this is a safe operation.
if (Files.exists(programClassPath)) {
logger.debug(s"Found code cache hit for $id. Re-loading all code caches and skipping Scala compilation.")
// Load everything again.
Expand Down Expand Up @@ -252,12 +255,14 @@ class Scala2JvmCompiler(
if (compilerReporter.hasErrors) {
val errorMessage =
if (compilerReporter.cancelled) {
logger.warn("Compilation cancelled")
"Compilation was cancelled"
logger.warn("Compilation cancelled.")
"Compilation cancelled"
} else {
val compilerErrors = compilerReporter.infos.mkString("\n")
logger.warn(s"Errors during compilation. Compiler output:\n$compilerErrors")
compilerErrors
logger.warn("Compilation failed.")
// Log each error separately as they can be very large.
// Concatenating them could failed with "UTF8 String too large".
compilerReporter.infos.foreach(info => logger.warn(info.toString()))
"Compilation failed"
}

// The reporter keeps the state between runs, so it must be explicitly reset so that errors from previous
Expand Down

0 comments on commit aacd249

Please sign in to comment.