Skip to content

Commit

Permalink
Missing callbacks if shell dies in Interactive mode
Browse files Browse the repository at this point in the history
libsuperuser is currently able to handle failures in several different
stages:

1) If the shell cannot be started at all (e.g. missing "su" binary),
Runtime.exec() should throw an exception, causing our open() method to
return failure.

2) If the shell dies while a command is executing, and the watchdog timer
is enabled, the onCommandResult() callback will get invoked so that the
caller can handle the error.

3) If the device operator denies a Superuser request, this typically
falls under case Chainfire#2 as the "su" executable will stay alive for a couple
of seconds until the operator clicks "deny."

But if the shell dies when there is no active command, any queued commands
will get stuck in limbo and the caller will never receive a status
indication.  This will probably cause the app to freeze until the
operator kills it, resulting in a subpar user experience.

The easiest way to reproduce the problem is to try running
libsuperuser_example in interactive mode on the SDK emulator (which has
a trivial "su" implementation that instantly denies root access to all
apps).  It will hang at "Requesting root privilege" forever.

So, we'll add an extra loop to send the bad news to the calling app if
libsuperuser isn't able to execute the requested commands.
  • Loading branch information
cernekee committed Dec 8, 2013
1 parent 9316e07 commit fcef4d2
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions libsuperuser/src/eu/chainfire/libsuperuser/Shell.java
Original file line number Diff line number Diff line change
Expand Up @@ -764,10 +764,8 @@ protected void finalize() throws Throwable {
* @param onCommandResultListener Callback to be called on completion (of all commands)
*/
public synchronized void addCommand(String[] commands, int code, OnCommandResultListener onCommandResultListener) {
if (running) {
this.commands.add(new Command(commands, code, onCommandResultListener));
runNextCommand();
}
this.commands.add(new Command(commands, code, onCommandResultListener));
runNextCommand();
}

/**
Expand Down Expand Up @@ -880,6 +878,11 @@ private void runNextCommand(boolean notifyIdle) {
} else {
runNextCommand(false);
}
} else if (!running) {
// our shell died for unknown reasons - abort all submissions
while (commands.size() > 0) {
postCallback(commands.remove(0), OnCommandResultListener.SHELL_DIED, null);
}
}

if (idle && notifyIdle) {
Expand Down

0 comments on commit fcef4d2

Please sign in to comment.