fix: Move temp APK into place instead of copying it

replaceFileSafely copied the source into a .new candidate before the
atomic move, doubling peak disk usage: with free space between 1.5x and
2x the APK size, the download completed and then promotion failed on the
copy, retrying against the same full temp file.

Source and target always live in the same cache directory, so a direct
ATOMIC_MOVE (rename) needs no extra space and keeps the same guarantee:
it either fully succeeds or leaves both files intact. The existing 1.5x
margin in checkDiskSpace is now genuinely sufficient.

Addresses the Codex review finding on UniversalApkManager.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Moe Hamade 2026-07-24 20:00:28 +03:00
parent 13f940271b
commit 74225c21c3

View File

@ -699,34 +699,25 @@ class UniversalApkManager(private val context: Context) {
/**
* Commit [source] to [target] without removing a valid target first.
* Both files live in the same cache directory, so ATOMIC_MOVE is available
* on normal Android filesystems. The fallback still uses REPLACE_EXISTING
* and leaves the old target intact if preparing the candidate fails.
* Both files live in the same cache directory, so this is a rename, not a
* copy no extra disk space is needed and ATOMIC_MOVE either fully
* succeeds or leaves both files intact.
*/
private fun replaceFileSafely(source: File, target: File) {
val candidate = File(target.parentFile, "${target.name}.new")
if (source != candidate) {
source.copyTo(candidate, overwrite = true)
}
try {
Files.move(
candidate.toPath(),
source.toPath(),
target.toPath(),
StandardCopyOption.ATOMIC_MOVE,
StandardCopyOption.REPLACE_EXISTING
)
} catch (_: AtomicMoveNotSupportedException) {
Files.move(
candidate.toPath(),
source.toPath(),
target.toPath(),
StandardCopyOption.REPLACE_EXISTING
)
}
if (source != target && source.exists()) {
source.delete()
}
}
/**