From 664a618c069b52d2e03d287d164dfdacd6038489 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 5 Jun 2017 14:54:45 -0700 Subject: [PATCH 1/2] adb: don't hold queue lock while performing callbacks. Fix yet another source of deadlocks. Bug: http://b/62020217 Test: unplugged device on darwin Change-Id: I3fb0b3a84c56aed7d0da8ddba36e2d01fdb682ee --- adb/adb_utils.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/adb/adb_utils.h b/adb/adb_utils.h index 20c63b39d..11c0ec9cc 100644 --- a/adb/adb_utils.h +++ b/adb/adb_utils.h @@ -74,13 +74,18 @@ class BlockingQueue { template void PopAll(Fn fn) { - std::unique_lock lock(mutex); - cv.wait(lock, [this]() { return !queue.empty(); }); + std::vector popped; - for (const T& t : queue) { + { + std::unique_lock lock(mutex); + cv.wait(lock, [this]() { return !queue.empty(); }); + popped = std::move(queue); + queue.clear(); + } + + for (const T& t : popped) { fn(t); } - queue.clear(); } }; From 60b8c26520922f296a36efb17b8914b6f1e115ea Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 5 Jun 2017 15:08:13 -0700 Subject: [PATCH 2/2] adb: libusb: don't try to delete a usb_handle twice. Previously, we would attempt to delete a usb_handle in both device_disconnected and usb_close. If the one in device_disconnected happened to happen first, usb_close would abort when it failed to find the handle it was supposed to own. Bug: http://b/62020217 Test: unplugging device on darwin Change-Id: I6c6bf61bf89a4d9a23458c00b457080d3d6cc744 --- adb/client/usb_libusb.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/adb/client/usb_libusb.cpp b/adb/client/usb_libusb.cpp index 7e77b5ede..b2fdc07f0 100644 --- a/adb/client/usb_libusb.cpp +++ b/adb/client/usb_libusb.cpp @@ -422,8 +422,10 @@ static void device_disconnected(libusb_device* device) { if (!it->second->device_handle) { // If the handle is null, we were never able to open the device. unregister_usb_transport(it->second.get()); + usb_handles.erase(it); + } else { + // Closure of the transport will erase the usb_handle. } - usb_handles.erase(it); } }