Skip to content

Commit

Permalink
termux-usb: fix issue with usb fd being closed before sent over socket
Browse files Browse the repository at this point in the history
This is a minor refactor of the WithAncillaryFd ResultReturner that
achieves two goals:

1. The usb fd is no longer closed before the message is sent over the
   socket. This resolves #643

2. We queue the fds to be sent (using setFileDescriptorsForSend) Before
   printing an output message to ensure the order is correct. This was
   correct before by virtue of the buffering mechanism, but would break
   if there were no buffering.
  • Loading branch information
dougvj committed Nov 29, 2023
1 parent b732327 commit 5e23b09
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 17 deletions.
2 changes: 1 addition & 1 deletion app/src/main/java/com/termux/api/apis/UsbAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void writeResult(PrintWriter out) {
if (result < 0) {
out.append("Failed to open device\n");
} else {
this.setFd(result);
this.sendFd(result);
out.append("@"); // has to be non-empty
}
} else out.append("No permission\n");
Expand Down
48 changes: 32 additions & 16 deletions app/src/main/java/com/termux/api/util/ResultReturner.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,31 @@ public final void setInput(InputStream inputStream) throws Exception {
}

public static abstract class WithAncillaryFd implements ResultWriter {
private int fd = -1;
private LocalSocket outputSocket = null;
private final ParcelFileDescriptor[] pfds = { null };

public final void setFd(int newFd) {
fd = newFd;
public final void setOutputSocketForFds(LocalSocket outputSocket) {
this.outputSocket = outputSocket;
}

public final int getFd() {
return fd;
public final void sendFd(int fd) {
if (this.pfds[0] != null) {
Logger.logStackTraceWithMessage(LOG_TAG, "File descriptor already sent! We currently only support sending one per result", new Exception());
return;
}
this.pfds[0] = ParcelFileDescriptor.adoptFd(fd);
FileDescriptor[] fds = { pfds[0].getFileDescriptor() };
outputSocket.setFileDescriptorsForSend(fds);
}

public final void cleanupFds() {
if (this.pfds[0] != null) {
try {
this.pfds[0].close();
} catch (IOException e) {
Logger.logStackTraceWithMessage(LOG_TAG, "Failed to close file descriptor", e);
}
}
}
}

Expand Down Expand Up @@ -152,7 +169,6 @@ public static void returnData(Object context, final Intent intent, final ResultW
PrintWriter writer = null;
LocalSocket outputSocket = null;
try {
final ParcelFileDescriptor[] pfds = { null };
outputSocket = new LocalSocket();
String outputSocketAdress = intent.getStringExtra(SOCKET_OUTPUT_EXTRA);
if (outputSocketAdress == null || outputSocketAdress.isEmpty())
Expand All @@ -162,6 +178,9 @@ public static void returnData(Object context, final Intent intent, final ResultW
writer = new PrintWriter(outputSocket.getOutputStream());

if (resultWriter != null) {
if(resultWriter instanceof WithAncillaryFd) {
((WithAncillaryFd) resultWriter).setOutputSocketForFds(outputSocket);
}
if (resultWriter instanceof BinaryOutput) {
BinaryOutput bout = (BinaryOutput) resultWriter;
bout.setOutput(outputSocket.getOutputStream());
Expand All @@ -178,19 +197,16 @@ public static void returnData(Object context, final Intent intent, final ResultW
} else {
resultWriter.writeResult(writer);
}
if(resultWriter instanceof WithAncillaryFd) {
int fd = ((WithAncillaryFd) resultWriter).getFd();
if (fd >= 0) {
pfds[0] = ParcelFileDescriptor.adoptFd(fd);
FileDescriptor[] fds = { pfds[0].getFileDescriptor() };
outputSocket.setFileDescriptorsForSend(fds);
}
if (resultWriter instanceof WithAncillaryFd) {
// Make sure we send the file descriptors before
// closing them, otherwise we mysterious socket
// issues.
writer.flush();
outputSocket.getOutputStream().flush();
((WithAncillaryFd) resultWriter).cleanupFds();
}
}

if(pfds[0] != null) {
pfds[0].close();
}

if (asyncResult != null) {
asyncResult.setResultCode(0);
Expand Down

0 comments on commit 5e23b09

Please sign in to comment.