Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix usage of process builder #370

Closed
wants to merge 2 commits into from
Closed

fix usage of process builder #370

wants to merge 2 commits into from

Conversation

mythsman
Copy link

No description provided.

@mythsman
Copy link
Author

#369

@rom1v
Copy link
Collaborator

rom1v commented Mar 12, 2021

Following the discussion on #369, this should be sufficient, isn't it?

Diff ignoring spaces (git diff -b):

diff --git a/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java b/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java
index 03b6efc..7430560 100644
--- a/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java
+++ b/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java
@@ -357,10 +357,7 @@ public final class Main {
         List<String> command = createAdbCommand(serial, "shell", "dumpsys", "package", "com.genymobile.gnirehtet");
         Log.d(TAG, "Execute: " + command);
         Process process = new ProcessBuilder(command).start();
-        int exitCode = process.waitFor();
-        if (exitCode != 0) {
-            throw new CommandExecutionException(command, exitCode);
-        }
+        try {
             Scanner scanner = new Scanner(process.getInputStream());
             // read the versionCode of the installed package
             Pattern pattern = Pattern.compile("^    versionCode=(\\p{Digit}+).*");
@@ -371,6 +368,13 @@ public final class Main {
                     return !REQUIRED_APK_VERSION_CODE.equals(installedVersionCode);
                 }
             }
+        } finally {
+            int exitCode = process.waitFor();
+            if (exitCode != 0) {
+                // Overwrite any pending exception, the command just failed
+                throw new CommandExecutionException(command, exitCode);
+            }
+        }
         return true;
     }
 

Full diff:

diff --git a/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java b/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java
index 03b6efc..7430560 100644
--- a/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java
+++ b/relay-java/src/main/java/com/genymobile/gnirehtet/Main.java
@@ -357,18 +357,22 @@ public final class Main {
         List<String> command = createAdbCommand(serial, "shell", "dumpsys", "package", "com.genymobile.gnirehtet");
         Log.d(TAG, "Execute: " + command);
         Process process = new ProcessBuilder(command).start();
-        int exitCode = process.waitFor();
-        if (exitCode != 0) {
-            throw new CommandExecutionException(command, exitCode);
-        }
-        Scanner scanner = new Scanner(process.getInputStream());
-        // read the versionCode of the installed package
-        Pattern pattern = Pattern.compile("^    versionCode=(\\p{Digit}+).*");
-        while (scanner.hasNextLine()) {
-            Matcher matcher = pattern.matcher(scanner.nextLine());
-            if (matcher.matches()) {
-                String installedVersionCode = matcher.group(1);
-                return !REQUIRED_APK_VERSION_CODE.equals(installedVersionCode);
+        try {
+            Scanner scanner = new Scanner(process.getInputStream());
+            // read the versionCode of the installed package
+            Pattern pattern = Pattern.compile("^    versionCode=(\\p{Digit}+).*");
+            while (scanner.hasNextLine()) {
+                Matcher matcher = pattern.matcher(scanner.nextLine());
+                if (matcher.matches()) {
+                    String installedVersionCode = matcher.group(1);
+                    return !REQUIRED_APK_VERSION_CODE.equals(installedVersionCode);
+                }
+            }
+        } finally {
+            int exitCode = process.waitFor();
+            if (exitCode != 0) {
+                // Overwrite any pending exception, the command just failed
+                throw new CommandExecutionException(command, exitCode);
             }
         }
         return true;

@mythsman
Copy link
Author

Oh, I tested it ,and you are right 👍 !

rom1v added a commit that referenced this pull request Mar 14, 2021
The process may stuck because its output buffer is full, but the output
was consumed only after the process was terminated, causing a deadlock.

Consume process output before waiting for process termination to fix the
problem.

Refs #369 <#369>
Refs #370 <#370>

Co-authored-by: tusenpo <tusenpo@pinduoduo.com>
@rom1v
Copy link
Collaborator

rom1v commented Mar 14, 2021

Thank you 👍

Fixed on dev branch: 9ad9bc8

I added you as a co-author of the commit, since you did the main part of the job, which is explaining the root cause: #369 (comment)

@rom1v rom1v closed this Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants