Skip to content

Commit 908b09c

Browse files
committed
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
1 parent 7bbb484 commit 908b09c

2 files changed

Lines changed: 11 additions & 3 deletions

File tree

libs/binder/ProcessState.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,17 @@ status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) {
429429
}
430430

431431
size_t ProcessState::getThreadPoolMaxTotalThreadCount() const {
432+
// Need to read `mKernelStartedThreads` before `mThreadPoolStarted` (with
433+
// non-relaxed memory ordering) to avoid a race like the following:
434+
//
435+
// thread A: if (mThreadPoolStarted) { // evaluates false
436+
// thread B: mThreadPoolStarted = true;
437+
// thread B: mKernelStartedThreads++;
438+
// thread A: size_t kernelStarted = mKernelStartedThreads;
439+
// thread A: LOG_ALWAYS_FATAL_IF(kernelStarted != 0, ...);
440+
size_t kernelStarted = mKernelStartedThreads;
441+
432442
if (mThreadPoolStarted) {
433-
size_t kernelStarted = mKernelStartedThreads;
434443
size_t max = mMaxThreads;
435444
size_t current = mCurrentThreads;
436445

@@ -460,7 +469,6 @@ size_t ProcessState::getThreadPoolMaxTotalThreadCount() const {
460469

461470
// must not be initialized or maybe has poll thread setup, we
462471
// currently don't track this in libbinder
463-
size_t kernelStarted = mKernelStartedThreads;
464472
LOG_ALWAYS_FATAL_IF(kernelStarted != 0, "Expecting 0 kernel started threads but have %zu",
465473
kernelStarted);
466474
return mCurrentThreads;

libs/binder/include/binder/ProcessState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ class ProcessState : public virtual RefBase {
188188
Vector<handle_entry> mHandleToObject;
189189

190190
bool mForked;
191-
bool mThreadPoolStarted;
191+
std::atomic_bool mThreadPoolStarted;
192192
volatile int32_t mThreadPoolSeq;
193193

194194
CallRestriction mCallRestriction;

0 commit comments

Comments
 (0)