From 510cdfc32caac0beb2fc10918f4113a5226fa02b Mon Sep 17 00:00:00 2001 From: Olivier Gaillard Date: Mon, 17 Sep 2018 09:35:32 +0100 Subject: [PATCH 1/2] Adds a mechanism to listen to proxy transact method calls. There are multiple use cases for it: 1) Make it easy for another process to set the worksource. The worksource can be propagated in a thread local - this is how gmscore and soon system server works - the worksource can then be set for all binder calls using Object transactStarted() { Binder.setWorkSource(ThreadLocalWorkSourceUid.get()); return null; // No token needed. } void transactEnded() { Binder.setWorkSource(null); } This will be used by system process and gmscore. 2) SystemUI team was interested in detecting binder calls done from the main thread in dogfood/tests. This listener will make it easy to figure out which thread is used. Performance impact of transact method: - With current code: 45ns per call - With this code: 57ns per call This is not significant compared to the total binder call time which is 10-100s of microseconds. Test: unit test Change-Id: Id0a2f52cba33b390ff83f703284b79471cc80b1c --- core/java/android/os/Binder.java | 40 +++++++++ core/java/android/os/BinderProxy.java | 22 +++++ .../src/android/os/BinderProxyTest.java | 88 +++++++++++++++++++ 3 files changed, 150 insertions(+) create mode 100644 core/tests/coretests/src/android/os/BinderProxyTest.java diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java index 8b6194c29707b..b2b1d33cf58f7 100644 --- a/core/java/android/os/Binder.java +++ b/core/java/android/os/Binder.java @@ -554,6 +554,46 @@ public class Binder implements IBinder { sDumpDisabled = msg; } + /** + * Listener to be notified about each proxy-side binder call. + * + * See {@link setProxyTransactListener}. + * @hide + */ + public interface ProxyTransactListener { + /** + * Called before onTransact. + * + * @return an object that will be passed back to #onTransactEnded (or null). + */ + Object onTransactStarted(IBinder binder, int transactionCode); + + /** + * Called after onTranact (even when an exception is thrown). + * + * @param session The object return by #onTransactStarted. + */ + void onTransactEnded(@Nullable Object session); + } + + /** + * Sets a listener for the transact method on the proxy-side. + * + *
  • The listener is global. Only fast operations should be done to avoid thread + * contentions. + *
  • The listener implementation needs to handle synchronization if needed. The methods on the + * listener can be called concurrently. + *
  • Listener set will be used for new transactions. On-going transaction will still use the + * previous listener (if already set). + *
  • The listener is called on the critical path of the binder transaction so be careful about + * performance. + *
  • Never execute another binder transaction inside the listener. + * @hide + */ + public static void setProxyTransactListener(@Nullable ProxyTransactListener listener) { + BinderProxy.setTransactListener(listener); + } + /** * Default implementation is a stub that returns false. You will want * to override this to do the appropriate unmarshalling of transactions. diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java index 591370ff728bf..720c16723c63c 100644 --- a/core/java/android/os/BinderProxy.java +++ b/core/java/android/os/BinderProxy.java @@ -17,6 +17,7 @@ package android.os; import android.annotation.NonNull; +import android.annotation.Nullable; import android.util.Log; import android.util.SparseIntArray; @@ -45,6 +46,15 @@ public final class BinderProxy implements IBinder { // Assume the process-wide default value when created volatile boolean mWarnOnBlocking = Binder.sWarnOnBlocking; + private static volatile Binder.ProxyTransactListener sTransactListener = null; + + /** + * @see {@link Binder#setProxyTransactListener(listener)}. + */ + public static void setTransactListener(@Nullable Binder.ProxyTransactListener listener) { + sTransactListener = listener; + } + /* * Map from longs to BinderProxy, retaining only a WeakReference to the BinderProxies. * We roll our own only because we need to lazily remove WeakReferences during accesses @@ -469,9 +479,21 @@ public final class BinderProxy implements IBinder { Trace.traceBegin(Trace.TRACE_TAG_ALWAYS, stackTraceElement.getClassName() + "." + stackTraceElement.getMethodName()); } + + // Make sure the listener won't change while processing a transaction. + final Binder.ProxyTransactListener transactListener = sTransactListener; + Object session = null; + if (transactListener != null) { + session = transactListener.onTransactStarted(this, code); + } + try { return transactNative(code, data, reply, flags); } finally { + if (transactListener != null) { + transactListener.onTransactEnded(session); + } + if (tracingEnabled) { Trace.traceEnd(Trace.TRACE_TAG_ALWAYS); } diff --git a/core/tests/coretests/src/android/os/BinderProxyTest.java b/core/tests/coretests/src/android/os/BinderProxyTest.java new file mode 100644 index 0000000000000..4c36b5c359a20 --- /dev/null +++ b/core/tests/coretests/src/android/os/BinderProxyTest.java @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.os; + +import android.annotation.Nullable; +import android.content.Context; +import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.MediumTest; + +public class BinderProxyTest extends AndroidTestCase { + private static class CountingListener implements Binder.ProxyTransactListener { + int mStartedCount; + int mEndedCount; + + public Object onTransactStarted(IBinder binder, int transactionCode) { + mStartedCount++; + return null; + } + + public void onTransactEnded(@Nullable Object session) { + mEndedCount++; + } + }; + + private PowerManager mPowerManager; + + /** + * Setup any common data for the upcoming tests. + */ + @Override + public void setUp() throws Exception { + super.setUp(); + mPowerManager = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE); + } + + @MediumTest + public void testNoListener() throws Exception { + CountingListener listener = new CountingListener(); + Binder.setProxyTransactListener(listener); + Binder.setProxyTransactListener(null); + + mPowerManager.isInteractive(); + + assertEquals(0, listener.mStartedCount); + assertEquals(0, listener.mEndedCount); + } + + @MediumTest + public void testListener() throws Exception { + CountingListener listener = new CountingListener(); + Binder.setProxyTransactListener(listener); + + mPowerManager.isInteractive(); + + assertEquals(1, listener.mStartedCount); + assertEquals(1, listener.mEndedCount); + } + + @MediumTest + public void testSessionPropagated() throws Exception { + Binder.setProxyTransactListener(new Binder.ProxyTransactListener() { + public Object onTransactStarted(IBinder binder, int transactionCode) { + return "foo"; + } + + public void onTransactEnded(@Nullable Object session) { + assertEquals("foo", session); + } + }); + + // Check it does not throw.. + mPowerManager.isInteractive(); + } +} From def1b90dec2f8620adcf5b95a03b0445b4caf5da Mon Sep 17 00:00:00 2001 From: Olivier Gaillard Date: Wed, 17 Oct 2018 17:10:41 +0100 Subject: [PATCH 2/2] Use the BinderProxy#TransactListener to propagate the UID. PropagateWorkSourceTransactListener intercepts outgoing calls and calls Binde#setThreadWorkSource. Also install the listener to system server to propagate the worksource through binder calls. Test: manual Change-Id: I02e88c93eebdf200691dd72b79aa7648f4d85bcb --- core/java/android/os/Binder.java | 33 +++++++++++++++++++ .../server/BinderCallsStatsService.java | 10 +++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/core/java/android/os/Binder.java b/core/java/android/os/Binder.java index b2b1d33cf58f7..e4f0358173e09 100644 --- a/core/java/android/os/Binder.java +++ b/core/java/android/os/Binder.java @@ -576,6 +576,39 @@ public class Binder implements IBinder { void onTransactEnded(@Nullable Object session); } + /** + * Propagates the work source to binder calls executed by the system server. + * + *
  • By default, this listener will propagate the worksource if the outgoing call happens on + * the same thread as the incoming binder call. + *
  • Custom attribution can be done by calling {@link ThreadLocalWorkSourceUid#set(int)}. + * @hide + */ + public static class PropagateWorkSourceTransactListener implements ProxyTransactListener { + @Override + public Object onTransactStarted(IBinder binder, int transactionCode) { + // Note that {@link Binder#getCallingUid()} is already set to the UID of the current + // process when this method is called. + // + // We use ThreadLocalWorkSourceUid instead. It also allows feature owners to set + // {@link ThreadLocalWorkSourceUid#set(int) manually to attribute resources to a UID. + int uid = ThreadLocalWorkSourceUid.get(); + if (uid >= 0) { + int originalUid = Binder.setThreadWorkSource(uid); + return Integer.valueOf(originalUid); + } + return null; + } + + @Override + public void onTransactEnded(Object session) { + if (session != null) { + int uid = (int) session; + Binder.setThreadWorkSource(uid); + } + } + } + /** * Sets a listener for the transact method on the proxy-side. * diff --git a/services/core/java/com/android/server/BinderCallsStatsService.java b/services/core/java/com/android/server/BinderCallsStatsService.java index 872261a035e5f..dd960751ab218 100644 --- a/services/core/java/com/android/server/BinderCallsStatsService.java +++ b/services/core/java/com/android/server/BinderCallsStatsService.java @@ -49,6 +49,7 @@ public class BinderCallsStatsService extends Binder { private static final String PERSIST_SYS_BINDER_CALLS_DETAILED_TRACKING = "persist.sys.binder_calls_detailed_tracking"; + /** Listens for flag changes. */ private static class SettingsObserver extends ContentObserver { private static final String SETTINGS_ENABLED_KEY = "enabled"; @@ -101,7 +102,14 @@ public class BinderCallsStatsService extends Binder { final boolean enabled = mParser.getBoolean(SETTINGS_ENABLED_KEY, BinderCallsStats.ENABLED_DEFAULT); if (mEnabled != enabled) { - Binder.setObserver(enabled ? mBinderCallsStats : null); + if (enabled) { + Binder.setObserver(mBinderCallsStats); + Binder.setProxyTransactListener( + new Binder.PropagateWorkSourceTransactListener()); + } else { + Binder.setObserver(null); + Binder.setProxyTransactListener(null); + } mEnabled = enabled; mBinderCallsStats.reset(); }