From 6bcda6684add413956bc0d28bfb6b94a64f7feeb Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Thu, 4 Aug 2022 10:54:43 +0200 Subject: [PATCH] Use BackgroundJobService context instead of MainLoop context A new context is created and closed for each state of the MainLoop. But the context of the backgroundJob must stay alive. So we use a context that is owned by the BackgroundJobService. It creates a new logger for each background job and cleans it when the job stops. --- .../src/main/scala/sbt/util/LoggerContext.scala | 3 ++- .../internal/DefaultBackgroundJobService.scala | 15 ++++----------- main/src/main/scala/sbt/internal/LogManager.scala | 12 ++++++++++-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/internal/util-logging/src/main/scala/sbt/util/LoggerContext.scala b/internal/util-logging/src/main/scala/sbt/util/LoggerContext.scala index ee1069061..0435793dd 100644 --- a/internal/util-logging/src/main/scala/sbt/util/LoggerContext.scala +++ b/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() } } diff --git a/main/src/main/scala/sbt/internal/DefaultBackgroundJobService.scala b/main/src/main/scala/sbt/internal/DefaultBackgroundJobService.scala index b9effa8a9..79d431ebd 100644 --- a/main/src/main/scala/sbt/internal/DefaultBackgroundJobService.scala +++ b/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 { diff --git a/main/src/main/scala/sbt/internal/LogManager.scala b/main/src/main/scala/sbt/internal/LogManager.scala index e28fd91ee..7029cde2e 100644 --- a/main/src/main/scala/sbt/internal/LogManager.scala +++ b/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) }