Skip to content

Commit ee32622

Browse files
lihaoyilihaoyi
committed
.
1 parent 8fa305b commit ee32622

12 files changed

Lines changed: 294 additions & 181 deletions

File tree

‎core/api/daemon/src/mill/api/daemon/internal/bsp/BspBootstrapBridge.scala‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,22 @@ trait BspBootstrapBridge {
3535
}
3636

3737
object BspBootstrapBridge {
38+
final case class BootstrapState(
39+
evaluators: java.util.List[EvaluatorApi],
40+
watched: java.util.List[Watchable],
41+
errorOpt: Option[String]
42+
)
3843

3944
/**
4045
* Java-functional-interface form of the body: receives evaluators and the
41-
* watched inputs accumulated during this bootstrap. Defined as a SAM trait
42-
* (not Scala FunctionN) so the BSP-worker classloader can implement it
46+
* watched inputs accumulated during this bootstrap, together with any
47+
* bootstrap failure that occurred while preparing them. Defined as a SAM
48+
* trait (not Scala FunctionN) so the BSP-worker classloader can implement it
4349
* without depending on Scala's standard-library function classes resolved
4450
* against the daemon's classloader.
4551
*/
4652
@FunctionalInterface
4753
trait Body[T] {
48-
def apply(evaluators: java.util.List[EvaluatorApi], watched: java.util.List[Watchable]): T
54+
def apply(state: BootstrapState): T
4955
}
5056
}

