From 7284daa96ed5c154673ab138eea85c6b679c2dca Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 30 May 2019 14:58:29 +0900 Subject: [PATCH] Prevent a 4-way binder interlock leading to a leak. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit System server | NetworkStack | NetworkMonitorCallbacks ←----|--- NetworkMonitorCallbacks$Stub$Proxy ↓ | ↑ NetworkAgentInfo | NetworkMonitor ↓ | ↑ NetworkMonitor$Stub$Proxy ----|---→ NetworkMonitorImpl Bug: b/133174607 Test: Manual. The simplest artifact is observed by watching the output of adb shell dumpsys meminfo -d com.android.networkstack | grep 'Proxy Binders' while connecting and disconnecting multiple times to any network. This will display the number of binder proxies. Before this, the binder proxy count increases by 1 with each connection and never goes down (there is some noise, as proxy objects are sometimes created for other reasons, and get GC'd eventually). After this, the binder proxy count is always eventually stable at 27 + connected network count. See the bug for the complete analysis. Change-Id: Ide2428dab3fcd6d7cd00aa2a9fd99d6c99b815a4 --- .../android/server/ConnectivityService.java | 19 +++++---- .../connectivity/AutodestructReference.java | 42 +++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 services/core/java/com/android/server/connectivity/AutodestructReference.java diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4b2d481cfd..cdab8c6ce0 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -169,6 +169,7 @@ import com.android.internal.util.MessageUtils; import com.android.internal.util.WakeupMessage; import com.android.internal.util.XmlUtils; import com.android.server.am.BatteryStatsService; +import com.android.server.connectivity.AutodestructReference; import com.android.server.connectivity.DataConnectionStats; import com.android.server.connectivity.DnsManager; import com.android.server.connectivity.DnsManager.PrivateDnsValidationUpdate; @@ -2761,29 +2762,31 @@ public class ConnectivityService extends IConnectivityManager.Stub } private class NetworkMonitorCallbacks extends INetworkMonitorCallbacks.Stub { - private final NetworkAgentInfo mNai; + private final int mNetId; + private final AutodestructReference mNai; private NetworkMonitorCallbacks(NetworkAgentInfo nai) { - mNai = nai; + mNetId = nai.network.netId; + mNai = new AutodestructReference(nai); } @Override public void onNetworkMonitorCreated(INetworkMonitor networkMonitor) { mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_AGENT, - new Pair<>(mNai, networkMonitor))); + new Pair<>(mNai.getAndDestroy(), networkMonitor))); } @Override public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) { mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage(EVENT_NETWORK_TESTED, - testResult, mNai.network.netId, redirectUrl)); + testResult, mNetId, redirectUrl)); } @Override public void notifyPrivateDnsConfigResolved(PrivateDnsConfigParcel config) { mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage( EVENT_PRIVATE_DNS_CONFIG_RESOLVED, - 0, mNai.network.netId, PrivateDnsConfig.fromParcel(config))); + 0, mNetId, PrivateDnsConfig.fromParcel(config))); } @Override @@ -2801,15 +2804,13 @@ public class ConnectivityService extends IConnectivityManager.Stub } mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage( EVENT_PROVISIONING_NOTIFICATION, PROVISIONING_NOTIFICATION_SHOW, - mNai.network.netId, - pendingIntent)); + mNetId, pendingIntent)); } @Override public void hideProvisioningNotification() { mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage( - EVENT_PROVISIONING_NOTIFICATION, PROVISIONING_NOTIFICATION_HIDE, - mNai.network.netId)); + EVENT_PROVISIONING_NOTIFICATION, PROVISIONING_NOTIFICATION_HIDE, mNetId)); } @Override diff --git a/services/core/java/com/android/server/connectivity/AutodestructReference.java b/services/core/java/com/android/server/connectivity/AutodestructReference.java new file mode 100644 index 0000000000..009a43e582 --- /dev/null +++ b/services/core/java/com/android/server/connectivity/AutodestructReference.java @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2019 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 com.android.server.connectivity; + +import android.annotation.NonNull; + +import java.util.concurrent.atomic.AtomicReference; + +/** + * A ref that autodestructs at the first usage of it. + * @param The type of the held object + * @hide + */ +public class AutodestructReference { + private final AtomicReference mHeld; + public AutodestructReference(@NonNull T obj) { + if (null == obj) throw new NullPointerException("Autodestruct reference to null"); + mHeld = new AtomicReference<>(obj); + } + + /** Get the ref and destruct it. NPE if already destructed. */ + @NonNull + public T getAndDestroy() { + final T obj = mHeld.getAndSet(null); + if (null == obj) throw new NullPointerException("Already autodestructed"); + return obj; + } +}