Failure in incremental bootstrapped builds #4929
Comments
Recreate `tempDir` if we're reused after `complete` was called and deleted the existing `tempDir`. The output in scala#4929 was produced by this commit.
|
Looking at the logs you posted, I don’t fully understand the life-cycle of the external class file manager. I see two instances of the TASTY file manager, so I’m not sure when an instance is reused. Can we figure this out before committing to any fix? Sorry, I cannot look into this currently. I’m on vacation with limited access to internet and a computer. |
|
@jvican answered my questions and approved the fix in #4930. Since this is hard to workaround and you're on vacation (enjoy!), do you mind if I merge this quickfix but reopen the issue for further investigation when you're back? See also below for an answer to your question.
There are two instances, but we see deletion and reuse on the same instance in these lines: And if reuse is possible at all, we need to reinitialize the tempDir. Based on the output, it also seems we have one file manager for I found the zinc code calling |
Recreate `tempDir` if we're reused after `complete` was called and deleted the existing `tempDir`. The output in scala#4929 was produced by this commit.
|
Note to myself: |
|
I cannot reproduce the original issue. I set up a sbt project with the Dotty plugin enabled and some instrumentation. diff --git a/sbt-dotty/src/dotty/tools/sbtplugin/TastyFileManager.scala b/sbt-dotty/src/dotty/tools/sbtplugin/TastyFileManager.scala
index 38d4d029f..448e70688 100644
--- a/sbt-dotty/src/dotty/tools/sbtplugin/TastyFileManager.scala
+++ b/sbt-dotty/src/dotty/tools/sbtplugin/TastyFileManager.scala
@@ -20,18 +20,13 @@ import scala.collection.mutable
* temporary directory as class files.
*/
final class TastyFileManager extends ClassFileManager {
- private[this] var _tempDir: File = null
- private[this] def tempDir = {
- if (_tempDir == null) {
- _tempDir = Files.createTempDirectory("backup").toFile
- }
- _tempDir
- }
+ private[this] val tempDir = Files.createTempDirectory("backup").toFile
private[this] val generatedTastyFiles = new mutable.HashSet[File]
private[this] val movedTastyFiles = new mutable.HashMap[File, File]
override def delete(classes: Array[File]): Unit = {
+ println(s"ClassFileManager.delete: $this")
val tasties = tastyFiles(classes)
val toBeBackedUp = tasties
.filter(t => t.exists && !movedTastyFiles.contains(t) && !generatedTastyFiles(t))
@@ -40,18 +35,17 @@ final class TastyFileManager extends ClassFileManager {
IO.deleteFilesEmptyDirs(tasties)
}
- override def generated(classes: Array[File]): Unit =
+ override def generated(classes: Array[File]): Unit = {
+ println(s"ClassFileManager.generated: $this")
generatedTastyFiles ++= tastyFiles(classes)
+ }
override def complete(success: Boolean): Unit = {
+ println(s"ClassFileManager.complete($success): $this")
if (!success) {
IO.deleteFilesEmptyDirs(generatedTastyFiles)
for ((orig, tmp) <- movedTastyFiles) IO.move(tmp, orig)
}
- if (_tempDir != null) {
- IO.delete(tempDir)
- _tempDir = null
- }
}The TastyFileManger is never reused across multiple compilations: It is only reused when a compilation has multiple incremental steps. But this is expected and should not be an issue since the @Blaisorblade Do you know how I could reproduce the crash you reported? |
|
I forget, but assuming what I wrote is accurate:
the manager is reused for That suggests having different managers for |
Right, I was able to reproduce.
Yes, I think that's the proper fix. A project and its tests should be treated as two separate modules. If I have a |
|
Looking more into it, it seems that the external class file manager must be reusable. The reason we get multiple instances of the TastyFileManager is sbt Here is the code in Zinc that deals with class file manager. For the default class file manager, |
|
@allanrenucci Yes, that fits with my understanding and @jvican's comments on Gitter.
|
Fix #4929: Make TastyFileManager reusable
Uh oh!
There was an error while loading. Please reload this page.
Since a few days (I guess since c9f782c and #4857), I get non-repeatably traces such as, which are only worked around by a clean compilation:
Apparently, that's because a single
TastyFileManagercan be reused aftercompleteis called: then, we usetempDiraftercompletehas removed it. Logs with instrumentation (after the fix) witnessing this (beware the different instance pointers):IIUC, @jvican suggested that the lifecycle of external ClassFileManager (which TastyFileManager is) might be different from the one of internal ones. That would explain why this bug appears here but not in
TransactionalClassFileManager(which uses the same pattern).The text was updated successfully, but these errors were encountered: