Merge "Use a single otapreopt_chroot invocation for all otapreopt runs." into main
diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp
index c86993c..c40caf5 100644
--- a/cmds/installd/otapreopt_chroot.cpp
+++ b/cmds/installd/otapreopt_chroot.cpp
@@ -19,9 +19,12 @@
 #include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <unistd.h>
 
+#include <algorithm>
 #include <array>
 #include <fstream>
+#include <iostream>
 #include <sstream>
 
 #include <android-base/file.h>
@@ -29,6 +32,7 @@
 #include <android-base/macros.h>
 #include <android-base/scopeguard.h>
 #include <android-base/stringprintf.h>
+#include <android-base/strings.h>
 #include <android-base/unique_fd.h>
 #include <libdm/dm.h>
 #include <selinux/android.h>
@@ -37,7 +41,7 @@
 #include "otapreopt_utils.h"
 
 #ifndef LOG_TAG
-#define LOG_TAG "otapreopt"
+#define LOG_TAG "otapreopt_chroot"
 #endif
 
 using android::base::StringPrintf;
@@ -49,20 +53,22 @@
 // so just try the possibilities one by one.
 static constexpr std::array kTryMountFsTypes = {"ext4", "erofs"};
 
-static void CloseDescriptor(int fd) {
-    if (fd >= 0) {
-        int result = close(fd);
-        UNUSED(result);  // Ignore result. Printing to logcat will open a new descriptor
-                         // that we do *not* want.
-    }
-}
-
 static void CloseDescriptor(const char* descriptor_string) {
     int fd = -1;
     std::istringstream stream(descriptor_string);
     stream >> fd;
     if (!stream.fail()) {
-        CloseDescriptor(fd);
+        if (fd >= 0) {
+            if (close(fd) < 0) {
+                PLOG(ERROR) << "Failed to close " << fd;
+            }
+        }
+    }
+}
+
+static void SetCloseOnExec(int fd) {
+    if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) {
+        PLOG(ERROR) << "Failed to set FD_CLOEXEC on " << fd;
     }
 }
 
@@ -129,24 +135,39 @@
 }
 
 // Entry for otapreopt_chroot. Expected parameters are:
