Fix LockdownVpnTracker deadlock when resetting legacy Always-On VPN

When resetting legacy Always-On VPN, the intent is handled by
main thread. And it will also initialize VPN shutdown process,
which will cause networkInfo changed event on
ConnectivityService internal thread. These two events need to
hold their corresponding lock and ask for the other one, which
causes the deadlock.

This patch move the event handling to the same thread to prevent
such deadlock, and cleanup some unused variables.

Change-Id: I5b656c0d0381acb4e33409a11f502db9b180296c
Fix: 139122208
Test: atest FrameworksNetTests, manual test
This commit is contained in:
junyulai
2019-08-30 16:44:45 +08:00
parent bef5d2165d
commit 465088ed2f
2 changed files with 20 additions and 16 deletions

View File

@@ -4550,7 +4550,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
Slog.w(TAG, "VPN for user " + user + " not ready yet. Skipping lockdown");
return false;
}
setLockdownTracker(new LockdownVpnTracker(mContext, mNMS, this, vpn, profile));
setLockdownTracker(new LockdownVpnTracker(mContext, this, mHandler, vpn, profile));
} else {
setLockdownTracker(null);
}

View File

@@ -19,6 +19,8 @@ package com.android.server.net;
import static android.Manifest.permission.CONNECTIVITY_INTERNAL;
import static android.provider.Settings.ACTION_VPN_SETTINGS;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.Notification;
import android.app.NotificationManager;
import android.app.PendingIntent;
@@ -32,7 +34,7 @@ import android.net.LinkProperties;
import android.net.NetworkInfo;
import android.net.NetworkInfo.DetailedState;
import android.net.NetworkInfo.State;
import android.os.INetworkManagementService;
import android.os.Handler;
import android.security.Credentials;
import android.security.KeyStore;
import android.text.TextUtils;
@@ -63,19 +65,18 @@ public class LockdownVpnTracker {
private static final String ACTION_LOCKDOWN_RESET = "com.android.server.action.LOCKDOWN_RESET";
private static final int ROOT_UID = 0;
@NonNull private final Context mContext;
@NonNull private final ConnectivityService mConnService;
@NonNull private final Handler mHandler;
@NonNull private final Vpn mVpn;
@NonNull private final VpnProfile mProfile;
private final Context mContext;
private final INetworkManagementService mNetService;
private final ConnectivityService mConnService;
private final Vpn mVpn;
private final VpnProfile mProfile;
@NonNull private final Object mStateLock = new Object();
private final Object mStateLock = new Object();
private final PendingIntent mConfigIntent;
private final PendingIntent mResetIntent;
@NonNull private final PendingIntent mConfigIntent;
@NonNull private final PendingIntent mResetIntent;
@Nullable
private String mAcceptedEgressIface;
private int mErrorCount;
@@ -84,11 +85,14 @@ public class LockdownVpnTracker {
return KeyStore.getInstance().contains(Credentials.LOCKDOWN_VPN);
}
public LockdownVpnTracker(Context context, INetworkManagementService netService,
ConnectivityService connService, Vpn vpn, VpnProfile profile) {
public LockdownVpnTracker(@NonNull Context context,
@NonNull ConnectivityService connService,
@NonNull Handler handler,
@NonNull Vpn vpn,
@NonNull VpnProfile profile) {
mContext = Preconditions.checkNotNull(context);
mNetService = Preconditions.checkNotNull(netService);
mConnService = Preconditions.checkNotNull(connService);
mHandler = Preconditions.checkNotNull(handler);
mVpn = Preconditions.checkNotNull(vpn);
mProfile = Preconditions.checkNotNull(profile);
@@ -198,7 +202,7 @@ public class LockdownVpnTracker {
mVpn.setLockdown(true);
final IntentFilter resetFilter = new IntentFilter(ACTION_LOCKDOWN_RESET);
mContext.registerReceiver(mResetReceiver, resetFilter, CONNECTIVITY_INTERNAL, null);
mContext.registerReceiver(mResetReceiver, resetFilter, CONNECTIVITY_INTERNAL, mHandler);
handleStateChangedLocked();
}