Skip to content

Commit 795c1ae

Browse files
committed
Fix race conditions due to non-volatile lazy vals
These races could be observed by running: dotty-compiler-bootstrapped/test-only *.CompilationTests These never happened on the CI so far because the order in which the classes are run is not deterministic and we got "lucky" that the first tests running the compiler did not use parallelism. The races were caused by a combination of multiple factors: - The new classpath implementation imported from 2.12 has a shared global cache of ClassPath instances (see ZipAndJarFileLookupFactory.scala) - If multiple threads try to initialize a lazy val in dotty, only one will succeed and the other will get null (in scalac, each thread will redo the initialization) - The classpath implementation relies on code from scala.reflect.io that was recently imported into dotty, this code contains fields with lazy vals. The race with the lazy val in ZipArchive was easy to fix since it immediately lead to NullPointerException, but the one in AbstractFile was much harder to find because the only method called on "lazy val extension" is `==` which works with null. This suggests that we should implement a compiler option to check for lazy val races at runtime. In both cases we fixed the issue by simply dropping the `lazy` instead of adding an `@volatile`, for ZipArchive I cannot think of a usecase where you would not want to force the lazy val, and for `AbstractFile` the cost of always computing the extension is probably less than adding a lock.
1 parent b7f5bf9 commit 795c1ae

File tree

2 files changed

+2
-2
lines changed

2 files changed

+2
-2
lines changed

compiler/src/dotty/tools/io/AbstractFile.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ abstract class AbstractFile extends Iterable[AbstractFile] {
9898

9999
/** Checks extension case insensitively. */
100100
def hasExtension(other: String) = extension == other.toLowerCase
101-
private lazy val extension: String = Path.extension(name)
101+
private val extension: String = Path.extension(name)
102102

103103
/** The absolute file, if this is a relative file. */
104104
def absolute: AbstractFile

compiler/src/dotty/tools/io/ZipArchive.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ final class FileZipArchive(file: JFile) extends ZipArchive(file) {
149149
override def sizeOption: Option[Int] = Some(zipEntry.getSize.toInt)
150150
}
151151

152-
lazy val (root, allDirs) = {
152+
val (root, allDirs) = {
153153
val root = new DirEntry("/")
154154
val dirs = mutable.HashMap[String, DirEntry]("/" -> root)
155155
val zipFile = openZipFile()

0 commit comments

Comments
 (0)