From 63b52ec13b8b612b79d9c6884776b72bff6bebd0 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 26 Mar 2019 13:06:38 -0700 Subject: [PATCH 1/2] adbd: increment writes_submitted_ before submitting writes. If we fail to submit writes for some reason (e.g. the USB cable was unplugged), another thread that's waiting on the write mutex can enter SubmitWrites and attempt to resubmit the writes that we already failed to submit, leading to a failed assertion of !IoBlock::pending. Increment writes_submitted_ before actually calling io_submit, so we skip over these writes and fall through to exit. Bug: http://b/129134256 Test: manually unplugged a blueline Change-Id: I2fd0034e45db22c8f637c81039ce686b7aa6a03b --- adb/daemon/usb.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp index b42236ece..765357971 100644 --- a/adb/daemon/usb.cpp +++ b/adb/daemon/usb.cpp @@ -552,6 +552,8 @@ struct UsbFfsConnection : public Connection { LOG(VERBOSE) << "submitting write_request " << static_cast(iocbs[i]); } + writes_submitted_ += writes_to_submit; + int rc = io_submit(aio_context_.get(), writes_to_submit, iocbs); if (rc == -1) { HandleError(StringPrintf("failed to submit write requests: %s", strerror(errno))); @@ -560,8 +562,6 @@ struct UsbFfsConnection : public Connection { LOG(FATAL) << "failed to submit all writes: wanted to submit " << writes_to_submit << ", actually submitted " << rc; } - - writes_submitted_ += rc; } void HandleError(const std::string& error) { From 6933d54e0944c773a39ee69ecb34ede3bebf3799 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 26 Mar 2019 13:21:42 -0700 Subject: [PATCH 2/2] adbd: listen to all functionfs events. Monitor for FUNCTIONFS_UNBIND as well, so that in the case where we get FUNCTIONFS_BIND, FUNCTIONFS_UNBIND, FUNCTIONFS_BIND, we don't trigger an assertion failure from seeing two FUNCTIONFS_BINDs. Bug: http://b/129134256 Test: manual Change-Id: I80af5f4b833513e932262638b9f8d76bbcb35504 --- adb/daemon/usb.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp index 765357971..8a5000336 100644 --- a/adb/daemon/usb.cpp +++ b/adb/daemon/usb.cpp @@ -267,7 +267,7 @@ struct UsbFfsConnection : public Connection { adb_thread_setname("UsbFfs-monitor"); bool bound = false; - bool started = false; + bool enabled = false; bool running = true; while (running) { adb_pollfd pfd[2] = { @@ -298,16 +298,32 @@ struct UsbFfsConnection : public Connection { switch (event.type) { case FUNCTIONFS_BIND: CHECK(!bound) << "received FUNCTIONFS_BIND while already bound?"; + CHECK(!enabled) << "received FUNCTIONFS_BIND while already enabled?"; bound = true; + break; case FUNCTIONFS_ENABLE: - CHECK(!started) << "received FUNCTIONFS_ENABLE while already running?"; - started = true; + CHECK(bound) << "received FUNCTIONFS_ENABLE while not bound?"; + CHECK(!enabled) << "received FUNCTIONFS_ENABLE while already enabled?"; + enabled = true; + StartWorker(); break; case FUNCTIONFS_DISABLE: + CHECK(bound) << "received FUNCTIONFS_DISABLE while not bound?"; + CHECK(enabled) << "received FUNCTIONFS_DISABLE while not enabled?"; + enabled = false; + + running = false; + break; + + case FUNCTIONFS_UNBIND: + CHECK(!enabled) << "received FUNCTIONFS_UNBIND while still enabled?"; + CHECK(bound) << "received FUNCTIONFS_UNBIND when not bound?"; + bound = false; + running = false; break; } @@ -339,7 +355,7 @@ struct UsbFfsConnection : public Connection { LOG(FATAL) << "hit EOF on eventfd"; } - WaitForEvents(); + ReadEvents(); } }); } @@ -389,7 +405,7 @@ struct UsbFfsConnection : public Connection { return block; } - void WaitForEvents() { + void ReadEvents() { static constexpr size_t kMaxEvents = kUsbReadQueueDepth + kUsbWriteQueueDepth; struct io_event events[kMaxEvents]; struct timespec timeout = {.tv_sec = 0, .tv_nsec = 0};