summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Frederick Mayle <fmayle@google.com> 2024-08-05 14:10:40 -0700
committer Frederick Mayle <fmayle@google.com> 2024-08-05 14:15:47 -0700
commit908b09ce78edc6a3403d152f9f350077a3007dae (patch)
treed5088daa6ca19a3561f752c2ac306df10fd1def1
parent7bbb484092a7e40fc38df0acd6e9df3856216238 (diff)
fix race in ProcessState::getThreadPoolMaxTotalThreadCount
The race was introduced in https://r.android.com/3107366. Triggering the race requires calling `getThreadPoolMaxTotalThreadCount` concurrently with `startThreadPool`, which is a bad practice, but, we shouldn't crash. Bug: 355739944 Test: ran https://r.android.com/3207755 test for hours Change-Id: Iee9a99a213474f5b1a398e703b2af585ece6828f
-rw-r--r--libs/binder/ProcessState.cpp12
-rw-r--r--libs/binder/include/binder/ProcessState.h2
2 files changed, 11 insertions, 3 deletions
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index a42ede29a2..7c29dba2d0 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -429,8 +429,17 @@ status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) {
}
size_t ProcessState::getThreadPoolMaxTotalThreadCount() const {
+ // Need to read `mKernelStartedThreads` before `mThreadPoolStarted` (with
+ // non-relaxed memory ordering) to avoid a race like the following:
+ //
+ // thread A: if (mThreadPoolStarted) { // evaluates false
+ // thread B: mThreadPoolStarted = true;
+ // thread B: mKernelStartedThreads++;
+ // thread A: size_t kernelStarted = mKernelStartedThreads;
+ // thread A: LOG_ALWAYS_FATAL_IF(kernelStarted != 0, ...);
+ size_t kernelStarted = mKernelStartedThreads;
+
if (mThreadPoolStarted) {
- size_t kernelStarted = mKernelStartedThreads;
size_t max = mMaxThreads;
size_t current = mCurrentThreads;
@@ -460,7 +469,6 @@ size_t ProcessState::getThreadPoolMaxTotalThreadCount() const {
// must not be initialized or maybe has poll thread setup, we
// currently don't track this in libbinder
- size_t kernelStarted = mKernelStartedThreads;
LOG_ALWAYS_FATAL_IF(kernelStarted != 0, "Expecting 0 kernel started threads but have %zu",
kernelStarted);
return mCurrentThreads;
diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h
index 021bd58e21..ffa222907a 100644
--- a/libs/binder/include/binder/ProcessState.h
+++ b/libs/binder/include/binder/ProcessState.h
@@ -188,7 +188,7 @@ private:
Vector<handle_entry> mHandleToObject;
bool mForked;
- bool mThreadPoolStarted;
+ std::atomic_bool mThreadPoolStarted;
volatile int32_t mThreadPoolSeq;
CallRestriction mCallRestriction;