-//   [cmd] [status-fd] [target-slot] "dexopt" [dexopt-params]
-// The file descriptor denoted by status-fd will be closed. The rest of the parameters will
-// be passed on to otapreopt in the chroot.
+//
+//   [cmd] [status-fd] [target-slot-suffix]
+//
+// The file descriptor denoted by status-fd will be closed. Dexopt commands on
+// the form
+//
+//   "dexopt" [dexopt-params]
+//
+// are then read from stdin until EOF and passed on to /system/bin/otapreopt one
+// by one. After each call a line with the current command count is written to
+// stdout and flushed.
 static int otapreopt_chroot(const int argc, char **arg) {
     // Validate arguments
-    // We need the command, status channel and target slot, at a minimum.
-    if(argc < 3) {
-        PLOG(ERROR) << "Not enough arguments.";
+    if (argc == 2 && std::string_view(arg[1]) == "--version") {
+        // Accept a single --version flag, to allow the script to tell this binary
+        // from the earlier one.
+        std::cout << "2" << std::endl;
+        return 0;
+    }
+    if (argc != 3) {
+        LOG(ERROR) << "Wrong number of arguments: " << argc;
         exit(208);
     }
-    // Close all file descriptors. They are coming from the caller, we do not want to pass them
-    // on across our fork/exec into a different domain.
-    // 1) Default descriptors.
-    CloseDescriptor(STDIN_FILENO);
-    CloseDescriptor(STDOUT_FILENO);
-    CloseDescriptor(STDERR_FILENO);
-    // 2) The status channel.
-    CloseDescriptor(arg[1]);
+    const char* status_fd = arg[1];
+    const char* slot_suffix = arg[2];
+
+    // Set O_CLOEXEC on standard fds. They are coming from the caller, we do not
+    // want to pass them on across our fork/exec into a different domain.
+    SetCloseOnExec(STDIN_FILENO);
+    SetCloseOnExec(STDOUT_FILENO);
+    SetCloseOnExec(STDERR_FILENO);
+    // Close the status channel.
+    CloseDescriptor(status_fd);
 
     // We need to run the otapreopt tool from the postinstall partition. As such, set up a
     // mount namespace and change root.
@@ -185,20 +206,20 @@
     //  2) We're in a mount namespace here, so when we die, this will be cleaned up.
     //  3) Ignore errors. Printing anything at this stage will open a file descriptor
     //     for logging.
-    if (!ValidateTargetSlotSuffix(arg[2])) {
-        LOG(ERROR) << "Target slot suffix not legal: " << arg[2];
+    if (!ValidateTargetSlotSuffix(slot_suffix)) {
+        LOG(ERROR) << "Target slot suffix not legal: " << slot_suffix;
         exit(207);
     }
-    TryExtraMount("vendor", arg[2], "/postinstall/vendor");
+    TryExtraMount("vendor", slot_suffix, "/postinstall/vendor");
 
     // Try to mount the product partition. update_engine doesn't do this for us, but we
     // want it for product APKs. Same notes as vendor above.
-    TryExtraMount("product", arg[2], "/postinstall/product");
+    TryExtraMount("product", slot_suffix, "/postinstall/product");
 
     // Try to mount the system_ext partition. update_engine doesn't do this for
     // us, but we want it for system_ext APKs. Same notes as vendor and product
     // above.
-    TryExtraMount("system_ext", arg[2], "/postinstall/system_ext");
+    TryExtraMount("system_ext", slot_suffix, "/postinstall/system_ext");
 
     constexpr const char* kPostInstallLinkerconfig = "/postinstall/linkerconfig";
     // Try to mount /postinstall/linkerconfig. we will set it up after performing the chroot
@@ -329,30 +350,37 @@
         exit(218);
     }
 
-    // Now go on and run otapreopt.
+    // Now go on and read dexopt lines from stdin and pass them on to otapreopt.
 
-    // Incoming:  cmd + status-fd + target-slot + cmd...      | Incoming | = argc
-    // Outgoing:  cmd             + target-slot + cmd...      | Outgoing | = argc - 1
-    std::vector<std::string> cmd;
-    cmd.reserve(argc);
-    cmd.push_back("/system/bin/otapreopt");
+    int count = 1;
+    for (std::array<char, 1000> linebuf;
+         std::cin.clear(), std::cin.getline(&linebuf[0], linebuf.size()); ++count) {
+        // Subtract one from gcount() since getline() counts the newline.
+        std::string line(&linebuf[0], std::cin.gcount() - 1);
 
-    // The first parameter is the status file descriptor, skip.
-    for (size_t i = 2; i < static_cast<size_t>(argc); ++i) {
-        cmd.push_back(arg[i]);
+        if (std::cin.fail()) {
+            LOG(ERROR) << "Command exceeds max length " << linebuf.size() << " - skipped: " << line;
+            continue;
+        }
+
+        std::vector<std::string> tokenized_line = android::base::Tokenize(line, " ");
+        std::vector<std::string> cmd{"/system/bin/otapreopt", slot_suffix};
+        std::move(tokenized_line.begin(), tokenized_line.end(), std::back_inserter(cmd));
+
+        LOG(INFO) << "Command " << count << ": " << android::base::Join(cmd, " ");
+
+        // Fork and execute otapreopt in its own process.
+        std::string error_msg;
+        bool exec_result = Exec(cmd, &error_msg);
+        if (!exec_result) {
+            LOG(ERROR) << "Running otapreopt failed: " << error_msg;
+        }
+
+        // Print the count to stdout and flush to indicate progress.
+        std::cout << count << std::endl;
     }
 
-    // Fork and execute otapreopt in its own process.
-    std::string error_msg;
-    bool exec_result = Exec(cmd, &error_msg);
-    if (!exec_result) {
-        LOG(ERROR) << "Running otapreopt failed: " << error_msg;
-    }
-
-    if (!exec_result) {
-        exit(213);
-    }
-
+    LOG(INFO) << "No more dexopt commands";
     return 0;
 }
 
diff --git a/cmds/installd/otapreopt_script.sh b/cmds/installd/otapreopt_script.sh
index e483d54..28bd793 100644
--- a/cmds/installd/otapreopt_script.sh
+++ b/cmds/installd/otapreopt_script.sh
@@ -16,7 +16,9 @@
 # limitations under the License.
 #
 
-# This script will run as a postinstall step to drive otapreopt.
+# This script runs as a postinstall step to drive otapreopt. It comes with the
+# OTA package, but runs /system/bin/otapreopt_chroot in the (old) active system
+# image. See system/extras/postinst/postinst.sh for some docs.
 
 TARGET_SLOT="$1"
 STATUS_FD="$2"
@@ -31,12 +33,11 @@
 
 BOOT_COMPLETE=$(getprop $BOOT_PROPERTY_NAME)
 if [ "$BOOT_COMPLETE" != "1" ] ; then
-  echo "Error: boot-complete not detected."
+  echo "$0: Error: boot-complete not detected."
   # We must return 0 to not block sideload.
   exit 0
 fi
 
-
 # Compute target slot suffix.
 # TODO: Once bootctl is not restricted, we should query from there. Or get this from
 #       update_engine as a parameter.
@@ -45,44 +46,63 @@
 elif [ "$TARGET_SLOT" = "1" ] ; then
   TARGET_SLOT_SUFFIX="_b"
 else
-  echo "Unknown target slot $TARGET_SLOT"
+  echo "$0: Unknown target slot $TARGET_SLOT"
   exit 1
 fi
 
+if [ "$(/system/bin/otapreopt_chroot --version)" != 2 ]; then
+  # We require an updated chroot wrapper that reads dexopt commands from stdin.
+  # Even if we kept compat with the old binary, the OTA preopt wouldn't work due
+  # to missing sepolicy rules, so there's no use spending time trying to dexopt
+  # (b/291974157).
+  echo "$0: Current system image is too old to work with OTA preopt - skipping."
+  exit 0
+fi
 
 PREPARE=$(cmd otadexopt prepare)
 # Note: Ignore preparation failures. Step and done will fail and exit this.
 #       This is necessary to support suspends - the OTA service will keep
 #       the state around for us.
 
-PROGRESS=$(cmd otadexopt progress)
-print -u${STATUS_FD} "global_progress $PROGRESS"
-
-i=0
-while ((i<MAXIMUM_PACKAGES)) ; do
+# Create an array with all dexopt commands in advance, to know how many there are.
+otadexopt_cmds=()
+while (( ${#otadexopt_cmds[@]} < MAXIMUM_PACKAGES )) ; do
   DONE=$(cmd otadexopt done)
   if [ "$DONE" = "OTA complete." ] ; then
     break
   fi
-
-  DEXOPT_PARAMS=$(cmd otadexopt next)
-
-  /system/bin/otapreopt_chroot $STATUS_FD $TARGET_SLOT_SUFFIX $DEXOPT_PARAMS >&- 2>&-
-
-  PROGRESS=$(cmd otadexopt progress)
-  print -u${STATUS_FD} "global_progress $PROGRESS"
-
-  i=$((i+1))
+  otadexopt_cmds+=("$(cmd otadexopt next)")
 done
 
 DONE=$(cmd otadexopt done)
+cmd otadexopt cleanup
+
+echo "$0: Using streaming otapreopt_chroot on ${#otadexopt_cmds[@]} packages"
+
+function print_otadexopt_cmds {
+  for cmd in "${otadexopt_cmds[@]}" ; do
+    print "$cmd"
+  done
+}
+
+function report_progress {
+  while read count ; do
+    # mksh can't do floating point arithmetic, so emulate a fixed point calculation.
+    (( permilles = 1000 * count / ${#otadexopt_cmds[@]} ))
+    printf 'global_progress %d.%03d\n' $((permilles / 1000)) $((permilles % 1000)) >&${STATUS_FD}
+  done
+}
+
+print_otadexopt_cmds | \
+  /system/bin/otapreopt_chroot $STATUS_FD $TARGET_SLOT_SUFFIX | \
+  report_progress
+
 if [ "$DONE" = "OTA incomplete." ] ; then
-  echo "Incomplete."
+  echo "$0: Incomplete."
 else
-  echo "Complete or error."
+  echo "$0: Complete or error."
 fi
 
 print -u${STATUS_FD} "global_progress 1.0"
-cmd otadexopt cleanup
 
 exit 0