diff --git a/meta-oe/recipes-devtools/android-tools/android-tools/0018-adbd-do-not-use-pthread_kill-as-a-thread-liveness-pr.patch b/meta-oe/recipes-devtools/android-tools/android-tools/0018-adbd-do-not-use-pthread_kill-as-a-thread-liveness-pr.patch new file mode 100644 index 000000000000..0ebe7aba69b5 --- /dev/null +++ b/meta-oe/recipes-devtools/android-tools/android-tools/0018-adbd-do-not-use-pthread_kill-as-a-thread-liveness-pr.patch @@ -0,0 +1,118 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Alex Kiernan +Date: Sun, 13 Sep 2026 08:31:56 +0100 +Subject: [PATCH] adbd: do not use pthread_kill as a thread-liveness probe + +UsbFfsConnection::StopWorker() polled `pthread_kill(handle, 0) == ESRCH` to ask +whether the worker had exited. That reads as portable and is not: it relies on +bionic clearing the thread id on exit, so a signal to a dead thread lands on +nothing. glibc keeps the thread descriptor valid until join(), so for an +exited-but-unjoined thread pthread_kill returns 0, the probe answers "still +running" for ever, and the loop never terminates. + +It is not a race on glibc, it is a hang, and the path that reaches it is the +ordinary one: a cable pull kills the worker on its own with "Cannot send after +transport endpoint shutdown", so StopWorker is entered with a corpse. +~UsbFfsConnection then never returns, usb_ffs_open_thread never reopens ep0, +and the kernel gadget re-enumerates perfectly while the host claims the +interface and never receives a CNXN. The device shows as `offline` until +something restarts adbd. On a unit whose only route in is adb, nothing can. + +The worker now reports its own exit through an atomic, which is true on both +libcs. It is set from a scope guard rather than as the lambda's last statement +because the SubmitRead failure path returns early, and that is precisely a case +StopWorker must not wait for ever on. The signal is still sent on every pass, +since interrupting a worker blocked in io_getevents is what the loop is for. + +Diagnosed on two RK3308 units, both wedged the first time either had its cable +pulled since boot: main thread in futex_wait joining the monitor, monitor +thread in hrtimer_nanosleep at the 100ms spin, the open thread waiting on a +connection reset that never comes, and no worker thread at all. strace of the +monitor shows both pthread_kill calls returning success with no tgkill issued. + +Upstream-Status: Pending + +AI-Generated: Claude Opus 5 (Claude Code) +Signed-off-by: Alex Kiernan +--- + packages/modules/adb/daemon/usb.cpp | 38 +++++++++++++++++++++-------- + 1 file changed, 28 insertions(+), 10 deletions(-) + +diff --git a/packages/modules/adb/daemon/usb.cpp b/packages/modules/adb/daemon/usb.cpp +index 1fe2d3aa3215..e44c7b788587 100644 +--- a/packages/modules/adb/daemon/usb.cpp ++++ b/packages/modules/adb/daemon/usb.cpp +@@ -146,6 +146,7 @@ struct UsbFfsConnection : public Connection { + UsbFfsConnection(unique_fd control, unique_fd read, unique_fd write, + std::promise destruction_notifier) + : worker_started_(false), ++ worker_exited_(false), + stopped_(false), + destruction_notifier_(std::move(destruction_notifier)), + control_fd_(std::move(control)), +@@ -406,6 +407,14 @@ struct UsbFfsConnection : public Connection { + adb_thread_setname("UsbFfs-worker"); + LOG(INFO) << "UsbFfs-worker thread spawned"; + ++ // On EVERY exit path, not as a final statement: the SubmitRead ++ // failure below returns early, and that is precisely a case ++ // StopWorker() must not wait forever on. ++ struct ExitReporter { ++ std::atomic* flag; ++ ~ExitReporter() { flag->store(true, std::memory_order_release); } ++ } exit_reporter{&worker_exited_}; ++ + for (size_t i = 0; i < kUsbReadQueueDepth; ++i) { + read_requests_[i] = CreateReadBlock(next_read_id_++); + if (!SubmitRead(&read_requests_[i])) { +@@ -435,8 +444,23 @@ struct UsbFfsConnection : public Connection { + return; + } + ++ // NOT pthread_kill(handle, 0) == ESRCH as a liveness probe. That reads ++ // as portable and is not: it relies on bionic clearing the thread id ++ // when a thread exits, so the signal lands on nothing. glibc keeps the ++ // descriptor valid until join(), so for an exited-but-unjoined thread ++ // pthread_kill returns 0 and the probe says "still running" for ever. ++ // ++ // The loop is reached with a dead worker in the one case that matters: ++ // a cable pull kills the worker on its own (transport endpoint ++ // shutdown), and then ~UsbFfsConnection never returns, the ffs ++ // endpoints are never reopened, and the host sees a device that ++ // enumerates but never sends CNXN - offline until adbd is restarted. ++ // On a unit whose only way in IS adb, that is unrecoverable. ++ // ++ // The signal is still sent on every pass, because that is what the loop ++ // is for: interrupting a worker blocked in io_getevents. + pthread_t worker_thread_handle = worker_thread_.native_handle(); +- while (true) { ++ while (!worker_exited_.load(std::memory_order_acquire)) { + int rc = pthread_kill(worker_thread_handle, kInterruptionSignal); + if (rc != 0) { + LOG(ERROR) << "failed to send interruption signal to worker: " << strerror(rc); +@@ -444,15 +468,6 @@ struct UsbFfsConnection : public Connection { + } + + std::this_thread::sleep_for(100ms); +- +- rc = pthread_kill(worker_thread_handle, 0); +- if (rc == 0) { +- continue; +- } else if (rc == ESRCH) { +- break; +- } else { +- LOG(ERROR) << "failed to send interruption signal to worker: " << strerror(rc); +- } + } + + worker_thread_.join(); +@@ -709,6 +724,9 @@ struct UsbFfsConnection : public Connection { + + bool worker_started_; + std::thread worker_thread_; ++ // Set by the worker itself on the way out. StopWorker() waits on this ++ // rather than probing the thread handle - see the comment there. ++ std::atomic worker_exited_; + + std::atomic stopped_; + std::promise destruction_notifier_; diff --git a/meta-oe/recipes-devtools/android-tools/android-tools_35.0.2.bb b/meta-oe/recipes-devtools/android-tools/android-tools_35.0.2.bb index b1e66198f88c..9a0bae98dd19 100644 --- a/meta-oe/recipes-devtools/android-tools/android-tools_35.0.2.bb +++ b/meta-oe/recipes-devtools/android-tools/android-tools_35.0.2.bb @@ -62,6 +62,7 @@ SRC_URI = "https://deb.debian.org/debian/pool/main/a/android-platform-tools/andr file://0015-adbd-make-fs-config-opt-in-on-non-android-builds.patch \ file://0016-adbd-forward-the-reboot-target-to-RESTART2.patch \ file://0017-adbd-shell-start-in-HOME-and-drop-the-Android-only-T.patch \ + file://0018-adbd-do-not-use-pthread_kill-as-a-thread-liveness-pr.patch \ " SRC_URI[orig.md5sum] = "352376965cdef7bd7505d8fefdd43d50"