Update implementation to prevent NPE

This commit is contained in:
topjohnwu 2024-06-23 18:20:26 -07:00
parent fe296c8ac2
commit 1698dbffef
8 changed files with 119 additions and 123 deletions

View File

@ -127,7 +127,6 @@ public abstract class Shell implements Closeable {
@NonNull
public static Executor EXECUTOR = Executors.newCachedThreadPool();
/**
* Set to {@code true} to enable verbose logging throughout the library.
*/
@ -284,6 +283,13 @@ public abstract class Shell implements Closeable {
*/
public abstract void execTask(@NonNull Task task) throws IOException;
/**
* Submits a low-level {@link Task} for execution in a queue of the shell.
* @param task the desired task.
* @see #execTask(Task)
*/
public abstract void submitTask(@NonNull Task task);
/**
* Construct a new {@link Job} that uses the shell for execution.
* <p>
@ -658,7 +664,7 @@ public abstract class Shell implements Closeable {
public interface Task {
/**
* This method will be called when a task is executed by a shell.
* Calling {@link Closeable#close()} on all streams is NOP (does nothing).
* Calling {@link Closeable#close()} on any stream is NOP (does nothing).
* @param stdin the STDIN of the shell.
* @param stdout the STDOUT of the shell.
* @param stderr the STDERR of the shell.
@ -667,6 +673,11 @@ public abstract class Shell implements Closeable {
void run(@NonNull OutputStream stdin,
@NonNull InputStream stdout,
@NonNull InputStream stderr) throws IOException;
/**
* This method will be called when a shell is unable to execute this task.
*/
default void shellDied() {}
}
/**

View File

@ -20,13 +20,13 @@ import static com.topjohnwu.superuser.Shell.EXECUTOR;
import static java.nio.charset.StandardCharsets.UTF_8;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.topjohnwu.superuser.Shell;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.io.InterruptedIOException;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Collections;
@ -36,7 +36,7 @@ import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.FutureTask;
abstract class JobTask extends Shell.Job implements Shell.Task, Closeable {
abstract class JobTask extends Shell.Job implements Shell.Task {
static final String END_UUID = UUID.randomUUID().toString();
static final int UUID_LEN = 36;
@ -50,62 +50,72 @@ abstract class JobTask extends Shell.Job implements Shell.Task, Closeable {
boolean redirect;
protected List<String> out;
protected List<String> err = UNSET_ERR;
protected Executor callbackExecutor;
protected Shell.ResultCallback callback;
@Nullable protected List<String> out;
@Nullable protected List<String> err = UNSET_ERR;
@Nullable protected Executor callbackExecutor;
@Nullable protected Shell.ResultCallback callback;
private void setResult(@NonNull ResultImpl result) {
if (callback != null) {
if (callbackExecutor == null)
callback.onResult(result);
else
callbackExecutor.execute(() -> callback.onResult(result));
}
}
private void close() {
for (ShellInputSource src : sources)
src.close();
}
@Override
public void run(@NonNull OutputStream stdin,
@NonNull InputStream stdout,
@NonNull InputStream stderr) {
boolean noErr = err == UNSET_ERR;
ResultImpl result = new ResultImpl();
result.out = out;
result.err = noErr ? null : err;
if (noErr && redirect)
result.err = out;
List<String> outList = out;
List<String> errList = noErr ? (redirect ? out : null) : err;
if (result.out != null && result.out == result.err && !Utils.isSynchronized(result.out)) {
if (outList != null && outList == errList && !Utils.isSynchronized(outList)) {
// Synchronize the list internally only if both lists are the same and are not
// already synchronized by the user
List<String> list = Collections.synchronizedList(result.out);
result.out = list;
result.err = list;
List<String> list = Collections.synchronizedList(outList);
outList = list;
errList = list;
}
FutureTask<Integer> outGobbler =
new FutureTask<>(new StreamGobbler.OUT(stdout, result.out));
FutureTask<Void> errGobbler = new FutureTask<>(new StreamGobbler.ERR(stderr, result.err));
FutureTask<Integer> outGobbler = new FutureTask<>(new StreamGobbler.OUT(stdout, outList));
FutureTask<Void> errGobbler = new FutureTask<>(new StreamGobbler.ERR(stderr, errList));
EXECUTOR.execute(outGobbler);
EXECUTOR.execute(errGobbler);
ResultImpl result = new ResultImpl();
try {
for (ShellInputSource src : sources)
src.serve(stdin);
stdin.write(END_CMD);
stdin.flush();
try {
result.code = outGobbler.get();
errGobbler.get();
result.out = out;
result.err = noErr ? null : err;
} catch (ExecutionException | InterruptedException e) {
throw (InterruptedIOException) new InterruptedIOException().initCause(e);
}
} catch (IOException e) {
if (e instanceof ShellTerminatedException) {
result = ResultImpl.SHELL_ERR;
} else {
Utils.err(e);
result = ResultImpl.INSTANCE;
}
} finally {
close();
result.callback(callbackExecutor, callback);
int code = outGobbler.get();
errGobbler.get();
result.code = code;
result.out = out;
result.err = noErr ? null : err;
} catch (IOException | ExecutionException | InterruptedException e) {
Utils.err(e);
}
close();
setResult(new ResultImpl());
}
@Override
public void shellDied() {
close();
setResult(new ResultImpl());
}
@NonNull
@ -138,10 +148,4 @@ abstract class JobTask extends Shell.Job implements Shell.Task, Closeable {
sources.add(new CommandSource(cmds));
return this;
}
@Override
public void close() {
for (ShellInputSource src : sources)
src.close();
}
}

View File

@ -29,60 +29,49 @@ import java.util.concurrent.Future;
class PendingJob extends JobTask {
@Nullable
private Runnable retryTask;
PendingJob() {
to(NOPList.getInstance());
}
private Shell.Result exec0(ResultHolder h) {
@Override
public void shellDied() {
if (retryTask != null) {
Runnable r = retryTask;
retryTask = null;
r.run();
} else {
super.shellDied();
}
}
private void exec0() {
ShellImpl shell;
try {
shell = MainShell.get();
} catch (NoShellException e) {
close();
return ResultImpl.INSTANCE;
super.shellDied();
return;
}
try {
shell.execTask(this);
} catch (IOException ignored) { /* JobTask does not throw */ }
return h.result;
}
@NonNull
@Override
public Shell.Result exec() {
ResultHolder h = new ResultHolder();
callback = h;
retryTask = this::exec0;
ResultHolder holder = new ResultHolder();
callback = holder;
callbackExecutor = null;
if (out instanceof NOPList)
out = new ArrayList<>();
Shell.Result r = exec0(h);
if (r == ResultImpl.SHELL_ERR) {
// The cached shell is terminated, try to re-run this task
return exec0(h);
}
return r;
}
private class RetryCallback implements Shell.ResultCallback {
private final Shell.ResultCallback base;
private boolean retry = true;
RetryCallback(Shell.ResultCallback b) {
base = b;
}
@Override
public void onResult(@NonNull Shell.Result out) {
if (retry && out == ResultImpl.SHELL_ERR) {
// The cached shell is terminated, try to re-schedule this task
retry = false;
submit0();
} else if (base != null) {
base.onResult(out);
}
}
exec0();
return holder.getResult();
}
private void submit0() {
@ -95,19 +84,21 @@ class PendingJob extends JobTask {
@NonNull
@Override
public Future<Shell.Result> enqueue() {
ResultFuture f = new ResultFuture();
callback = new RetryCallback(f);
retryTask = this::submit0;
ResultFuture future = new ResultFuture();
callback = future;
callbackExecutor = null;
if (out instanceof NOPList)
out = new ArrayList<>();
submit0();
return f;
return future;
}
@Override
public void submit(@Nullable Executor executor, @Nullable Shell.ResultCallback cb) {
retryTask = this::submit0;
callbackExecutor = executor;
callback = new RetryCallback(cb);
callback = cb;
if (out instanceof NOPList)
out = (cb == null) ? null : new ArrayList<>();
submit0();

View File

@ -25,14 +25,13 @@ import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
class ResultFuture implements Shell.ResultCallback, Future<Shell.Result> {
class ResultFuture extends ResultHolder implements Future<Shell.Result> {
Shell.Result result;
private CountDownLatch latch = new CountDownLatch(1);
private final CountDownLatch latch = new CountDownLatch(1);
@Override
public void onResult(@NonNull Shell.Result out) {
result = out;
super.onResult(out);
latch.countDown();
}
@ -54,7 +53,7 @@ class ResultFuture implements Shell.ResultCallback, Future<Shell.Result> {
@Override
public Shell.Result get() throws InterruptedException {
latch.await();
return result;
return getResult();
}
@Override
@ -63,6 +62,6 @@ class ResultFuture implements Shell.ResultCallback, Future<Shell.Result> {
if (!latch.await(timeout, unit)) {
throw new TimeoutException();
}
return result;
return getResult();
}
}

View File

@ -17,15 +17,22 @@
package com.topjohnwu.superuser.internal;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.topjohnwu.superuser.Shell;
class ResultHolder implements Shell.ResultCallback {
Shell.Result result;
@Nullable
private Shell.Result result;
@Override
public void onResult(@NonNull Shell.Result out) {
result = out;
}
@NonNull
Shell.Result getResult() {
return result == null ? new ResultImpl() : result;
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2023 John "topjohnwu" Wu
* Copyright 2024 John "topjohnwu" Wu
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -22,14 +22,11 @@ import com.topjohnwu.superuser.Shell;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.Executor;
class ResultImpl extends Shell.Result {
List<String> out;
List<String> err;
int code = JOB_NOT_EXECUTED;
static ResultImpl INSTANCE = new ResultImpl();
static ResultImpl SHELL_ERR = new ResultImpl();
@NonNull
@Override
@ -47,13 +44,4 @@ class ResultImpl extends Shell.Result {
public int getCode() {
return code;
}
void callback(Executor executor, Shell.ResultCallback cb) {
if (cb != null) {
if (executor == null)
cb.onResult(this);
else
executor.execute(() -> cb.onResult(this));
}
}
}

View File

@ -39,13 +39,6 @@ import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
class ShellTerminatedException extends IOException {
ShellTerminatedException() {
super("Shell terminated unexpectedly");
}
}
class ShellImpl extends Shell {
private volatile int status;
@ -216,9 +209,11 @@ class ShellImpl extends Shell {
}
}
private synchronized void exec0(Task task) throws IOException {
if (status < 0)
throw new ShellTerminatedException();
private synchronized void exec0(@NonNull Task task) throws IOException {
if (status < 0) {
task.shellDied();
return;
}
ShellUtils.cleanInputStream(STDOUT);
ShellUtils.cleanInputStream(STDERR);
@ -226,9 +221,9 @@ class ShellImpl extends Shell {
STDIN.write('\n');
STDIN.flush();
} catch (IOException e) {
// Shell is dead
release();
throw new ShellTerminatedException();
task.shellDied();
return;
}
if (task instanceof JobTask) {
@ -239,8 +234,8 @@ class ShellImpl extends Shell {
}
private void processTasks() {
Task task;
for (;;) {
Task task;
synchronized (tasks) {
if ((task = tasks.poll()) == null) {
runningTasks = false;
@ -254,7 +249,8 @@ class ShellImpl extends Shell {
}
}
void submitTask(Task task) {
@Override
public void submitTask(@NonNull Task task) {
synchronized (tasks) {
tasks.offer(task);
if (!runningTasks) {
@ -282,5 +278,4 @@ class ShellImpl extends Shell {
public Job newJob() {
return new ShellJob(this);
}
}

View File

@ -27,22 +27,23 @@ import java.util.concurrent.Future;
class ShellJob extends JobTask {
@NonNull
private final ShellImpl shell;
ShellJob(ShellImpl s) {
ShellJob(@NonNull ShellImpl s) {
shell = s;
}
@NonNull
@Override
public Shell.Result exec() {
ResultHolder h = new ResultHolder();
callback = h;
ResultHolder holder = new ResultHolder();
callback = holder;
callbackExecutor = null;
try {
shell.execTask(this);
} catch (IOException ignored) { /* JobTask does not throw */ }
return h.result;
return holder.getResult();
}
@Override
@ -55,10 +56,10 @@ class ShellJob extends JobTask {
@NonNull
@Override
public Future<Shell.Result> enqueue() {
ResultFuture f = new ResultFuture();
callback = f;
ResultFuture future = new ResultFuture();
callback = future;
callbackExecutor = null;
shell.submitTask(this);
return f;
return future;
}
}