‎core/internal/src/mill/internal/LauncherOutFilesImpl.scala‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ private[mill] final class LauncherOutFilesImpl(
6262

6363
os.makeDir.all(runDir)
6464
writeLauncherRunFile()
65-
cleanup(out, daemonDir)
65+
cleanup(out, daemonDir, launcherLocks)
6666

6767
override def publishLiveArtifacts(): Unit =
6868
if (!closed.get()) publishLatest(publishedArtifacts.head, launcherLocks)
@@ -74,7 +74,7 @@ private[mill] final class LauncherOutFilesImpl(
7474
if (closed.compareAndSet(false, true)) {
7575
try mill.api.BuildCtx.withFilesystemCheckerDisabled(os.remove(launcherRunFile))
7676
catch { case _: Throwable => }
77-
cleanup(out, daemonDir)
77+
cleanup(out, daemonDir, launcherLocks)
7878
}
7979

8080
private def writeLauncherRunFile(): Unit = {
@@ -140,7 +140,11 @@ private[mill] object LauncherOutFilesImpl {
140140
*
141141
* Then sweep any `out/mill-*` symlink whose target no longer resolves.
142142
*/
143-
private def cleanup(out: os.Path, daemonDir: os.Path): Unit = {
143+
private def cleanup(
144+
out: os.Path,
145+
daemonDir: os.Path,
146+
launcherLocks: LauncherSessionState
147+
): Unit = {
144148
try {
145149
val runRootDir = out / LauncherSessionState.runRootDirName
146150
if (os.exists(runRootDir)) {
@@ -160,25 +164,28 @@ private[mill] object LauncherOutFilesImpl {
160164
)
161165
}
162166

163-
// Sweep dangling well-known symlinks (point at a target that no longer
164-
// resolves, e.g. because the target's run dir was just GC'd above). The
165-
// isLink + exists + remove chain is technically racy against another
166-
// launcher that's mid-publish for the same artifact: between our checks
167-
// and our delete, that launcher could atomic-move a fresh symlink into
168-
// place and we'd delete a freshly-valid link. We accept the rare case
169-
// — the next publish or cleanup will restore it — rather than serialize
170-
// publishes against cleanups, which would defeat the concurrent-
171-
// launcher design.
172167
wellKnownArtifactLinks.foreach { rel =>
173168
val link = out / rel
174-
if (os.isLink(link) && !os.exists(link, followLinks = true)) {
175-
try os.remove(link)
176-
catch { case _: Throwable => }
169+
withArtifactLock(link, launcherLocks) {
170+
if (os.isLink(link) && !os.exists(link, followLinks = true)) {
171+
try os.remove(link)
172+
catch { case _: Throwable => }
173+
}
177174
}
178175
}
179176
} catch { case _: Throwable => }
180177
}
181178

179+
private def withArtifactLock[T](
180+
link: os.Path,
181+
launcherLocks: LauncherSessionState
182+
)(body: => T): T = {
183+
val normalizedAbsolutePath = link.toNIO.toAbsolutePath.normalize.toString
184+
launcherLocks.artifactLockFor(normalizedAbsolutePath).synchronized {
185+
body
186+
}
187+
}
188+
182189
/**
183190
* Atomic-move a symlink at `link` to point at `target`. Uses a sibling tmp
184191
* file and `ATOMIC_MOVE` so readers never observe an in-progress state.
@@ -236,15 +243,17 @@ private[mill] object LauncherOutFilesImpl {
236243
artifact: PublishedArtifact,
237244
launcherLocks: LauncherSessionState
238245
): Unit = {
239-
if (os.exists(artifact.target))
240-
try updateSymlink(artifact.link, artifact.target, launcherLocks)
241-
catch {
242-
case e: Throwable =>
243-
mill.api.Debug(
244-
s"Failed to publish ${artifact.link.last} as a symlink: ${e.getClass.getSimpleName}: ${e.getMessage}"
245-
)
246-
if (artifact.copyFallback) replaceWithCopy(artifact.link, artifact.target, launcherLocks)
247-
}
246+
withArtifactLock(artifact.link, launcherLocks) {
247+
if (os.exists(artifact.target))
248+
try updateSymlink(artifact.link, artifact.target, launcherLocks)
249+
catch {
250+
case e: Throwable =>
251+
mill.api.Debug(
252+
s"Failed to publish ${artifact.link.last} as a symlink: ${e.getClass.getSimpleName}: ${e.getMessage}"
253+
)
254+
if (artifact.copyFallback) replaceWithCopy(artifact.link, artifact.target, launcherLocks)
255+
}
256+
}
248257
}
249258

250259
/**

‎core/internal/src/mill/internal/LauncherSessionState.scala‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import java.util.concurrent.atomic.AtomicLong
1919
private[mill] final class LauncherSessionState {
2020
private val metaBuildLocks = new ConcurrentHashMap[Int, WriterPreferringRwLock]()
2121
private val taskLocks = new ConcurrentHashMap[String, WriterPreferringRwLock]()
22+
private val artifactLocks = new ConcurrentHashMap[String, AnyRef]()
2223
private val bootstrapLockInstance = new WriterPreferringRwLock("bootstrap-module")
2324
private val runIdCounter = new AtomicLong(0L)
2425
private val tmpNameCounter = new AtomicLong(0L)
@@ -29,6 +30,9 @@ private[mill] final class LauncherSessionState {
2930
def taskLockFor(normalizedAbsolutePath: String): WriterPreferringRwLock =
3031
taskLocks.computeIfAbsent(normalizedAbsolutePath, p => new WriterPreferringRwLock(p))
3132

33+
def artifactLockFor(normalizedAbsolutePath: String): AnyRef =
34+
artifactLocks.computeIfAbsent(normalizedAbsolutePath, _ => new Object)
35+
3236
def bootstrapLock: WriterPreferringRwLock = bootstrapLockInstance
3337

3438
def nextRunId(): String =

‎core/internal/test/src/mill/internal/LauncherSessionStateTests.scala‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ import java.util.concurrent.{CountDownLatch, TimeUnit}
1010

1111
object LauncherSessionStateTests extends TestSuite {
1212
val tests: Tests = Tests {
13+
test("artifact-locks-are-shared-per-path") {
14+
val state = new LauncherSessionState
15+
val lockA0 = state.artifactLockFor("/tmp/mill/out/mill-console-tail")
16+
val lockA1 = state.artifactLockFor("/tmp/mill/out/mill-console-tail")
17+
val lockB = state.artifactLockFor("/tmp/mill/out/mill-profile.json")
18+
19+
assert(lockA0 eq lockA1)
20+
assert(!(lockA0 eq lockB))
21+
}
22+
1323
test("meta-build-locks-are-independent-per-depth") {
1424
val state = new LauncherSessionState
1525
val blocker = new LauncherLockingImpl(

‎integration/dedicated/bsp-server/src/BspServerTests.scala‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -590,17 +590,20 @@ object BspServerTests extends UtestIntegrationTestSuite {
590590
val delayedResult = delayedCompileFuture.get(30, TimeUnit.SECONDS)
591591
assert(delayedResult.getStatusCode == b.StatusCode.OK)
592592

593+
def resolvesIntoRunDir(rel: os.RelPath): Boolean = {
594+
val link = workspacePath / "out" / rel
595+
val runRoot = workspacePath / "out" / "mill-run"
596+
os.isLink(link) &&
597+
os.exists(link, followLinks = true) &&
598+
os.Path(link.toNIO.toRealPath()).toString.startsWith(runRoot.toString + "/")
599+
}
600+
593601
assertEventually {
594-
os.isLink(workspacePath / "out" / mill.constants.DaemonFiles.millConsoleTail) &&
595-
os.exists(workspacePath / "out" / mill.constants.DaemonFiles.millConsoleTail, followLinks = true) &&
596-
os.isLink(workspacePath / "out" / OutFiles.millProfile) &&
597-
os.exists(workspacePath / "out" / OutFiles.millProfile, followLinks = true) &&
598-
os.isLink(workspacePath / "out" / OutFiles.millChromeProfile) &&
599-
os.exists(workspacePath / "out" / OutFiles.millChromeProfile, followLinks = true) &&
600-
os.isLink(workspacePath / "out" / OutFiles.millDependencyTree) &&
601-
os.exists(workspacePath / "out" / OutFiles.millDependencyTree, followLinks = true) &&
602-
os.isLink(workspacePath / "out" / OutFiles.millInvalidationTree) &&
603-
os.exists(workspacePath / "out" / OutFiles.millInvalidationTree, followLinks = true)
602+
resolvesIntoRunDir(os.RelPath(mill.constants.DaemonFiles.millConsoleTail)) &&
603+
resolvesIntoRunDir(os.RelPath(OutFiles.millProfile)) &&
604+
resolvesIntoRunDir(os.RelPath(OutFiles.millChromeProfile)) &&
605+
resolvesIntoRunDir(os.RelPath(OutFiles.millDependencyTree)) &&
606+
resolvesIntoRunDir(os.RelPath(OutFiles.millInvalidationTree))
604607
}
605608
}
606609
}

‎runner/bsp/worker/src/mill/bsp/worker/BspEvaluators.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ class BspEvaluators(
2121
workspaceDir: os.Path,
2222
val evaluators: Seq[EvaluatorApi],
2323
debug: (() => String) => Unit,
24-
val watched: Seq[Watchable]
24+
val watched: Seq[Watchable],
25+
val bootstrapErrorOpt: Option[String] = None
2526
) {
2627
private lazy val disabledBspModules: Set[ModuleApi] =
2728
Utils.computeDisabledBspModules(evaluators)

‎runner/bsp/worker/src/mill/bsp/worker/MillBuildServer.scala‎

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -186,32 +186,33 @@ private abstract class MillBuildServer(
186186
val watchedSeq = bootstrapBridge.runBootstrap(
187187
"BSP:watch",
188188
new BspBootstrapBridge.Body[Seq[Watchable]] {
189-
override def apply(
190-
evaluators: java.util.List[EvaluatorApi],
191-
watched: java.util.List[Watchable]
192-
): Seq[Watchable] = {
193-
val bspEvaluators = new BspEvaluators(
194-
topLevelProjectRoot,
195-
evaluators.asScala.toSeq,
196-
s => baseLogger.debug(s()),
197-
watched.asScala.toSeq
198-
)
199-
val current = bspEvaluators.targetSnapshots
200-
// Re-read `client` each iteration: `onConnectWithClient`
201-
// may run after `startWatcherThread`, so capturing once at
202-
// thread start would silently leave `client0` null forever
203-
// and never deliver `buildTargetDidChange`.
204-
val currentClient = client
205-
if (seenAnyBootstrap && currentClient != null)
206-
ChangeNotifier.notifyChanges(
207-
currentClient,
208-
prevTargetSnapshots,
209-
current
189+
override def apply(state: BspBootstrapBridge.BootstrapState): Seq[Watchable] =
190+
if (state.errorOpt.isDefined && state.evaluators.isEmpty) {
191+
state.watched.asScala.toSeq
192+
} else {
193+
val bspEvaluators = new BspEvaluators(
194+
topLevelProjectRoot,
195+
state.evaluators.asScala.toSeq,
196+
s => baseLogger.debug(s()),
197+
state.watched.asScala.toSeq,
198+
state.errorOpt
210199
)
211-
prevTargetSnapshots = current
212-
seenAnyBootstrap = true
213-
watched.asScala.toSeq
214-
}
200+
val current = bspEvaluators.targetSnapshots
201+
// Re-read `client` each iteration: `onConnectWithClient`
202+
// may run after `startWatcherThread`, so capturing once at
203+
// thread start would silently leave `client0` null forever
204+
// and never deliver `buildTargetDidChange`.
205+
val currentClient = client
206+
if (seenAnyBootstrap && currentClient != null)
207+
ChangeNotifier.notifyChanges(
208+
currentClient,
209+
prevTargetSnapshots,
210+
current
211+
)
212+
prevTargetSnapshots = current
213+
seenAnyBootstrap = true
214+
state.watched.asScala.toSeq
215+
}
215216
}
216217
)
217218

@@ -369,7 +370,7 @@ private abstract class MillBuildServer(
369370
requestName: String,
370371
logger: Logger,
371372
future: CompletableFuture[?],
372-
run: (Seq[EvaluatorApi], Seq[Watchable]) => Unit,
373+
run: BspBootstrapBridge.BootstrapState => Unit,
373374
failFuture: Throwable => Unit
374375
)
375376

@@ -425,11 +426,12 @@ private abstract class MillBuildServer(
425426
bootstrapBridge.runBootstrap(
426427
s"BSP:${req.requestName}",
427428
new BspBootstrapBridge.Body[Unit] {
428-
override def apply(
429-
evaluators: java.util.List[EvaluatorApi],
430-
watched: java.util.List[Watchable]
431-
): Unit =
432-
req.run(evaluators.asScala.toSeq, watched.asScala.toSeq)
429+
override def apply(state: BspBootstrapBridge.BootstrapState): Unit =
430+
if (state.errorOpt.isDefined && state.evaluators.isEmpty) {
431+
val error = state.errorOpt.get
432+
req.logger.error(error)
433+
req.failFuture(new IllegalStateException(error))
434+
} else req.run(state)
433435
}
434436
)
435437
} catch {
@@ -473,7 +475,7 @@ private abstract class MillBuildServer(
473475
requestName = prefix,
474476
logger = logger,
475477
future = future,
476-
run = (evaluators, watched) => {
478+
run = bootstrapState => {
477479
// The cancel check is also done in `runQueuedRequest` before the
478480
// bootstrap, but a request might be cancelled between bootstrap
479481
// start and body invocation; preserve that behavior here too.
@@ -482,9 +484,10 @@ private abstract class MillBuildServer(
482484
} else {
483485
val bspEvaluators = new BspEvaluators(
484486
topLevelProjectRoot,
485-
evaluators,
487+
bootstrapState.evaluators.asScala.toSeq,
486488
s => baseLogger.debug(s()),
487-
watched
489+
bootstrapState.watched.asScala.toSeq,
490+
bootstrapState.errorOpt
488491
)
489492
executeWithTiming(prefix, logger, future)(block(bspEvaluators, logger))
490493
}

‎runner/daemon/src/mill/daemon/MillBuildBootstrap.scala‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,12 @@ class MillBuildBootstrap(
324324
val collectSelectiveMetadata = tasksAndParams.nonEmpty
325325

326326
def outerModuleWatched: Seq[Watchable] =
327-
sharedState.get().moduleWatchedAt(depth - 1)
328-
.orElse(prevCommandState.moduleWatchedAt(depth - 1))
327+
// Final-depth module watches are launcher-local, so keep the previous
328+
// watch-loop iteration's value as a fallback there. Meta-build depths
329+
// publish their latest watch snapshot into sharedState even on failure,
330+
// so sharedState remains the canonical source for parent invalidation.
331+
prevCommandState.finalModuleWatchedAt(depth - 1)
332+
.orElse(sharedState.get().moduleWatchedAt(depth - 1))
329333
.getOrElse(Nil)
330334

331335
def watchedParentInputsChanged(): Boolean =
@@ -409,6 +413,16 @@ class MillBuildBootstrap(
409413
reporter = reporter(evaluator)
410414
) match {
411415
case (f: Result.Failure, evalWatches, moduleWatches) =>
416+
val failedShared = RunnerSharedState.Frame(
417+
evalWatched = evalWatches,
418+
moduleWatched = Some(moduleWatches)
419+
)
420+
val previous = sharedState.getAndUpdate(_.withFrame(depth, failedShared))
421+
Seq(
422+
prevCommandState.metaBuildFrameAt(depth)
423+
.flatMap(_.sharedFrame.classLoaderOpt),
424+
previous.frames.get(depth).flatMap(_.classLoaderOpt)
425+
).flatten.distinct.foreach(_.close())
412426
writeLease.close()
413427
nestedState
414428
.withMetaBuildFrame(
@@ -417,7 +431,7 @@ class MillBuildBootstrap(
417431
evaluator,
418432
evalWatches,
419433
moduleWatches
420-
)
434+
).copy(sharedFrame = failedShared)
421435
)
422436
.withError(mill.internal.Util.formatError(f, logger.prompt.errorColor))
423437

0 commit comments

Comments
 (0)