IC: Close caches before falling back to non-incremental compile
Previously, we didn't close caches if incremental compilation failed.
The assumption was that we would fall back to non-incremental
compilation, where it shouldn't matter whether caches were closed or not
as non-incremental compilation should be able to recover from a
corrupted state of the caches.
However, this choice might have caused issues such as KT-55709 where
non-incremental compilation fails after fallback.
In this commit, we will now always close the caches before falling back,
just to be safe.
TODO:
1. We'll need to verify whether not closing the caches was the actual
cause of KT-55709, and that this commit fixes it.
2. Even if this commit fixes the issue, we'll need to see why
non-incremental compilation fails to recover from unclosed caches
(it would remain unsafe if that was not addressed).
^KT-55709 In progress (see TODO #1 above)
This commit is contained in:
committed by
CherepanovAleksei
parent
7156200953
commit
63a0660cc3
+17
-8
@@ -154,6 +154,19 @@ abstract class IncrementalCompilerRunner<
|
|||||||
}
|
}
|
||||||
changedFiles as ChangedFiles.Known?
|
changedFiles as ChangedFiles.Known?
|
||||||
|
|
||||||
|
// Because `caches` is a Closeable resource, it is important to close them in the event of an exception or before any return
|
||||||
|
// statements.
|
||||||
|
// Note: Historically, closing caches used to throw an exception sometimes, so currently we want to collect those exceptions. In the
|
||||||
|
// future, if closing caches is safe, and we are no longer interested in those exceptions, we can simplify this code by using
|
||||||
|
// Kotlin's `Closable.use` function (see `compileNonIncrementally`).
|
||||||
|
fun closeCaches(caches: CacheManager, activeException: Throwable) {
|
||||||
|
try {
|
||||||
|
caches.close()
|
||||||
|
} catch (e: Throwable) {
|
||||||
|
activeException.addSuppressed(e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
val caches = createCacheManager(args, projectDir)
|
val caches = createCacheManager(args, projectDir)
|
||||||
val exitCode: ExitCode
|
val exitCode: ExitCode
|
||||||
try {
|
try {
|
||||||
@@ -161,7 +174,7 @@ abstract class IncrementalCompilerRunner<
|
|||||||
val knownChangedFiles: ChangedFiles.Known = try {
|
val knownChangedFiles: ChangedFiles.Known = try {
|
||||||
getChangedFiles(changedFiles, allSourceFiles, caches)
|
getChangedFiles(changedFiles, allSourceFiles, caches)
|
||||||
} catch (e: Throwable) {
|
} catch (e: Throwable) {
|
||||||
// Don't need to close caches in cases where we return `ICResult.Failed` because we will compile non-incrementally anyway
|
closeCaches(caches, e)
|
||||||
return ICResult.Failed(IC_FAILED_TO_GET_CHANGED_FILES, e)
|
return ICResult.Failed(IC_FAILED_TO_GET_CHANGED_FILES, e)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -173,6 +186,7 @@ abstract class IncrementalCompilerRunner<
|
|||||||
calculateSourcesToCompile(caches, knownChangedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
|
calculateSourcesToCompile(caches, knownChangedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
|
||||||
}
|
}
|
||||||
} catch (e: Throwable) {
|
} catch (e: Throwable) {
|
||||||
|
closeCaches(caches, e)
|
||||||
return ICResult.Failed(IC_FAILED_TO_COMPUTE_FILES_TO_RECOMPILE, e)
|
return ICResult.Failed(IC_FAILED_TO_COMPUTE_FILES_TO_RECOMPILE, e)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -196,16 +210,11 @@ abstract class IncrementalCompilerRunner<
|
|||||||
exitCode = try {
|
exitCode = try {
|
||||||
compileImpl(compilationMode as CompilationMode.Incremental, allSourceFiles, args, caches, abiSnapshotData, messageCollector)
|
compileImpl(compilationMode as CompilationMode.Incremental, allSourceFiles, args, caches, abiSnapshotData, messageCollector)
|
||||||
} catch (e: Throwable) {
|
} catch (e: Throwable) {
|
||||||
|
closeCaches(caches, e)
|
||||||
return ICResult.Failed(IC_FAILED_TO_COMPILE_INCREMENTALLY, e)
|
return ICResult.Failed(IC_FAILED_TO_COMPILE_INCREMENTALLY, e)
|
||||||
}
|
}
|
||||||
} catch (e: Throwable) {
|
} catch (e: Throwable) {
|
||||||
// Because `caches` is a Closeable resource, it is good practice to close them in the event of an exception (in addition to
|
closeCaches(caches, e)
|
||||||
// closing them after a normal use).
|
|
||||||
try {
|
|
||||||
caches.close()
|
|
||||||
} catch (e2: Throwable) {
|
|
||||||
e.addSuppressed(e2)
|
|
||||||
}
|
|
||||||
throw e
|
throw e
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user