Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions app/src/main/java/com/winlator/core/ProcessHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executors;

public abstract class ProcessHelper {
Expand Down Expand Up @@ -106,6 +107,71 @@ public static int exec(String command, String[] envp, File workingDir) {
return exec(command, envp, workingDir, null);
}

// Execute the command and capture its output.
// Use ProcessBuilder to merge stderr into stdout
// Deadlock-safe strategy:
// includeStderr=true - redirectErrorStream(true): stderr merged into stdout at the OS
// level, single pipe, zero deadlock risk.
// includeStderr=false - streams are kept separate; a daemon thread drains and discards
// stderr concurrently so the 64 KB pipe buffer never fills while
// the main thread reads stdout. This keeps stdout clean for callers
// such as SteamTokenLogin
public static String execWithOutput(String command, String[] envp, File workingDir, boolean includeStderr) {
StringBuilder output = new StringBuilder();
Thread stderrDrainer = null;
try {
ProcessBuilder pb = new ProcessBuilder(splitCommand(command));
Map<String, String> env = pb.environment();
env.clear();
if (envp != null) {
Comment on lines +124 to +126
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Java/Android, when Runtime.exec(String[], String[], File) is called with envp = null, does the child inherit the current process environment? How should ProcessBuilder be configured to preserve that behavior?

💡 Result:

Yes, when Runtime.exec(String[], String[], File) is called with envp = null, the child process inherits the environment settings of the current Java process [1][2][3]. To achieve this same behavior using ProcessBuilder, you do not need to perform any special configuration. By default, a ProcessBuilder instance initializes its environment map as a copy of the current process's environment [4][5]. When you call start on a ProcessBuilder, the subprocess uses this map as its environment [5]. If you have modified the environment map (e.g., by adding or removing variables) and wish to revert to the default behavior of inheriting the current process's environment, you would need to manually clear the map and repopulate it with the current process's environment variables, typically retrieved via System.getenv [4][5]. However, for a newly created ProcessBuilder, inheritance is the default state [4][5].

Citations:


🏁 Script executed:

find . -name "ProcessHelper.java" -type f

Repository: utkarshdalal/GameNative

Length of output: 124


🏁 Script executed:

cat -n ./app/src/main/java/com/winlator/core/ProcessHelper.java | sed -n '115,135p'

Repository: utkarshdalal/GameNative

Length of output: 1366


Preserve inherited environment when envp is null.

env.clear() at line 125 runs before the null check, so execWithOutput(..., null, ...) now starts children with an empty environment instead of inheriting the current one like Runtime.exec(..., null, ...) does.

Suggested fix
             ProcessBuilder pb = new ProcessBuilder(splitCommand(command));
             Map<String, String> env = pb.environment();
-            env.clear();
             if (envp != null) {
+                env.clear();
                 for (String kv : envp) {
                     int eq = kv.indexOf('=');
                     if (eq > 0) env.put(kv.substring(0, eq), kv.substring(eq + 1));
                 }
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/com/winlator/core/ProcessHelper.java` around lines 124 -
126, The code in ProcessHelper (within execWithOutput where pb.environment() is
assigned to env) currently calls env.clear() unconditionally which removes the
inherited environment; change the logic so the existing environment is preserved
when envp is null: only clear and replace the environment when envp is non-null
(i.e., move or gate env.clear() and the population loop behind the if (envp !=
null) check), or alternatively, if envp is non-null, apply/overwrite only the
provided entries into pb.environment() rather than wiping it out; update the
code around pb.environment(), env.clear(), and the envp handling to reflect
this.

for (String kv : envp) {
int eq = kv.indexOf('=');
if (eq > 0) env.put(kv.substring(0, eq), kv.substring(eq + 1));
}
}
if (workingDir != null) pb.directory(workingDir);
pb.redirectErrorStream(includeStderr); // merge only when caller wants stderr

java.lang.Process process = pb.start();

// When not merging, drain stderr on a daemon thread to prevent pipe-buffer deadlock.
// SteamTokenLogin uses this when calling includeStderr=false
if (!includeStderr) {
final InputStream stderrStream = process.getErrorStream();
stderrDrainer = new Thread(() -> {
try {
byte[] buf = new byte[4096];
while (stderrStream.read(buf) != -1) {
if (Thread.currentThread().isInterrupted())
break;
}
} catch (IOException ignored) {}
}, "stderr-drainer");
stderrDrainer.setDaemon(true); // won't block app shutdown if something goes wrong
stderrDrainer.start();
}

// Read stdout (or the merged stream) inline; EOF arrives after the process exits.
try (BufferedReader r = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
String l;
while ((l = r.readLine()) != null) output.append(l).append("\n");
}

// Process has already exited (we drained its stdout to EOF).
// waitFor() reaps the OS process-table entry.
process.waitFor();

if (stderrDrainer != null) {
stderrDrainer.join(5_000); // bounded wait; daemon thread is reaped on JVM exit anyway
}
} catch (Exception e) {
output.append("Error: ").append(e.getMessage());
}

// Format output: trim trailing whitespace/newlines
return output.toString().trim();
}

public static class ProcessInfo {
public final int pid;
public final int ppid;
Expand Down
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a few AI comments here. Would you mind addressing these @Catpotatos and I'll take another look at this afterwards?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @phobos665 , i believe it's all been addressed now, i added a seperate method with pipe drain for the callers that explicitly mention includeStderr=false to address the bot comment. All others that don't mention it for booting/launching, will go through the merged pipe.
Tested on Retroid Pocket 5:

  • All Protons installed on first time install with Wine logs + Box64 logs on. No infinite bootings.
  • Tested an Amazon, Steam and custom game.

Original file line number Diff line number Diff line change
Expand Up @@ -709,48 +709,9 @@ public String execShellCommand(String command, boolean includeStderr) {
FileUtils.chmod(box64File, 0755);
}

// Execute the command and capture its output.
//
// IMPORTANT: stderr MUST be drained concurrently with stdout, even when
// includeStderr=false. Wine spits out a flood of fixme:/err: lines on
// stderr; if we don't read it, the kernel's pipe buffer (~64 KB) fills,
// wine's next write(stderr,...) blocks, and the whole subprocess hangs
// forever -- which then deadlocks our stdout read too. SteamTokenLogin
// calls this with includeStderr=false, so this used to hang on boot.
try {
Log.d("BionicProgramLauncherComponent", "Shell command is " + finalCommand);
java.lang.Process process = Runtime.getRuntime().exec(finalCommand, envVars.toStringArray(), workingDir != null ? workingDir : imageFs.getRootDir());
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
BufferedReader errorReader = new BufferedReader(new InputStreamReader(process.getErrorStream()));

final StringBuilder stderrBuf = new StringBuilder();
Thread stderrPump = new Thread(() -> {
try {
String l;
while ((l = errorReader.readLine()) != null) {
if (includeStderr) stderrBuf.append(l).append('\n');
// else: discard, but we MUST still consume the stream
}
} catch (IOException ignored) {
// Subprocess closed stderr; fine.
}
}, "execShellCommand-stderr-pump");
stderrPump.setDaemon(true);
stderrPump.start();

String line;
while ((line = reader.readLine()) != null) {
output.append(line).append("\n");
}
process.waitFor();
stderrPump.join();
if (includeStderr) output.append(stderrBuf);
} catch (Exception e) {
output.append("Error: ").append(e.getMessage());
}

// Format output: trim trailing whitespace/newlines
return output.toString().trim();
Log.d("BionicProgramLauncherComponent", "Shell command is " + finalCommand);
return ProcessHelper.execWithOutput(finalCommand, envVars.toStringArray(),
workingDir != null ? workingDir : imageFs.getRootDir(), includeStderr);
Comment on lines +505 to +507
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't log the raw shell command.

finalCommand can carry sensitive arguments, and execShellCommand(...) is used by auth-adjacent flows, so this will leak secrets and paths into logcat. Prefer a static message or a redacted form, and mirror that change in the Glibc launcher too.

Suggested fix
-        Log.d("BionicProgramLauncherComponent", "Shell command is " + finalCommand);
+        Log.d("BionicProgramLauncherComponent", "Executing shell command");
         return ProcessHelper.execWithOutput(finalCommand, envVars.toStringArray(),
                 workingDir != null ? workingDir : imageFs.getRootDir(), includeStderr);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/com/winlator/xenvironment/components/BionicProgramLauncherComponent.java`
around lines 505 - 507, Remove the log that prints the raw shell command in
BionicProgramLauncherComponent (the Log.d call that concatenates finalCommand);
instead log a static message or a redacted string (e.g., "Shell command executed
(redacted)") and, if helpful, log only non-sensitive metadata (exit code,
workingDir presence) rather than full arguments. Update the call site around
ProcessHelper.execWithOutput and any helper execShellCommand usage to avoid
exposing finalCommand or sensitive envVars in logs, and apply the same change in
the Glibc launcher counterpart to keep behavior consistent across launchers.

}

public void restartWineServer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,27 +331,8 @@ public String execShellCommand(String command, boolean includeStderr) {
String finalCommand = box64Path + " " + command;

// Execute the command and capture its output
try {
Log.d("GlibcProgramLauncherComponent", "Shell command is " + finalCommand);
java.lang.Process process = Runtime.getRuntime().exec(finalCommand, envVars.toStringArray(), workingDir != null ? workingDir : imageFs.getRootDir());
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
BufferedReader errorReader = new BufferedReader(new InputStreamReader(process.getErrorStream()));

String line;
while ((line = reader.readLine()) != null) {
output.append(line).append("\n");
}
if (includeStderr) {
while ((line = errorReader.readLine()) != null) {
output.append(line).append("\n");
}
}
process.waitFor();
} catch (Exception e) {
output.append("Error: ").append(e.getMessage());
}

// Format output: trim trailing whitespace/newlines
return output.toString().trim();
Log.d("GlibcProgramLauncherComponent", "Shell command is " + finalCommand);
return ProcessHelper.execWithOutput(finalCommand, envVars.toStringArray(),
workingDir != null ? workingDir : imageFs.getRootDir(), includeStderr);
}
}
Loading