From 2780e0bd7b7c1d4bdbbfc07e7a2b978b48abf3c2 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 5 Dec 2024 19:56:53 +0100 Subject: [PATCH 1/2] Do not interrupt cleanup configuration Some options, such as --show-touches or --stay-awake, modify Android settings and must be restored upon exit. If scrcpy terminates (e.g. due to an early error) in the middle of the clean up configuration, the device may be left in an inconsistent state (some settings might be changed but not restored). This issue can be reproduced with high probability by forcing scrcpy to fail: scrcpy --show-touches --video-encoder=fail To prevent this problem, ensure that the clean up thread is not interrupted until the clean up process is started. Refs #5601 PR #5613 --- .../java/com/genymobile/scrcpy/CleanUp.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/genymobile/scrcpy/CleanUp.java b/server/src/main/java/com/genymobile/scrcpy/CleanUp.java index 1c6f170147..f372855bf2 100644 --- a/server/src/main/java/com/genymobile/scrcpy/CleanUp.java +++ b/server/src/main/java/com/genymobile/scrcpy/CleanUp.java @@ -24,6 +24,7 @@ public final class CleanUp { private boolean pendingRestoreDisplayPower; private Thread thread; + private boolean interrupted; private CleanUp(Options options) { thread = new Thread(() -> runCleanUp(options), "cleanup"); @@ -34,8 +35,10 @@ public static CleanUp start(Options options) { return new CleanUp(options); } - public void interrupt() { - thread.interrupt(); + public synchronized void interrupt() { + // Do not use thread.interrupt() because only the wait() call must be interrupted, not Command.exec() + interrupted = true; + notify(); } public void join() throws InterruptedException { @@ -97,15 +100,13 @@ private void runCleanUp(Options options) { try { run(displayId, restoreStayOn, disableShowTouches, powerOffScreen, restoreScreenOffTimeout); - } catch (InterruptedException e) { - // ignore } catch (IOException e) { Ln.e("Clean up I/O exception", e); } } private void run(int displayId, int restoreStayOn, boolean disableShowTouches, boolean powerOffScreen, int restoreScreenOffTimeout) - throws IOException, InterruptedException { + throws IOException { String[] cmd = { "app_process", "/", @@ -126,8 +127,15 @@ private void run(int displayId, int restoreStayOn, boolean disableShowTouches, b int localPendingChanges; boolean localPendingRestoreDisplayPower; synchronized (this) { - while (pendingChanges == 0) { - wait(); + while (!interrupted && pendingChanges == 0) { + try { + wait(); + } catch (InterruptedException e) { + throw new AssertionError("Clean up thread MUST NOT be interrupted"); + } + } + if (interrupted) { + break; } localPendingChanges = pendingChanges; localPendingRestoreDisplayPower = pendingRestoreDisplayPower; From c59a3c3169973abb4ce236e06990d58ae6567481 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 5 Dec 2024 20:08:21 +0100 Subject: [PATCH 2/2] Start cleanup process with setsid or nohup If available, start the cleanup process in a new session to reduce the likelihood of it being terminated along with the scrcpy server process on some devices. The binaries setsid and nohup are often available, but it is not guaranteed. Refs #5601 PR #5613 --- .../java/com/genymobile/scrcpy/CleanUp.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/com/genymobile/scrcpy/CleanUp.java b/server/src/main/java/com/genymobile/scrcpy/CleanUp.java index f372855bf2..ac265229d4 100644 --- a/server/src/main/java/com/genymobile/scrcpy/CleanUp.java +++ b/server/src/main/java/com/genymobile/scrcpy/CleanUp.java @@ -10,6 +10,8 @@ import java.io.File; import java.io.IOException; import java.io.OutputStream; +import java.util.ArrayList; +import java.util.List; /** * Handle the cleanup of scrcpy, even if the main process is killed. @@ -107,16 +109,22 @@ private void runCleanUp(Options options) { private void run(int displayId, int restoreStayOn, boolean disableShowTouches, boolean powerOffScreen, int restoreScreenOffTimeout) throws IOException { - String[] cmd = { - "app_process", - "/", - CleanUp.class.getName(), - String.valueOf(displayId), - String.valueOf(restoreStayOn), - String.valueOf(disableShowTouches), - String.valueOf(powerOffScreen), - String.valueOf(restoreScreenOffTimeout), - }; + + List cmd = new ArrayList<>(); + if (new File("/system/bin/setsid").exists()) { + cmd.add("/system/bin/setsid"); + } else if (new File("/system/bin/nohup").exists()) { + cmd.add("/system/bin/nohup"); + } + + cmd.add("app_process"); + cmd.add("/"); + cmd.add(CleanUp.class.getName()); + cmd.add(String.valueOf(displayId)); + cmd.add(String.valueOf(restoreStayOn)); + cmd.add(String.valueOf(disableShowTouches)); + cmd.add(String.valueOf(powerOffScreen)); + cmd.add(String.valueOf(restoreScreenOffTimeout)); ProcessBuilder builder = new ProcessBuilder(cmd); builder.environment().put("CLASSPATH", Server.SERVER_PATH);