From aacd249b6b824b801eba668b876dcde8132af56f Mon Sep 17 00:00:00 2001
From: Miguel Branco <miguel@raw-labs.com>
Date: Fri, 18 Aug 2023 13:34:21 +0200
Subject: [PATCH] Fix potential concurrency issue on Scala codegen. (#100)

---
 .../scala/raw/compiler/jvm/JvmCompiler.scala  | 17 ++++++++++-----
 .../compiler/scala2/Scala2JvmCompiler.scala   | 21 ++++++++++++-------
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/raw-compiler/src/main/scala/raw/compiler/jvm/JvmCompiler.scala b/raw-compiler/src/main/scala/raw/compiler/jvm/JvmCompiler.scala
index fecc44191..34412d700 100644
--- a/raw-compiler/src/main/scala/raw/compiler/jvm/JvmCompiler.scala
+++ b/raw-compiler/src/main/scala/raw/compiler/jvm/JvmCompiler.scala
@@ -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
@@ -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)
+      }
     }
   }
 
diff --git a/raw-compiler/src/main/scala/raw/compiler/scala2/Scala2JvmCompiler.scala b/raw-compiler/src/main/scala/raw/compiler/scala2/Scala2JvmCompiler.scala
index 93d3f7a4b..2776cba82 100644
--- a/raw-compiler/src/main/scala/raw/compiler/scala2/Scala2JvmCompiler.scala
+++ b/raw-compiler/src/main/scala/raw/compiler/scala2/Scala2JvmCompiler.scala
@@ -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.
@@ -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