diff options
author | 2024-08-05 14:10:40 -0700 | |
---|---|---|
committer | 2024-08-05 14:15:47 -0700 | |
commit | 908b09ce78edc6a3403d152f9f350077a3007dae (patch) | |
tree | d5088daa6ca19a3561f752c2ac306df10fd1def1 | |
parent | 7bbb484092a7e40fc38df0acd6e9df3856216238 (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.cpp | 12 | ||||
-rw-r--r-- | libs/binder/include/binder/ProcessState.h | 2 |
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; |