Merge pull request #6992 from adpi2/fix-bg-logging

Fix background job logging

internal/util-logging/src/main/scala/sbt/internal/util/ConsoleAppender.scala

@@ -126,11 +126,12 @@ object ConsoleAppender {
     def out: ConsoleOut
   }
   private[sbt] object Properties {
-    def from(terminal: Terminal): Properties = new Properties {
-      override def isAnsiSupported: Boolean = terminal.isAnsiSupported
-      override def isColorEnabled: Boolean = terminal.isColorEnabled
-      override def out = ConsoleOut.terminalOut(terminal)
-    }
+    def from(terminal: Terminal): Properties =
+      from(ConsoleOut.terminalOut(terminal), terminal.isAnsiSupported, terminal.isColorEnabled)
+
+    def safelyFrom(terminal: Terminal): Properties =
+      from(ConsoleOut.safeTerminalOut(terminal), terminal.isAnsiSupported, terminal.isColorEnabled)
+
     def from(o: ConsoleOut, ansi: Boolean, color: Boolean): Properties = new Properties {
       override def isAnsiSupported: Boolean = ansi
       override def isColorEnabled: Boolean = color
@@ -246,6 +247,18 @@ object ConsoleAppender {
     new ConsoleAppender(name, Properties.from(terminal), noSuppressedMessage)
   }
 
+  /**
+   * A new `ConsoleAppender` identified by `name`, and that writes to `terminal`.
+   * Printing to this Appender will not throw if the Terminal has been closed.
+   *
+   * @param name      An identifier for the `ConsoleAppender`.
+   * @param terminal  The terminal to which this appender corresponds
+   * @return A new `ConsoleAppender` that writes to `terminal`.
+   */
+  def safe(name: String, terminal: Terminal): Appender = {
+    new ConsoleAppender(name, Properties.safelyFrom(terminal), noSuppressedMessage)
+  }
+
   /**
    * A new `ConsoleAppender` identified by `name`, and that writes to `out`.
    *

internal/util-logging/src/main/scala/sbt/internal/util/ConsoleOut.scala

@@ -8,6 +8,7 @@
 package sbt.internal.util
 
 import java.io.{ BufferedWriter, PrintStream, PrintWriter }
+import java.nio.channels.ClosedChannelException
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.AtomicReference
 
@@ -90,6 +91,26 @@ object ConsoleOut {
     override def toString: String = s"TerminalOut"
   }
 
+  /** Same as terminalOut but it catches and ignores the ClosedChannelException
+   */
+  def safeTerminalOut(terminal: Terminal): ConsoleOut = {
+    val out = terminalOut(terminal)
+    new ConsoleOut {
+      override val lockObject: AnyRef = terminal
+      override def print(s: String): Unit = catchException(out.print(s))
+      override def println(s: String): Unit = catchException(out.println(s))
+      override def println(): Unit = catchException(out.println())
+      override def flush(): Unit = catchException(out.flush)
+      override def toString: String = s"SafeTerminalOut($terminal)"
+      private def catchException(f: => Unit): Unit = {
+        try f
+        catch {
+          case _: ClosedChannelException => ()
+        }
+      }
+    }
+  }
+
   private[this] val consoleOutPerTerminal = new ConcurrentHashMap[Terminal, ConsoleOut]
   def terminalOut(terminal: Terminal): ConsoleOut = consoleOutPerTerminal.get(terminal) match {
     case null =>

internal/util-logging/src/main/scala/sbt/internal/util/Terminal.scala

@@ -438,6 +438,7 @@ object Terminal {
     override def toString: String = s"ProxyTerminal(current = $t)"
   }
   private[sbt] def get: Terminal = ProxyTerminal
+  private[sbt] def current: Terminal = activeTerminal.get
 
   private[sbt] def withIn[T](in: InputStream)(f: => T): T = {
     val original = inputStream.get

internal/util-logging/src/main/scala/sbt/util/LoggerContext.scala

@@ -182,7 +182,8 @@ object LoggerContext {
       }
     }
     def close(): Unit = {
-      loggers.forEach((name, l) => l.clearAppenders())
+      closed.set(true)
+      loggers.forEach((_, l) => l.clearAppenders())
       loggers.clear()
     }
   }

main/src/main/scala/sbt/internal/DefaultBackgroundJobService.scala

@@ -74,6 +74,7 @@ private[sbt] abstract class AbstractJobHandle extends JobHandle {
 private[sbt] abstract class AbstractBackgroundJobService extends BackgroundJobService {
   private val nextId = new AtomicLong(1)
   private val pool = new BackgroundThreadPool()
+  private val context = LoggerContext(useLog4J)
 
   private[sbt] def serviceTempDirBase: File
   private[sbt] def useLog4J: Boolean
@@ -90,7 +91,6 @@ private[sbt] abstract class AbstractBackgroundJobService extends BackgroundJobSe
   // hooks for sending start/stop events
   protected def onAddJob(@deprecated("unused", "") job: JobHandle): Unit = ()
   protected def onRemoveJob(@deprecated("unused", "") job: JobHandle): Unit = ()
-  private val context = LoggerContext(useLog4J)
 
   // this mutable state could conceptually go on State except
   // that then every task that runs a background job would have
@@ -122,12 +122,9 @@ private[sbt] abstract class AbstractBackgroundJobService extends BackgroundJobSe
     def humanReadableName: String = job.humanReadableName
 
     job.onStop { () =>
-      // TODO: Fix this
-      // logger.close()
       removeJob(this)
       IO.delete(workingDirectory)
       context.clearAppenders(logger.name)
-      context.close()
     }
     addJob(this)
     override final def equals(other: Any): Boolean = other match {
@@ -144,15 +141,15 @@ private[sbt] abstract class AbstractBackgroundJobService extends BackgroundJobSe
     override val spawningTask: ScopedKey[_] = unknownTask
   }
 
-  protected def makeContext(id: Long, spawningTask: ScopedKey[_], state: State): ManagedLogger
-
   def doRunInBackground(
       spawningTask: ScopedKey[_],
       state: State,
       start: (Logger, File) => BackgroundJob
   ): JobHandle = {
     val id = nextId.getAndIncrement()
-    val logger = makeContext(id, spawningTask, state)
+    val extracted = Project.extract(state)
+    val logger =
+      LogManager.constructBackgroundLog(extracted.structure.data, state, context)(spawningTask)
     val workingDir = serviceTempDir / s"job-$id"
     IO.createDirectory(workingDir)
     val job = try {
@@ -502,10 +499,6 @@ private[sbt] class DefaultBackgroundJobService(
 ) extends AbstractBackgroundJobService {
   @deprecated("Use the constructor that specifies the background job temporary directory", "1.4.0")
   def this() = this(IO.createTemporaryDirectory, false)
-  override def makeContext(id: Long, spawningTask: ScopedKey[_], state: State): ManagedLogger = {
-    val extracted = Project.extract(state)
-    LogManager.constructBackgroundLog(extracted.structure.data, state)(spawningTask)
-  }
 }
 private[sbt] object DefaultBackgroundJobService {
 

main/src/main/scala/sbt/internal/LogManager.scala

@@ -73,15 +73,23 @@ object LogManager {
       manager(data, state, task, to, context)
     }
 
-  @nowarn
+  @deprecated("Use alternate constructBackgroundLog that provides a LoggerContext", "1.8.0")
   def constructBackgroundLog(
       data: Settings[Scope],
       state: State
+  ): ScopedKey[_] => ManagedLogger = {
+    val context = state.get(Keys.loggerContext).getOrElse(LoggerContext.globalContext)
+    constructBackgroundLog(data, state, context)
+  }
+
+  def constructBackgroundLog(
+      data: Settings[Scope],
+      state: State,
+      context: LoggerContext
   ): (ScopedKey[_]) => ManagedLogger =
     (task: ScopedKey[_]) => {
       val manager: LogManager =
         (logManager in task.scope).get(data) getOrElse defaultManager(state.globalLogging.console)
-      val context = state.get(Keys.loggerContext).getOrElse(LoggerContext.globalContext)
       manager.backgroundLog(data, state, task, context)
     }
 
@@ -141,7 +149,7 @@ object LogManager {
         task: ScopedKey[_],
         context: LoggerContext
     ): ManagedLogger = {
-      val console = screen(task, state)
+      val console = ConsoleAppender.safe("bg-" + ConsoleAppender.generateName(), ITerminal.current)
       LogManager.backgroundLog(data, state, task, console, relay(()), context)
     }
   }