fastbootd: Fix transport ownership.

This change moves Transport ownership back out of FastBootDriver.
Callers of set_transport must ensure that the previous transport is
destroyed. In addition, deleting a transport now ensures that it is
closed.

Bug: 78793464
Test: fastboot, fuzzy_fastboot works
Change-Id: I8f9ed2f7d5b09fd0820b2677d087a027378f26db

fastboot/engine.cpp

@@ -88,7 +88,9 @@ void fb_init(fastboot::FastBootDriver& fbi) {
 }
 
 void fb_reinit(Transport* transport) {
-    fb->set_transport(transport);
+    if (Transport* old_transport = fb->set_transport(transport)) {
+        delete old_transport;
+    }
 }
 
 const std::string fb_get_error() {
@@ -392,6 +394,6 @@ bool fb_reboot_to_userspace() {
     }
     fprintf(stderr, "OKAY\n");
 
-    fb->set_transport(nullptr);
+    fb_reinit(nullptr);
     return true;
 }

fastboot/fastboot.cpp

@@ -1847,6 +1847,10 @@ int FastBootTool::Main(int argc, char* argv[]) {
 
     int status = fb_execute_queue() ? EXIT_FAILURE : EXIT_SUCCESS;
     fprintf(stderr, "Finished. Total time: %.3fs\n", (now() - start));
+
+    if (Transport* old_transport = fb.set_transport(nullptr)) {
+        delete old_transport;
+    }
     return status;
 }
 

fastboot/fastboot_driver.cpp

@@ -58,7 +58,6 @@ FastBootDriver::FastBootDriver(Transport* transport, std::function<void(std::str
 }
 
 FastBootDriver::~FastBootDriver() {
-    set_transport(nullptr);
 }
 
 RetCode FastBootDriver::Boot(std::string* response, std::vector<std::string>* info) {
@@ -537,12 +536,9 @@ int FastBootDriver::SparseWriteCallback(std::vector<char>& tpbuf, const char* da
     return 0;
 }
 
-void FastBootDriver::set_transport(Transport* transport) {
-    if (transport_) {
-        transport_->Close();
-        delete transport_;
-    }
-    transport_ = transport;
+Transport* FastBootDriver::set_transport(Transport* transport) {
+    std::swap(transport_, transport);
+    return transport;
 }
 
 }  // End namespace fastboot

fastboot/fastboot_driver.h

@@ -109,8 +109,8 @@ class FastBootDriver {
     std::string Error();
     RetCode WaitForDisconnect();
 
-    // Note: changing the transport will close and delete the existing one.
-    void set_transport(Transport* transport);
+    // Note: set_transport will return the previous transport.
+    Transport* set_transport(Transport* transport);
     Transport* transport() const { return transport_; }
 
     // This is temporarily public for engine.cpp

fastboot/fuzzy_fastboot/fixtures.cpp

@@ -133,7 +133,6 @@ void FastBootTest::TearDown() {
     fb.reset();
 
     if (transport) {
-        transport->Close();
         transport.reset();
     }
 
@@ -188,7 +187,6 @@ void FastBootTest::SetLockState(bool unlock, bool assert_change) {
         ASSERT_EQ(fb->RawCommand("flashing " + cmd, &resp), SUCCESS)
                 << "Attempting to change locked state, but 'flashing" + cmd + "' command failed";
         fb.reset();
-        transport->Close();
         transport.reset();
         printf("PLEASE RESPOND TO PROMPT FOR '%sing' BOOTLOADER ON DEVICE\n", cmd.c_str());
         while (UsbStillAvailible())
@@ -249,7 +247,6 @@ void Fuzz::TearDown() {
     }
 
     if (transport) {
-        transport->Close();
         transport.reset();
     }
 

fastboot/fuzzy_fastboot/usb_transport_sniffer.cpp

@@ -12,6 +12,10 @@ UsbTransportSniffer::UsbTransportSniffer(std::unique_ptr<UsbTransport> transport
                                          const int serial_fd)
     : transport_(std::move(transport)), serial_fd_(serial_fd) {}
 
+UsbTransportSniffer::~UsbTransportSniffer() {
+    Close();
+}
+
 ssize_t UsbTransportSniffer::Read(void* data, size_t len) {
     ProcessSerial();
 

fastboot/fuzzy_fastboot/usb_transport_sniffer.h

@@ -68,6 +68,7 @@ class UsbTransportSniffer : public UsbTransport {
     };
 
     UsbTransportSniffer(std::unique_ptr<UsbTransport> transport, const int serial_fd = 0);
+    ~UsbTransportSniffer() override;
 
     virtual ssize_t Read(void* data, size_t len) override;
     virtual ssize_t Write(const void* data, size_t len) override;

fastboot/usb_linux.cpp

@@ -95,7 +95,7 @@ class LinuxUsbTransport : public UsbTransport {
   public:
     explicit LinuxUsbTransport(std::unique_ptr<usb_handle> handle, uint32_t ms_timeout = 0)
         : handle_(std::move(handle)), ms_timeout_(ms_timeout) {}
-    ~LinuxUsbTransport() override = default;
+    ~LinuxUsbTransport() override;
 
     ssize_t Read(void* data, size_t len) override;
     ssize_t Write(const void* data, size_t len) override;
@@ -387,6 +387,10 @@ static std::unique_ptr<usb_handle> find_usb_device(const char* base, ifc_match_f
     return usb;
 }
 
+LinuxUsbTransport::~LinuxUsbTransport() {
+    Close();
+}
+
 ssize_t LinuxUsbTransport::Write(const void* _data, size_t len)
 {
     unsigned char *data = (unsigned char*) _data;

fastboot/usb_osx.cpp

@@ -70,7 +70,7 @@ class OsxUsbTransport : public UsbTransport {
     // A timeout of 0 is blocking
     OsxUsbTransport(std::unique_ptr<usb_handle> handle, uint32_t ms_timeout = 0)
         : handle_(std::move(handle)), ms_timeout_(ms_timeout) {}
-    ~OsxUsbTransport() override = default;
+    ~OsxUsbTransport() override;
 
     ssize_t Read(void* data, size_t len) override;
     ssize_t Write(const void* data, size_t len) override;
@@ -471,6 +471,10 @@ UsbTransport* usb_open(ifc_match_func callback, uint32_t timeout_ms) {
     return new OsxUsbTransport(std::move(handle), timeout_ms);
 }
 
+OsxUsbTransport::~OsxUsbTransport() {
+    Close();
+}
+
 int OsxUsbTransport::Close() {
     /* TODO: Something better here? */
     return 0;

fastboot/usb_windows.cpp

@@ -69,7 +69,7 @@ struct usb_handle {
 class WindowsUsbTransport : public UsbTransport {
   public:
     WindowsUsbTransport(std::unique_ptr<usb_handle> handle) : handle_(std::move(handle)) {}
-    ~WindowsUsbTransport() override = default;
+    ~WindowsUsbTransport() override;
 
     ssize_t Read(void* data, size_t len) override;
     ssize_t Write(const void* data, size_t len) override;
@@ -250,6 +250,10 @@ void usb_kick(usb_handle* handle) {
     }
 }
 
+WindowsUsbTransport::~WindowsUsbTransport() {
+    Close();
+}
+
 int WindowsUsbTransport::Close() {
     DBG("usb_close\n");