bugfix: Only delete directories if nothing is running

backend/src/main/scala/bloop/Compiler.scala

@@ -435,7 +435,6 @@ object Compiler {
               // new compilation artifacts coming from scalac, which we will not
               // have in this case and it's going to remain empty.
               val firstTask = Task { BloopPaths.delete(AbsolutePath(newClassesDir)) }
-
               // Then we copy previous best effort artifacts to a clientDir from the
               // cached compilation result.
               // This is useful if e.g. the client restarted after the last compilation

frontend/src/main/scala/bloop/engine/caches/LastSuccessfulResult.scala

@@ -9,7 +9,6 @@ import bloop.data.Project
 import bloop.io.AbsolutePath
 import bloop.task.Task
 
-import monix.execution.atomic.AtomicInt
 import xsbti.compile.CompileAnalysis
 import xsbti.compile.MiniSetup
 import xsbti.compile.PreviousResult
@@ -18,7 +17,6 @@ case class LastSuccessfulResult(
     noClassPathAndSources: Boolean,
     previous: PreviousResult,
     classesDir: AbsolutePath,
-    counterForClassesDir: AtomicInt,
     populatingProducts: Task[Unit]
 ) {
   def isEmpty: Boolean = {
@@ -43,7 +41,6 @@ object LastSuccessfulResult {
       noClassPathAndSources = true,
       EmptyPreviousResult,
       emptyClassesDir,
-      AtomicInt(0),
       Task.now(())
     )
   }
@@ -57,7 +54,6 @@ object LastSuccessfulResult {
       noClassPathAndSources = inputs.sources.size == 0 && inputs.classpath.size == 0,
       products.resultForFutureCompilationRuns,
       AbsolutePath(products.newClassesDir),
-      AtomicInt(0),
       backgroundIO
     )
   }

frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala

@@ -1,8 +1,13 @@
 package bloop.engine.tasks
 
+import java.nio.file.Path
 import java.util.Optional
+import java.util.concurrent.ConcurrentLinkedQueue
+import java.util.concurrent.atomic.AtomicInteger
+
 import scala.collection.mutable
 import scala.concurrent.Promise
+
 import bloop.CompileBackgroundTasks
 import bloop.CompileInputs
 import bloop.CompileOutPaths
@@ -35,17 +40,18 @@ import bloop.reporter.ReporterInputs
 import bloop.task.Task
 import bloop.tracing.BraveTracer
 import bloop.util.BestEffortUtils.BestEffortProducts
+
 import monix.execution.CancelableFuture
 import monix.reactive.MulticastStrategy
 import monix.reactive.Observable
 import xsbti.compile.CompileAnalysis
 import xsbti.compile.MiniSetup
 import xsbti.compile.PreviousResult
 
-import java.nio.file.Path
-
 object CompileTask {
   private implicit val logContext: DebugFilter = DebugFilter.Compilation
+  private val compilationCounter = new AtomicInteger(0)
+  private val cleanUpTasks = new ConcurrentLinkedQueue[AbsolutePath]()
 
   def compile[UseSiteLogger <: Logger](
       state: State,
@@ -57,6 +63,8 @@ object CompileTask {
       store: CompileClientStore,
       rawLogger: UseSiteLogger
   ): Task[State] = Task.defer {
+    compilationCounter.incrementAndGet()
+
     import bloop.data.ClientInfo
     import bloop.internal.build.BuildInfo
     val originUri = state.build.origin
@@ -335,16 +343,15 @@ object CompileTask {
           o
         )
       }
-
       val client = state.client
       CompileGraph
         .traverse(dag, client, store, bestEffortAllowed, setup, compile, tracer)
         .flatMap { pdag =>
           val partialResults = Dag.dfs(pdag, mode = Dag.PreOrder)
           val finalResults = partialResults.map(r => PartialCompileResult.toFinalResult(r))
           Task.gatherUnordered(finalResults).map(_.flatten).flatMap { results =>
-            val cleanUpTasksToRunInBackground =
-              markUnusedClassesDirAndCollectCleanUpTasks(results, state, tracer, rawLogger)
+            val makrUnusedDirectoriesTask =
+              markUnusedClassesDir(results, state, tracer, rawLogger)
 
             val failures = results.flatMap {
               case FinalNormalCompileResult(p, results) =>
@@ -389,7 +396,14 @@ object CompileTask {
             }
 
             // Schedule to run clean-up tasks in the background
-            runIOTasksInParallel(cleanUpTasksToRunInBackground)
+            runIOTasksInParallel(
+              makrUnusedDirectoriesTask,
+              runOnFinish = cleanupIfNoCompilationRunningTask(
+                compilationCounter.decrementAndGet(),
+                tracer,
+                rawLogger
+              )
+            )
 
             val runningTasksRequiredForCorrectness = Task.sequence {
               results.flatMap {
@@ -409,6 +423,7 @@ object CompileTask {
           }
         }
     }
+
   }
 
   case class ConfiguredCompilation(scalacOptions: List[String])
@@ -445,14 +460,14 @@ object CompileTask {
     }
   }
 
-  private def markUnusedClassesDirAndCollectCleanUpTasks(
+  private def markUnusedClassesDir(
       results: List[FinalCompileResult],
       previousState: State,
       tracer: BraveTracer,
       logger: Logger
   ): List[Task[Unit]] =
     tracer.trace("markUnusedClassesDirAndCollectCleanUpTasks") { _ =>
-      val cleanUpTasksToSpawnInBackground = mutable.ListBuffer[Task[Unit]]()
+      val markDeleteTasks = mutable.ListBuffer[Task[Unit]]()
       results.foreach { finalResult =>
         val resultBundle = finalResult.result
         val newSuccessful = resultBundle.successful
@@ -464,36 +479,40 @@ object CompileTask {
             case _ => None
           }
         val populateNewProductsTask = newSuccessful.map(_.populatingProducts).getOrElse(Task.unit)
-        val cleanUpPreviousLastSuccessful = resultBundle.previous match {
+        val markDeletePreviousLastSuccessful = resultBundle.previous match {
           case None => populateNewProductsTask
           case Some(previousSuccessful) =>
             for {
               _ <- previousSuccessful.populatingProducts
               _ <- populateNewProductsTask
-              _ <- cleanUpPreviousResult(
+              toDelete = previousDirectoriesToCleanup(
                 previousSuccessful,
                 previousResult,
                 compilerResult,
                 tracer,
                 logger
               )
+              _ = toDelete.foreach(cleanUpTasks.add)
             } yield ()
         }
 
-        cleanUpTasksToSpawnInBackground.+=(cleanUpPreviousLastSuccessful)
+        markDeleteTasks.+=(markDeletePreviousLastSuccessful)
       }
 
-      cleanUpTasksToSpawnInBackground.toList
+      markDeleteTasks.toList
     }
 
   def runIOTasksInParallel[T](
       tasks: Traversable[Task[T]],
-      parallelUnits: Int = Runtime.getRuntime().availableProcessors()
+      parallelUnits: Int = Runtime.getRuntime().availableProcessors(),
+      runOnFinish: Task[Unit]
   ): Unit = {
     val aggregatedTask = Task.sequence(
       tasks.toList.grouped(parallelUnits).map(group => Task.gatherUnordered(group)).toList
     )
-    aggregatedTask.map(_ => ()).runAsync(ExecutionContext.ioScheduler)
+    aggregatedTask
+      .flatMap(_ => runOnFinish)
+      .runAsync(ExecutionContext.ioScheduler)
     ()
   }
 
@@ -514,15 +533,14 @@ object CompileTask {
    * superseeded by the new classes directory generated during a successful
    * compile.
    */
-  private def cleanUpPreviousResult(
+  private def previousDirectoriesToCleanup(
       previousSuccessful: LastSuccessfulResult,
       previousResult: Option[Compiler.Result],
       compilerResult: Compiler.Result,
       tracer: BraveTracer,
       logger: Logger
-  ): Task[Unit] = tracer.trace("clean up previous result") { implicit tracer =>
+  ): List[AbsolutePath] = tracer.trace("clean up previous result") { _ =>
     val previousClassesDir = previousSuccessful.classesDir
-    val currentlyUsedCounter = previousSuccessful.counterForClassesDir.decrementAndGet(1)
 
     val previousReadOnlyToDelete = compilerResult match {
       case Success(_, products, _, _, isNoOp, _, _) =>
@@ -532,11 +550,6 @@ object CompileTask {
         } else if (CompileOutPaths.hasEmptyClassesDir(previousClassesDir)) {
           logger.debug(s"Skipping delete of empty classes dir ${previousClassesDir}")
           None
-        } else if (currentlyUsedCounter != 0) {
-          logger.debug(
-            s"Skipping delete of $previousClassesDir, counter is $currentlyUsedCounter"
-          )
-          None
         } else {
           val newClassesDir = products.newClassesDir
           logger.debug(s"Scheduling to delete ${previousClassesDir} superseded by $newClassesDir")
@@ -566,28 +579,36 @@ object CompileTask {
       case _ => None
     }
 
-    def deleteOrphanDir(orphanDir: Option[AbsolutePath])(implicit
-        tracer: BraveTracer
-    ): Task[Unit] = {
-      tracer.trace("delete orphan dir") { _ =>
-        orphanDir match {
-          case None => Task.unit
-          case Some(classesDir) =>
-            Task.eval {
-              logger.debug(s"Deleting contents of orphan dir $classesDir")
-              BloopPaths.delete(classesDir)
-            }.asyncBoundary
+    List(
+      previousReadOnlyToDelete,
+      previousBestEffortToDelete
+    ).flatten
+
+  }
+
+  def cleanupIfNoCompilationRunningTask(
+      counter: Int,
+      tracer: BraveTracer,
+      logger: Logger
+  ): Task[Unit] = {
+    Task {
+      if (counter == 0) {
+        tracer.trace("delete orphan directories") { _ =>
+          val allDeleteDirectories = new mutable.ListBuffer[AbsolutePath]()
+          while (cleanUpTasks.size() > 0) allDeleteDirectories += cleanUpTasks.poll()
+          val allDeleteTasks = allDeleteDirectories
+            .result()
+            .map(classesDir => {
+              Task.eval {
+                logger.debug(s"Deleting contents of orphan dir $classesDir : ${classesDir.exists}")
+                BloopPaths.delete(classesDir)
+              }.asyncBoundary
+            })
+          Task.gatherUnordered(allDeleteTasks).map(_ => ())
         }
+      } else {
+        Task.unit
       }
-    }
-
-    Task
-      .gatherUnordered(
-        List(
-          deleteOrphanDir(previousReadOnlyToDelete),
-          deleteOrphanDir(previousBestEffortToDelete)
-        )
-      )
-      .map(_ => ())
+    }.flatten
   }
 }