Fix order of setting/saving state in VpnService.
and also refactor code making sure a thread won't grab two locks (which
may cause deadlocks in some corner cases).
diff --git a/packages/VpnServices/src/com/android/server/vpn/VpnService.java b/packages/VpnServices/src/com/android/server/vpn/VpnService.java
index f410c7b..5524ee5 100644
--- a/packages/VpnServices/src/com/android/server/vpn/VpnService.java
+++ b/packages/VpnServices/src/com/android/server/vpn/VpnService.java
@@ -133,11 +133,7 @@
if (VpnState.CONNECTED.equals(mState)) {
Log.i("VpnService", " recovered: " + mProfile.getName());
- new Thread(new Runnable() {
- public void run() {
- enterConnectivityLoop();
- }
- }).start();
+ startConnectivityMonitor();
}
}
@@ -213,16 +209,18 @@
SystemProperties.get(VPN_STATUS))) {
onConnected();
return;
- } else if (mDaemonHelper.anySocketError()) {
- return;
+ } else {
+ int err = mDaemonHelper.getSocketError();
+ if (err != 0) {
+ onError(err);
+ return;
+ }
}
sleep(500); // 0.5 second
}
- synchronized (VpnService.this) {
- if (mState == VpnState.CONNECTING) {
- onError(new IOException("Connecting timed out"));
- }
+ if (mState == VpnState.CONNECTING) {
+ onError(new IOException("Connecting timed out"));
}
}
@@ -235,13 +233,15 @@
mStartTime = System.currentTimeMillis();
- // set DNS after saving the states in case the process gets killed
- // before states are saved
+ // Correct order to make sure VpnService doesn't break when killed:
+ // (1) set state to CONNECTED
+ // (2) save states
+ // (3) set DNS
+ setState(VpnState.CONNECTED);
saveSelf();
setVpnDns();
- setState(VpnState.CONNECTED);
- enterConnectivityLoop();
+ startConnectivityMonitor();
}
private void saveSelf() throws IOException {
@@ -340,23 +340,28 @@
}
}
- private void enterConnectivityLoop() {
- Log.i(TAG, "VPN connectivity monitor running");
- try {
- for (;;) {
- synchronized (VpnService.this) {
- if (mState != VpnState.CONNECTED || !checkConnectivity()) {
- break;
+ private void startConnectivityMonitor() {
+ new Thread(new Runnable() {
+ public void run() {
+ Log.i(TAG, "VPN connectivity monitor running");
+ try {
+ for (;;) {
+ synchronized (VpnService.this) {
+ if ((mState != VpnState.CONNECTED)
+ || !checkConnectivity()) {
+ break;
+ }
+ mNotification.update();
+ checkDns();
+ VpnService.this.wait(1000); // 1 second
+ }
}
- mNotification.update();
- checkDns();
- VpnService.this.wait(1000); // 1 second
+ } catch (InterruptedException e) {
+ onError(e);
}
+ Log.i(TAG, "VPN connectivity monitor stopped");
}
- } catch (InterruptedException e) {
- onError(e);
- }
- Log.i(TAG, "VPN connectivity monitor stopped");
+ }).start();
}
private void saveLocalIpAndInterface(String serverIp) throws IOException {
@@ -432,11 +437,7 @@
}
synchronized void stopAll() {
- if (mDaemonList.isEmpty()) {
- onFinalCleanUp();
- } else {
- for (DaemonProxy s : mDaemonList) s.stop();
- }
+ for (DaemonProxy s : mDaemonList) s.stop();
}
synchronized void closeSockets() {
@@ -461,30 +462,26 @@
}
}
- synchronized boolean anySocketError() {
+ synchronized int getSocketError() {
for (DaemonProxy s : mDaemonList) {
switch (getResultFromSocket(s)) {
case 0:
- continue;
+ return 0;
case AUTH_ERROR_CODE:
- onError(VpnManager.VPN_ERROR_AUTH);
- return true;
+ return VpnManager.VPN_ERROR_AUTH;
case CHALLENGE_ERROR_CODE:
- onError(VpnManager.VPN_ERROR_CHALLENGE);
- return true;
+ return VpnManager.VPN_ERROR_CHALLENGE;
case REMOTE_HUNG_UP_ERROR_CODE:
- onError(VpnManager.VPN_ERROR_REMOTE_HUNG_UP);
- return true;
+ return VpnManager.VPN_ERROR_REMOTE_HUNG_UP;
default:
- onError(VpnManager.VPN_ERROR_CONNECTION_FAILED);
- return true;
+ return VpnManager.VPN_ERROR_CONNECTION_FAILED;
}
}
- return false;
+ return 0;
}
}