From 0186839fe0b15c0ce705ffba6a7bfbf5df627646 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Wed, 21 Aug 2024 21:25:34 +0000 Subject: Binder: avoid GC while holding lock The JavaBBinder constructor may trigger a GC. This causes deadlocks. Though, I also think, usually when you create a binder object, you just send it out, and so maybe we should always create the local object instead of doing it lazily like this, but idk the history behind that, I've been thinking of changing it. Bug: 360067751 Test: boot, TH Change-Id: I97963c5e7b859d4ad474c715bb657b9baa19fabf --- core/jni/android_util_Binder.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index 2068bd7bc8ea..46b4695a9cec 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -466,10 +466,25 @@ class JavaBBinderHolder public: sp get(JNIEnv* env, jobject obj) { - AutoMutex _l(mLock); - sp b = mBinder.promote(); - if (b == NULL) { - b = new JavaBBinder(env, obj); + sp b; + { + AutoMutex _l(mLock); + // must take lock to promote because we set the same wp<> + // on another thread. + b = mBinder.promote(); + } + + if (b) return b; + + // b/360067751: constructor may trigger GC, so call outside lock + b = new JavaBBinder(env, obj); + + { + AutoMutex _l(mLock); + // if it was constructed on another thread in the meantime, + // return that. 'b' will just get destructed. + if (sp b2 = mBinder.promote(); b2) return b2; + if (mVintf) { ::android::internal::Stability::markVintf(b.get()); } -- cgit v1.2.3-59-g8ed1b