Revert "adbd: don't close ep0 until we receive FUNCTIONFS_UNBIND."
We're only reading FUNCTIONFS_BIND on creation upon the first creation
of the control fd, which results in an abort when rebind happens (e.g.
when MTP is enabled) and we unexpectedly read FUNCTIONFS_BIND in the
second instantiation of the connection.
This reverts commit 2b668d3906a9b54d735113807c7a95a6dbdd270f.
Bug: http://b/135155652
Test: manually enabled MTP
diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp
index ac739c4..1abae87 100644
--- a/adb/daemon/usb.cpp
+++ b/adb/daemon/usb.cpp
@@ -169,12 +169,12 @@
};
struct UsbFfsConnection : public Connection {
- UsbFfsConnection(unique_fd* control, unique_fd read, unique_fd write,
+ UsbFfsConnection(unique_fd control, unique_fd read, unique_fd write,
std::promise<void> destruction_notifier)
: worker_started_(false),
stopped_(false),
destruction_notifier_(std::move(destruction_notifier)),
- control_fd_(control),
+ control_fd_(std::move(control)),
read_fd_(std::move(read)),
write_fd_(std::move(write)) {
LOG(INFO) << "UsbFfsConnection constructed";
@@ -183,6 +183,11 @@
PLOG(FATAL) << "failed to create eventfd";
}
+ monitor_event_fd_.reset(eventfd(0, EFD_CLOEXEC));
+ if (monitor_event_fd_ == -1) {
+ PLOG(FATAL) << "failed to create eventfd";
+ }
+
aio_context_ = ScopedAioContext::Create(kUsbReadQueueDepth + kUsbWriteQueueDepth);
}
@@ -194,6 +199,7 @@
// We need to explicitly close our file descriptors before we notify our destruction,
// because the thread listening on the future will immediately try to reopen the endpoint.
aio_context_.reset();
+ control_fd_.reset();
read_fd_.reset();
write_fd_.reset();
@@ -240,6 +246,13 @@
PLOG(FATAL) << "failed to notify worker eventfd to stop UsbFfsConnection";
}
CHECK_EQ(static_cast<size_t>(rc), sizeof(notify));
+
+ rc = adb_write(monitor_event_fd_.get(), ¬ify, sizeof(notify));
+ if (rc < 0) {
+ PLOG(FATAL) << "failed to notify monitor eventfd to stop UsbFfsConnection";
+ }
+
+ CHECK_EQ(static_cast<size_t>(rc), sizeof(notify));
}
private:
@@ -258,24 +271,33 @@
monitor_thread_ = std::thread([this]() {
adb_thread_setname("UsbFfs-monitor");
+ bool bound = false;
bool enabled = false;
bool running = true;
while (running) {
adb_pollfd pfd[2] = {
- {.fd = control_fd_->get(), .events = POLLIN, .revents = 0},
+ { .fd = control_fd_.get(), .events = POLLIN, .revents = 0 },
+ { .fd = monitor_event_fd_.get(), .events = POLLIN, .revents = 0 },
};
- int rc = TEMP_FAILURE_RETRY(adb_poll(pfd, 2, -1));
+ // If we don't see our first bind within a second, try again.
+ int timeout_ms = bound ? -1 : 1000;
+
+ int rc = TEMP_FAILURE_RETRY(adb_poll(pfd, 2, timeout_ms));
if (rc == -1) {
PLOG(FATAL) << "poll on USB control fd failed";
+ } else if (rc == 0) {
+ LOG(WARNING) << "timed out while waiting for FUNCTIONFS_BIND, trying again";
+ break;
}
if (pfd[1].revents) {
- // We were told to die, continue reading until FUNCTIONFS_UNBIND.
+ // We were told to die.
+ break;
}
struct usb_functionfs_event event;
- rc = TEMP_FAILURE_RETRY(adb_read(control_fd_->get(), &event, sizeof(event)));
+ rc = TEMP_FAILURE_RETRY(adb_read(control_fd_.get(), &event, sizeof(event)));
if (rc == -1) {
PLOG(FATAL) << "failed to read functionfs event";
} else if (rc == 0) {
@@ -291,10 +313,28 @@
switch (event.type) {
case FUNCTIONFS_BIND:
- LOG(FATAL) << "received FUNCTIONFS_BIND after already opened?";
+ if (bound) {
+ LOG(WARNING) << "received FUNCTIONFS_BIND while already bound?";
+ running = false;
+ break;
+ }
+
+ if (enabled) {
+ LOG(WARNING) << "received FUNCTIONFS_BIND while already enabled?";
+ running = false;
+ break;
+ }
+
+ bound = true;
break;
case FUNCTIONFS_ENABLE:
+ if (!bound) {
+ LOG(WARNING) << "received FUNCTIONFS_ENABLE while not bound?";
+ running = false;
+ break;
+ }
+
if (enabled) {
LOG(WARNING) << "received FUNCTIONFS_ENABLE while already enabled?";
running = false;
@@ -306,6 +346,10 @@
break;
case FUNCTIONFS_DISABLE:
+ if (!bound) {
+ LOG(WARNING) << "received FUNCTIONFS_DISABLE while not bound?";
+ }
+
if (!enabled) {
LOG(WARNING) << "received FUNCTIONFS_DISABLE while not enabled?";
}
@@ -319,6 +363,11 @@
LOG(WARNING) << "received FUNCTIONFS_UNBIND while still enabled?";
}
+ if (!bound) {
+ LOG(WARNING) << "received FUNCTIONFS_UNBIND when not bound?";
+ }
+
+ bound = false;
running = false;
break;
@@ -332,7 +381,7 @@
if ((event.u.setup.bRequestType & USB_DIR_IN)) {
LOG(INFO) << "acking device-to-host control transfer";
- ssize_t rc = adb_write(control_fd_->get(), "", 0);
+ ssize_t rc = adb_write(control_fd_.get(), "", 0);
if (rc != 0) {
PLOG(ERROR) << "failed to write empty packet to host";
break;
@@ -341,7 +390,7 @@
std::string buf;
buf.resize(event.u.setup.wLength + 1);
- ssize_t rc = adb_read(control_fd_->get(), buf.data(), buf.size());
+ ssize_t rc = adb_read(control_fd_.get(), buf.data(), buf.size());
if (rc != event.u.setup.wLength) {
LOG(ERROR)
<< "read " << rc
@@ -377,12 +426,6 @@
uint64_t dummy;
ssize_t rc = adb_read(worker_event_fd_.get(), &dummy, sizeof(dummy));
if (rc == -1) {
- if (errno == EINTR) {
- // We were interrupted either to stop, or because of a backtrace.
- // Check stopped_ again to see if we need to exit.
- continue;
- }
-
PLOG(FATAL) << "failed to read from eventfd";
} else if (rc == 0) {
LOG(FATAL) << "hit EOF on eventfd";
@@ -419,7 +462,6 @@
}
worker_thread_.join();
- worker_started_ = false;
}
void PrepareReadBlock(IoBlock* block, uint64_t id) {
@@ -637,13 +679,10 @@
std::once_flag error_flag_;
unique_fd worker_event_fd_;
+ unique_fd monitor_event_fd_;
ScopedAioContext aio_context_;
-
- // We keep a pointer to the control fd, so that we can reuse it to avoid USB reconfiguration,
- // and still be able to reset it to force a reopen after FUNCTIONFS_UNBIND or running into an
- // unexpected situation.
- unique_fd* control_fd_;
+ unique_fd control_fd_;
unique_fd read_fd_;
unique_fd write_fd_;
@@ -672,16 +711,15 @@
static void usb_ffs_open_thread() {
adb_thread_setname("usb ffs open");
- unique_fd control;
- unique_fd bulk_out;
- unique_fd bulk_in;
-
while (true) {
if (gFfsAioSupported.has_value() && !gFfsAioSupported.value()) {
LOG(INFO) << "failed to use nonblocking ffs, falling back to legacy";
return usb_init_legacy();
}
+ unique_fd control;
+ unique_fd bulk_out;
+ unique_fd bulk_in;
if (!open_functionfs(&control, &bulk_out, &bulk_in)) {
std::this_thread::sleep_for(1s);
continue;
@@ -692,7 +730,7 @@
std::promise<void> destruction_notifier;
std::future<void> future = destruction_notifier.get_future();
transport->SetConnection(std::make_unique<UsbFfsConnection>(
- &control, std::move(bulk_out), std::move(bulk_in),
+ std::move(control), std::move(bulk_out), std::move(bulk_in),
std::move(destruction_notifier)));
register_transport(transport);
future.wait();
diff --git a/adb/daemon/usb_ffs.cpp b/adb/daemon/usb_ffs.cpp
index 1b54022..a64ce40 100644
--- a/adb/daemon/usb_ffs.cpp
+++ b/adb/daemon/usb_ffs.cpp
@@ -297,32 +297,8 @@
PLOG(ERROR) << "failed to write USB strings";
return false;
}
-
- // Signal init after we've wwritten our descriptors.
+ // Signal only when writing the descriptors to ffs
android::base::SetProperty("sys.usb.ffs.ready", "1");
-
- // Read until we get FUNCTIONFS_BIND from the control endpoint.
- while (true) {
- struct usb_functionfs_event event;
- ssize_t rc = TEMP_FAILURE_RETRY(adb_read(control.get(), &event, sizeof(event)));
-
- if (rc == -1) {
- PLOG(FATAL) << "failed to read from FFS control fd";
- } else if (rc == 0) {
- LOG(WARNING) << "hit EOF on functionfs control fd during initialization";
- } else if (rc != sizeof(event)) {
- LOG(FATAL) << "read functionfs event of unexpected size, expected " << sizeof(event)
- << ", got " << rc;
- }
-
- if (event.type != FUNCTIONFS_BIND) {
- LOG(FATAL) << "first read on functionfs control fd returned non-bind: "
- << event.type;
- } else {
- break;
- }
- }
-
*out_control = std::move(control);
}
