From d6ba3d27d83eeff4e02ecbe4ea2c03c11b404d14 Mon Sep 17 00:00:00 2001 From: Kurt Himebauch <136133082+xXJSONDeruloXx@users.noreply.github.com> Date: Thu, 8 Oct 2026 21:39:00 -0400 Subject: [PATCH] fix: runtime removal never strands an undeletable leftover (#463) --- .../runtime/LinuxRuntimeInstaller.java | 58 +++++++++++++++---- .../launcher/runtime/RuntimeFileTree.java | 45 +++++++++++++- .../launcher/runtime/RuntimeRemovalTest.kt | 37 ++++++++++++ 3 files changed, 129 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/droiddeck/launcher/runtime/LinuxRuntimeInstaller.java b/app/src/main/java/com/droiddeck/launcher/runtime/LinuxRuntimeInstaller.java index 0598adf2f..302912924 100644 --- a/app/src/main/java/com/droiddeck/launcher/runtime/LinuxRuntimeInstaller.java +++ b/app/src/main/java/com/droiddeck/launcher/runtime/LinuxRuntimeInstaller.java @@ -227,7 +227,8 @@ public final class LinuxRuntimeInstaller { // Its quarantined home is not user data to carry into an update. File pendingRemoval = removalDirectory(LinuxRuntime.rootDir(context)); if (pendingRemoval.exists() && listener != null) listener.onProgress(context.getString(R.string.rtinst_removing_leftovers), -1); - RuntimeFileTree.delete(pendingRemoval, null); + if (!discard(pendingRemoval, null)) throw new IOException("Cannot clear the previous runtime: " + pendingRemoval); + emptyTrash(LinuxRuntime.rootDir(context).getParentFile()); String downloading = context.getString(R.string.rtinst_downloading); if (listener != null) listener.onProgress(Step.DOWNLOADING, downloading, 0); // Downloader reports a 0..1 fraction, or -1 while the total size is unknown. @@ -255,8 +256,7 @@ public final class LinuxRuntimeInstaller { File staging = new File(root.getParentFile(), LinuxRuntime.DIR + ".new"); File old = new File(root.getParentFile(), LinuxRuntime.DIR + ".old"); recoverInterruptedSwap(root, staging, old); - RuntimeFileTree.delete(staging, null); - if (!staging.mkdirs()) return false; + if (!discard(staging, null) || !staging.mkdirs()) return false; if (listener != null) listener.onProgress(context.getString(R.string.rtinst_extracting), -1); if (!extract(context, archive, staging, listener)) { RuntimeFileTree.delete(staging, null); @@ -264,7 +264,10 @@ public final class LinuxRuntimeInstaller { } FileUtils.writeString(new File(staging, VERSION_FILE), release.version); - RuntimeFileTree.delete(old, null); + if (!discard(old, null)) { + discard(staging, null); + return false; + } if (root.isDirectory() && !root.renameTo(old)) { RuntimeFileTree.delete(staging, null); return false; @@ -292,7 +295,7 @@ public final class LinuxRuntimeInstaller { if (old.isDirectory()) old.renameTo(root); return false; } - RuntimeFileTree.delete(old, null); + discard(old, null); return LinuxRuntime.isInstalled(context); } catch (Exception e) { Log.e(TAG, "install", e); @@ -315,6 +318,35 @@ public final class LinuxRuntimeInstaller { } } + /** Where entries the app cannot unlink are set aside, out of the way of the runtime's names. */ + static final String TRASH = ".runtime-trash"; + + /** + * Gets {@code tree} out from under its name. What can be deleted is; if something inside + * cannot be (a file made by root or {@code su} carries another SELinux category, and the app + * may not unlink it), the remainder is moved aside into {@link #TRASH} instead of blocking + * every later install and removal on it. False only when even that move is refused. + */ + static boolean discard(File tree, java.util.function.LongConsumer progress) { + if (RuntimeFileTree.deleteWhatCan(tree, progress)) return true; + File trash = new File(tree.getParentFile(), TRASH); + trash.mkdirs(); + File aside = new File(trash, tree.getName() + "-" + System.currentTimeMillis()); + if (tree.renameTo(aside)) { + Log.w(TAG, "could not delete all of " + tree + "; set the rest aside in " + aside); + return true; + } + Log.w(TAG, "could not delete or set aside " + tree); + return !tree.exists(); + } + + /** Another try at what {@link #discard} set aside. Whatever still cannot go stays there. */ + static void emptyTrash(File parent) { + File trash = new File(parent, TRASH); + if (trash.exists() && !RuntimeFileTree.deleteWhatCan(trash, null)) + Log.i(TAG, "some runtime leftovers still cannot be deleted: " + trash); + } + private static boolean isEmptyDir(File dir) { String[] names = dir.list(); return dir.isDirectory() && names != null && names.length == 0; @@ -369,15 +401,21 @@ public final class LinuxRuntimeInstaller { // Invalidate the installation before traversing it. A process killed halfway // through leaves a named removal to resume, never a launchable partial runtime. if (root.exists()) { - RuntimeFileTree.delete(pending, progress); - if (!root.renameTo(pending)) throw new IOException("Cannot prepare the runtime for removal"); + if (!discard(pending, progress) || !root.renameTo(pending)) + throw new IOException("Cannot prepare the runtime for removal"); } else if (!pending.isDirectory() && !pending.mkdirs()) { throw new IOException("Cannot prepare the runtime for removal"); } // Keep the quarantine until all leftovers are gone, so a failure stays retryable. - RuntimeFileTree.delete(new File(root.getParentFile(), root.getName() + ".new"), progress); - RuntimeFileTree.delete(new File(root.getParentFile(), root.getName() + ".old"), progress); - RuntimeFileTree.delete(pending, progress); + // An entry the app may not delete is set aside rather than failing the removal: + // a quarantine that can never clear would block reinstalling for good. + for (File leftover : new File[]{ + new File(root.getParentFile(), root.getName() + ".new"), + new File(root.getParentFile(), root.getName() + ".old"), + pending}) { + if (!discard(leftover, progress)) throw new IOException("Cannot remove " + leftover); + } + emptyTrash(root.getParentFile()); job.ok = true; report(text(app, R.string.rtinst_removed), 100); return true; diff --git a/app/src/main/java/com/droiddeck/launcher/runtime/RuntimeFileTree.java b/app/src/main/java/com/droiddeck/launcher/runtime/RuntimeFileTree.java index 5fd8d3611..ca59389f2 100644 --- a/app/src/main/java/com/droiddeck/launcher/runtime/RuntimeFileTree.java +++ b/app/src/main/java/com/droiddeck/launcher/runtime/RuntimeFileTree.java @@ -2,6 +2,7 @@ package com.droiddeck.launcher.runtime; import java.io.File; import java.io.IOException; +import java.nio.file.AccessDeniedException; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.LinkOption; @@ -9,11 +10,15 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; import java.util.function.LongConsumer; +import java.util.function.Predicate; /** Checked removal of an app-owned runtime tree, including read-only directories. */ final class RuntimeFileTree { private RuntimeFileTree() {} + /** Unit tests stand in for an entry the app cannot unlink (a file another SELinux user made). */ + static volatile Predicate refuseForTest; + static void delete(File root, LongConsumer progress) throws IOException { delete(root.toPath(), progress, new long[]{0}); } @@ -30,10 +35,48 @@ final class RuntimeFileTree { for (Path child : children) delete(child, progress, removed); } } - Files.delete(path); + unlink(path); if (progress != null) progress.accept(++removed[0]); } + /** + * Removes what it can of {@code root} and carries on past entries it cannot, rather than + * stopping at the first. Returns true when nothing is left. + */ + static boolean deleteWhatCan(File root, LongConsumer progress) { + return deleteWhatCan(root.toPath(), progress, new long[]{0}); + } + + private static boolean deleteWhatCan(Path path, LongConsumer progress, long[] removed) { + BasicFileAttributes attrs; + try { attrs = Files.readAttributes(path, BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS); } + catch (NoSuchFileException e) { return true; } + catch (IOException e) { return false; } + boolean emptied = true; + if (attrs.isDirectory()) { + try { + makeWritable(path.toFile()); + try (DirectoryStream children = Files.newDirectoryStream(path)) { + for (Path child : children) emptied &= deleteWhatCan(child, progress, removed); + } + } catch (IOException | RuntimeException e) { + emptied = false; + } + } + if (!emptied) return false; + try { unlink(path); } + catch (NoSuchFileException e) { return true; } + catch (IOException e) { return false; } + if (progress != null) progress.accept(++removed[0]); + return true; + } + + private static void unlink(Path path) throws IOException { + Predicate refuse = refuseForTest; + if (refuse != null && refuse.test(path)) throw new AccessDeniedException(path.toString()); + Files.delete(path); + } + private static void makeWritable(File dir) throws IOException { if (!dir.setReadable(true, true) || !dir.setWritable(true, true) || !dir.setExecutable(true, true)) throw new IOException("Cannot make directory removable: " + dir); diff --git a/app/src/test/java/com/droiddeck/launcher/runtime/RuntimeRemovalTest.kt b/app/src/test/java/com/droiddeck/launcher/runtime/RuntimeRemovalTest.kt index 9b93b9938..178e34d29 100644 --- a/app/src/test/java/com/droiddeck/launcher/runtime/RuntimeRemovalTest.kt +++ b/app/src/test/java/com/droiddeck/launcher/runtime/RuntimeRemovalTest.kt @@ -16,6 +16,7 @@ class RuntimeRemovalTest { private lateinit var dir: File @Before fun setUp() { dir = Files.createTempDirectory("runtime-removal").toFile() } @After fun tearDown() { + RuntimeFileTree.refuseForTest = null dir.setWritable(true) RuntimeFileTree.delete(dir, null) } @@ -160,4 +161,40 @@ class RuntimeRemovalTest { assertTrue(LinuxRuntimeInstaller.beginUninstall(null, root)!!.run(null)) assertNull(LinuxRuntimeInstaller.removalError()) } + + @Test fun anEntryTheAppCannotDeleteIsSetAsideInsteadOfStrandingTheRemoval() { + val root = runtime() + File(root, "root/.steampath").writeText("made by su") + RuntimeFileTree.refuseForTest = java.util.function.Predicate { it.fileName.toString() == ".steampath" } + assertTrue(LinuxRuntimeInstaller.beginUninstall(null, root)!!.run(null)) + assertNull(LinuxRuntimeInstaller.removalError()) + assertFalse(root.exists()) + assertFalse(File(dir, "linuxfs.removing").exists()) + // Only the undeletable entry and its parents remain, out of the way. + val trash = File(dir, LinuxRuntimeInstaller.TRASH) + assertEquals(listOf(".steampath"), trash.walk().filter { it.isFile }.map { it.name }.toList()) + assertEquals(setOf(LinuxRuntimeInstaller.TRASH), dir.list()!!.toSet()) + + // Once it can be deleted, the next removal clears the trash too. + RuntimeFileTree.refuseForTest = null + assertTrue(LinuxRuntimeInstaller.beginUninstall(null, root)!!.run(null)) + assertFalse(trash.exists()) + } + + @Test fun aStuckQuarantineFromAnEarlierBuildClearsOnTheNextRemoval() { + val pending = File(dir, "linuxfs.removing").apply { File(this, "root").mkdirs() } + File(pending, "root/.steampid").writeText("1") + File(pending, "root/game").writeText("game") + RuntimeFileTree.refuseForTest = java.util.function.Predicate { it.fileName.toString() == ".steampid" } + assertTrue(LinuxRuntimeInstaller.beginUninstall(null, File(dir, "linuxfs"))!!.run(null)) + assertFalse(pending.exists()) + } + + @Test fun deleteWhatCanKeepsGoingPastAnEntryItCannotDelete() { + val root = runtime() + RuntimeFileTree.refuseForTest = java.util.function.Predicate { it.fileName.toString() == "gamescope" } + assertFalse(RuntimeFileTree.deleteWhatCan(root, null)) + assertEquals(listOf("gamescope"), root.walk().filter { it.isFile }.map { it.name }.toList()) + assertFalse(File(root, "root").exists()) + } }