Stop holding mProxyLock while downloading PAC script data.
Holding PacManager#mProxyLock for long time eventually causes ANR when
launching new applications.
1. ActivityThread#handleBindApplication blocks until
ActivityThread#handleBindApplication
-> ConnectivityService#getProxyForNetwork()
-> ConnectivityService#getDefaultProxy()
obtains the lock of ConnectivityService#mProxyLock
2. ConnectivityService#mProxyLock can be held by
ConnectivityService#setGlobalProxy() running on another thread until
ConnectivityService#setGlobalProxy()
-> ConnectivityService#sendProxyBroadcast()
-> PacManager#setCurrentProxyScriptUrl()
obtains the lock of PacManager#mProxyLock
3. Before the CL, PacManager#mProxyLock could be held by
mPacDownloader#run() on mNetThread until downloading a PAC script
completed.
The CL fixes the step 3 so that mPacDownloader#run does not keep the
lock of PacManager#mProxyLock. It eventually fixes long blocking at
ActivityThread#handleBindApplication and ANR when launching an
application.
Bug: 36317236
Test: Build succeeded
Merged-In: I864e41e1142178681f8f1b4fb7750d37e3ab2076
Change-Id: I864e41e1142178681f8f1b4fb7750d37e3ab2076
diff --git a/services/core/java/com/android/server/connectivity/PacManager.java b/services/core/java/com/android/server/connectivity/PacManager.java
index 58c76ec..46f76b1 100644
--- a/services/core/java/com/android/server/connectivity/PacManager.java
+++ b/services/core/java/com/android/server/connectivity/PacManager.java
@@ -15,6 +15,7 @@
*/
package com.android.server.connectivity;
+import android.annotation.WorkerThread;
import android.app.AlarmManager;
import android.app.PendingIntent;
import android.content.BroadcastReceiver;
@@ -73,7 +74,7 @@
public static final String KEY_PROXY = "keyProxy";
private String mCurrentPac;
@GuardedBy("mProxyLock")
- private Uri mPacUrl = Uri.EMPTY;
+ private volatile Uri mPacUrl = Uri.EMPTY;
private AlarmManager mAlarmManager;
@GuardedBy("mProxyLock")
@@ -86,29 +87,34 @@
private int mCurrentDelay;
private int mLastPort;
- private boolean mHasSentBroadcast;
- private boolean mHasDownloaded;
+ private volatile boolean mHasSentBroadcast;
+ private volatile boolean mHasDownloaded;
private Handler mConnectivityHandler;
private int mProxyMessage;
/**
- * Used for locking when setting mProxyService and all references to mPacUrl or mCurrentPac.
+ * Used for locking when setting mProxyService and all references to mCurrentPac.
*/
private final Object mProxyLock = new Object();
+ /**
+ * Runnable to download PAC script.
+ * The behavior relies on the assamption it always run on mNetThread to guarantee that the
+ * latest data fetched from mPacUrl is stored in mProxyService.
+ */
private Runnable mPacDownloader = new Runnable() {
@Override
+ @WorkerThread
public void run() {
String file;
- synchronized (mProxyLock) {
- if (Uri.EMPTY.equals(mPacUrl)) return;
- try {
- file = get(mPacUrl);
- } catch (IOException ioe) {
- file = null;
- Log.w(TAG, "Failed to load PAC file: " + ioe);
- }
+ final Uri pacUrl = mPacUrl;
+ if (Uri.EMPTY.equals(pacUrl)) return;
+ try {
+ file = get(pacUrl);
+ } catch (IOException ioe) {
+ file = null;
+ Log.w(TAG, "Failed to load PAC file: " + ioe);
}
if (file != null) {
synchronized (mProxyLock) {
@@ -171,9 +177,7 @@
// Allow to send broadcast, nothing to do.
return false;
}
- synchronized (mProxyLock) {
- mPacUrl = proxy.getPacFileUrl();
- }
+ mPacUrl = proxy.getPacFileUrl();
mCurrentDelay = DELAY_1;
mHasSentBroadcast = false;
mHasDownloaded = false;