diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppHidingScreen.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppHidingScreen.kt index 4b16313..fe40fe7 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppHidingScreen.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppHidingScreen.kt @@ -114,7 +114,15 @@ fun AppHidingScreen( // (itself) and strip its own package from the result, so frameworks // see a self-lookup NameNotFoundException and bail. Collapse to // observer-only on load so the next Save persists the fix. + // + // While `dirty` is true the user has unsaved checkbox edits — don't + // overwrite them with a fresh snapshot from the cache. Caches can + // refresh under us (ON_RESUME, another screen calling + // `TargetsCache.refresh()`) and silently dropping the edits is the + // worst outcome here. Treat saving as the only legitimate point + // where the snapshot becomes authoritative again. LaunchedEffect(cachedApps, targets) { + if (dirty) return@LaunchedEffect val apps = cachedApps ?: return@LaunchedEffect val t = targets ?: return@LaunchedEffect val hidden = t.hiddenPkgs diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppPickerScreen.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppPickerScreen.kt index a331542..34176de 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppPickerScreen.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/AppPickerScreen.kt @@ -98,10 +98,14 @@ fun AppPickerScreen( } // Merge cached app metadata with per-screen target flags. Cheap — - // runs whenever either side of the cache changes. Resets local - // `dirty` when a fresh snapshot arrives, since that means disk - // state just synced (Save finished, or user tapped Refresh). + // runs whenever either side of the cache changes. + // + // While `dirty` is true the user has unsaved checkbox edits — don't + // overwrite them with a fresh snapshot. Caches can refresh under us + // (ON_RESUME, another screen calling `TargetsCache.refresh()`) and + // silently dropping the edits is the worst outcome. LaunchedEffect(cachedApps, targets) { + if (dirty) return@LaunchedEffect val apps = cachedApps ?: return@LaunchedEffect val t = targets ?: return@LaunchedEffect val selfPkg = context.packageName diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/PortsHidingScreen.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/PortsHidingScreen.kt index c60ebd2..4eefe5a 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/PortsHidingScreen.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/PortsHidingScreen.kt @@ -106,7 +106,12 @@ fun PortsHidingScreen( TargetsCache.ensureLoaded(scope, context) } + // While `dirty` is true the user has unsaved checkbox edits — don't + // overwrite them with a fresh snapshot. Caches can refresh under us + // (ON_RESUME, another screen calling `TargetsCache.refresh()`) and + // silently dropping the edits is the worst outcome. LaunchedEffect(cachedApps, targets) { + if (dirty) return@LaunchedEffect val apps = cachedApps ?: return@LaunchedEffect val t = targets ?: return@LaunchedEffect val selfPkg = context.packageName diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ShellUtils.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ShellUtils.kt index 43f0adf..03cb717 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ShellUtils.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/ShellUtils.kt @@ -6,6 +6,8 @@ import dev.okhsunrog.vpnhide.generated.IfaceLists import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import java.io.File +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference private const val TAG = "VpnHide" @@ -26,20 +28,50 @@ internal const val KMOD_LOAD_DMESG_FILE = "/data/adb/vpnhide_kmod/load_dmesg" internal const val ZYGISK_MODULE_DIR = "/data/adb/modules/vpnhide_zygisk" internal const val ZYGISK_STATUS_FILE_NAME = "vpnhide_zygisk_active" +/** Default cap on a single su invocation. Most root commands here finish + * in milliseconds; this only fires if the su binary is genuinely stuck + * (e.g. waiting on a GUI prompt that the user dismissed). */ +internal const val SU_DEFAULT_TIMEOUT_SEC: Long = 10 + /** * Returns exit code and stdout. Exit code -1 means the su binary - * couldn't be executed at all (not installed or permission denied). + * couldn't be executed at all (not installed, permission denied, or + * still running after [timeoutSec] seconds — in which case it gets + * destroyForcibly()'d so we don't leak the process). + * + * Both pipes are drained on dedicated threads — `readText()` directly + * on `proc.inputStream` would block until EOF, so a hung child means + * `waitFor(timeout)` is never even reached. The threads exit naturally + * once the child (or destroyForcibly) closes its pipes. */ -internal fun suExec(cmd: String): Pair = +internal fun suExec( + cmd: String, + timeoutSec: Long = SU_DEFAULT_TIMEOUT_SEC, +): Pair = try { val proc = Runtime.getRuntime().exec(arrayOf("su", "-c", cmd)) try { - val stderrDrain = Thread { proc.errorStream.readBytes() } + val stdoutHolder = AtomicReference("") + val stdoutDrain = + Thread { + runCatching { stdoutHolder.set(proc.inputStream.bufferedReader().readText()) } + } + val stderrDrain = Thread { runCatching { proc.errorStream.readBytes() } } + stdoutDrain.start() stderrDrain.start() - val stdout = proc.inputStream.bufferedReader().readText() - val exitCode = proc.waitFor() - stderrDrain.join() - exitCode to stdout + + val finished = proc.waitFor(timeoutSec, TimeUnit.SECONDS) + if (!finished) { + Log.w(TAG, "su exec timed out after ${timeoutSec}s: $cmd") + proc.destroyForcibly() + } + // After destroyForcibly the pipes close and the drains exit; + // a 1s join is plenty and bounds the worst case. + stdoutDrain.join(1_000) + stderrDrain.join(1_000) + + val exit = if (finished) proc.exitValue() else -1 + exit to stdoutHolder.get() } finally { proc.destroy() } @@ -48,7 +80,10 @@ internal fun suExec(cmd: String): Pair = -1 to "" } -internal suspend fun suExecAsync(cmd: String): Pair = withContext(Dispatchers.IO) { suExec(cmd) } +internal suspend fun suExecAsync( + cmd: String, + timeoutSec: Long = SU_DEFAULT_TIMEOUT_SEC, +): Pair = withContext(Dispatchers.IO) { suExec(cmd, timeoutSec) } /** * Single source of truth for "is a VPN currently up?". Both the dashboard