Skip to content

Commit

Permalink
Fixed: Resolve usb fd not sent due to buffering changes in 3c1a6be
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 termux#643

2. We queue the fds to be sent (using setFileDescriptorsForSend) and
   send and flush the output message in one operation to ensure the
   correct ordering

Explanation per @agnostic-apollo:

Previously, in UsbApi, the fd was only temporarily stored in WithAncillaryFd,
but not actually sent or passed to LocalSocket, and then a @ was written,
possibly in attempts to send it, even though like I said, it wasn't passed to
LocalSocket with setFileDescriptorsForSend() yet, so it was a pointless write,
but worked due to autoflush on close() (check below). After this, when
writeResult() returned, then fd was passed to LocalSocket, but still not sent,
since no data was written after it.

Previously, before 3c1a6be, the PrintWriter in ResultReturner was using java
try-with-resources, which automatically closes when try gets out of scope.
Additionally, when new PrintWriter(outputSocket.getOutputStream() is called, it
uses a BufferedWriter internally, which when closed, automatically flushes. What
this would do is actually send the socket that was previously passed to
LocalSocket. Moreover, PrintWriter was also closed before fd was closed since
try-with-resources finished before, but after the commit, it was closed
afterwards, causing your issue.

See termux#644
  • Loading branch information
dougvj authored and agnostic-apollo committed Mar 19, 2024
1 parent a9abc96 commit 418e977
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 18 deletions.
3 changes: 1 addition & 2 deletions app/src/main/java/com/termux/api/apis/UsbAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public void writeResult(PrintWriter out) {
if (result < 0) {
out.append("Failed to open device\n");
} else {
this.setFd(result);
out.append("@"); // has to be non-empty
this.sendFd(result, out);
}
} else out.append("No permission\n");
}
Expand Down
50 changes: 34 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,38 @@ 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, PrintWriter out) {
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);
// Per the docs, the file descriptors are sent with the next write
// of normal data. See https://developer.android.com/reference/android/net/LocalSocket#setFileDescriptorsForSend(java.io.FileDescriptor[])
out.print("@");
// Make sure we send the file descriptors before clearing
// them by flushing the data ('@')
out.flush();
outputSocket.setFileDescriptorsForSend(null);
}

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 +176,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 +185,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 +204,11 @@ 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) {
((WithAncillaryFd) resultWriter).cleanupFds();
}
}

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

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

0 comments on commit 418e977

Please sign in to comment.