From 5e23b09f50c4f4a893bc93d35b538cb86821e582 Mon Sep 17 00:00:00 2001
From: Doug Johnson <dougvj@gmail.com>
Date: Tue, 28 Nov 2023 21:17:18 -0700
Subject: [PATCH] termux-usb: fix issue with usb fd being closed before sent
 over socket

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.
---
 .../main/java/com/termux/api/apis/UsbAPI.java |  2 +-
 .../com/termux/api/util/ResultReturner.java   | 48 ++++++++++++-------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/app/src/main/java/com/termux/api/apis/UsbAPI.java b/app/src/main/java/com/termux/api/apis/UsbAPI.java
index 667ba1793..1197619ab 100644
--- a/app/src/main/java/com/termux/api/apis/UsbAPI.java
+++ b/app/src/main/java/com/termux/api/apis/UsbAPI.java
@@ -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");
diff --git a/app/src/main/java/com/termux/api/util/ResultReturner.java b/app/src/main/java/com/termux/api/util/ResultReturner.java
index 4ae93f097..df809724b 100644
--- a/app/src/main/java/com/termux/api/util/ResultReturner.java
+++ b/app/src/main/java/com/termux/api/util/ResultReturner.java
@@ -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);
+            }
+          }
         }
     }
 
@@ -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())
@@ -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());
@@ -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);