Prevent a 4-way binder interlock leading to a leak.
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
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 4b2d481..cdab8c6 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.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 @@
}
private class NetworkMonitorCallbacks extends INetworkMonitorCallbacks.Stub {
- private final NetworkAgentInfo mNai;
+ private final int mNetId;
+ private final AutodestructReference<NetworkAgentInfo> 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 @@
}
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 0000000..009a43e
--- /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 <T> The type of the held object
+ * @hide
+ */
+public class AutodestructReference<T> {
+ private final AtomicReference<T> 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;
+ }
+}