fix(lsposed/app): suExec timeout + cache refresh preserves edits

Two small Kotlin fixes — both prevent silent UX failures.

ShellUtils.suExec — bound the runtime of a single su invocation.
Previously `proc.waitFor()` blocked indefinitely if the su binary hung
(e.g. waiting on a GUI prompt the user dismissed). UI was already on
Dispatchers.IO so the main thread didn't block, but the spinner just
never went away.

  * Drain stdout on a dedicated thread (an AtomicReference holds the
    result), same as the existing stderr drain. `readText()` directly
    on the process input stream blocks until EOF — without a separate
    drain, a hung child means waitFor(timeout) never even gets reached.
  * `proc.waitFor(timeoutSec, TimeUnit.SECONDS)` with a 10-second cap.
    On timeout, destroyForcibly() the child and return exit=-1.
  * 1-second join on each drain to bound worst-case cleanup.
  * `SU_DEFAULT_TIMEOUT_SEC` constant + `timeoutSec` parameter on both
    `suExec` and `suExecAsync` so callers with a longer-running command
    (none today, but easy to add) can override.

AppHidingScreen / AppPickerScreen / PortsHidingScreen — don't drop
unsaved checkbox edits when a cache refresh fires under us.

  * `LaunchedEffect(cachedApps, targets)` rebuilds `allApps` from disk
    snapshot whenever either flow emits — ON_RESUME via
    `TargetsCache.ensureLoaded`, or another screen calling
    `TargetsCache.refresh()`. With dirty=true (user has unsaved
    checkbox toggles), the rebuild silently overwrote local state and
    flipped dirty back to false. Edits gone.
  * Add an `if (dirty) return@LaunchedEffect` guard at the top of each
    of the three rebuilders. Saves restore the snapshot-as-truth path
    naturally (Save flips dirty=false, then the next emission rebuilds).

Local: ktlint clean, `:app:lintDebug + :app:testDebugUnitTest` BUILD SUCCESSFUL.
Verified on Pixel 8 Pro: dashboard 26/26 PASS in Enforcing — no
regression in any probe.
This commit is contained in:
okhsunrog 2026-04-27 02:17:01 +03:00
parent c92d690694
commit 83ebbd340c
4 changed files with 63 additions and 11 deletions

View file

@ -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

View file

@ -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

View file

@ -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

View file

@ -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<Int, String> =
internal fun suExec(
cmd: String,
timeoutSec: Long = SU_DEFAULT_TIMEOUT_SEC,
): Pair<Int, String> =
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<Int, String> =
-1 to ""
}
internal suspend fun suExecAsync(cmd: String): Pair<Int, String> = withContext(Dispatchers.IO) { suExec(cmd) }
internal suspend fun suExecAsync(
cmd: String,
timeoutSec: Long = SU_DEFAULT_TIMEOUT_SEC,
): Pair<Int, String> = withContext(Dispatchers.IO) { suExec(cmd, timeoutSec) }
/**
* Single source of truth for "is a VPN currently up?". Both the dashboard