Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Ensure VcnGatewayConnection#isQuitting never gets unset after being set" into sc-dev

Browse Source
This commit is contained in:
Benedict Wong
2021-07-17 02:52:32 +00:00
committed by Android (Google) Code Review
parent a687e13ff3 29b3fdd6b6
commit 0df66631c0
9 changed files with 86 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
services/core/java/com/android/server/VcnManagementService.java
View File

@@ -550,12 +550,14 @@ public class VcnManagementService extends IVcnManagementService.Stub {
@GuardedBy("mLock")
private void stopVcnLocked(@NonNull ParcelUuid uuidToTeardown) {
final Vcn vcnToTeardown = mVcns.remove(uuidToTeardown);
// Remove in 2 steps. Make sure teardownAsync is triggered before removing from the map.
final Vcn vcnToTeardown = mVcns.get(uuidToTeardown);
if (vcnToTeardown == null) {
return;
}
vcnToTeardown.teardownAsynchronously();
mVcns.remove(uuidToTeardown);
// Now that the VCN is removed, notify all registered listeners to refresh their
// UnderlyingNetworkPolicy.
@@ -1054,18 +1056,15 @@ public class VcnManagementService extends IVcnManagementService.Stub {
private void logVdbg(String msg) {
if (VDBG) {
Slog.v(TAG, msg);
LOCAL_LOG.log(TAG + " VDBG: " + msg);
}
}
private void logDbg(String msg) {
Slog.d(TAG, msg);
LOCAL_LOG.log(TAG + " DBG: " + msg);
}
private void logDbg(String msg, Throwable tr) {
Slog.d(TAG, msg, tr);
LOCAL_LOG.log(TAG + " DBG: " + msg + tr);
}
private void logErr(String msg) {

3
services/core/java/com/android/server/vcn/Vcn.java
View File

@@ -528,18 +528,15 @@ public class Vcn extends Handler {
private void logVdbg(String msg) {
if (VDBG) {
Slog.v(TAG, getLogPrefix() + msg);
LOCAL_LOG.log(getLogPrefix() + "VDBG: " + msg);
}
}
private void logDbg(String msg) {
Slog.d(TAG, getLogPrefix() + msg);
LOCAL_LOG.log(getLogPrefix() + "DBG: " + msg);
}
private void logDbg(String msg, Throwable tr) {
Slog.d(TAG, getLogPrefix() + msg, tr);
LOCAL_LOG.log(getLogPrefix() + "DBG: " + msg + tr);
}
private void logErr(String msg) {

38
services/core/java/com/android/server/vcn/VcnGatewayConnection.java
View File

@@ -92,6 +92,7 @@ import com.android.server.vcn.UnderlyingNetworkTracker.UnderlyingNetworkTrackerC
import com.android.server.vcn.Vcn.VcnGatewayStatusCallback;
import com.android.server.vcn.util.LogUtils;
import com.android.server.vcn.util.MtuUtils;
import com.android.server.vcn.util.OneWayBoolean;
import java.io.IOException;
import java.net.Inet4Address;
@@ -551,8 +552,13 @@ public class VcnGatewayConnection extends StateMachine {
* <p>This variable is false for the lifecycle of the VcnGatewayConnection, until a command to
* teardown has been received. This may be flipped due to events such as the Network becoming
* unwanted, the owning VCN entering safe mode, or an irrecoverable internal failure.
*
* <p>WARNING: Assignments to this MUST ALWAYS (except for testing) use the or operator ("|="),
* otherwise the flag may be flipped back to false after having been set to true. This could
* lead to a case where the Vcn parent instance has commanded a teardown, but a spurious
* non-quitting disconnect request could flip this back to true.
*/
private boolean mIsQuitting = false;
private OneWayBoolean mIsQuitting = new OneWayBoolean();
/**
* Whether the VcnGatewayConnection is in safe mode.
@@ -794,7 +800,7 @@ public class VcnGatewayConnection extends StateMachine {
private void acquireWakeLock() {
mVcnContext.ensureRunningOnLooperThread();
if (!mIsQuitting) {
if (!mIsQuitting.getValue()) {
mWakeLock.acquire();
logVdbg("Wakelock acquired: " + mWakeLock);
@@ -1297,7 +1303,9 @@ public class VcnGatewayConnection extends StateMachine {
// TODO(b/180526152): notify VcnStatusCallback for Network loss
logDbg("Tearing down. Cause: " + info.reason);
mIsQuitting = info.shouldQuit;
if (info.shouldQuit) {
mIsQuitting.setTrue();
}
teardownNetwork();
@@ -1341,7 +1349,7 @@ public class VcnGatewayConnection extends StateMachine {
private class DisconnectedState extends BaseState {
@Override
protected void enterState() {
if (mIsQuitting) {
if (mIsQuitting.getValue()) {
quitNow(); // Ignore all queued events; cleanup is complete.
}
@@ -1365,7 +1373,7 @@ public class VcnGatewayConnection extends StateMachine {
break;
case EVENT_DISCONNECT_REQUESTED:
if (((EventDisconnectRequestedInfo) msg.obj).shouldQuit) {
mIsQuitting = true;
mIsQuitting.setTrue();
quitNow();
}
@@ -1451,7 +1459,10 @@ public class VcnGatewayConnection extends StateMachine {
break;
case EVENT_DISCONNECT_REQUESTED:
EventDisconnectRequestedInfo info = ((EventDisconnectRequestedInfo) msg.obj);
mIsQuitting = info.shouldQuit;
if (info.shouldQuit) {
mIsQuitting.setTrue();
}
teardownNetwork();
if (info.reason.equals(DISCONNECT_REASON_UNDERLYING_NETWORK_LOST)) {
@@ -1467,7 +1478,7 @@ public class VcnGatewayConnection extends StateMachine {
case EVENT_SESSION_CLOSED:
mIkeSession = null;
if (!mIsQuitting && mUnderlying != null) {
if (!mIsQuitting.getValue() && mUnderlying != null) {
transitionTo(mSkipRetryTimeout ? mConnectingState : mRetryTimeoutState);
} else {
teardownNetwork();
@@ -1626,7 +1637,7 @@ public class VcnGatewayConnection extends StateMachine {
teardownAsynchronously();
} /* networkUnwantedCallback */,
(status) -> {
if (mIsQuitting) {
if (mIsQuitting.getValue()) {
return; // Ignore; VcnGatewayConnection quitting or already quit
}
@@ -2180,18 +2191,15 @@ public class VcnGatewayConnection extends StateMachine {
private void logVdbg(String msg) {
if (VDBG) {
Slog.v(TAG, getLogPrefix() + msg);
LOCAL_LOG.log(getLogPrefix() + "VDBG: " + msg);
}
}
private void logDbg(String msg) {
Slog.d(TAG, getLogPrefix() + msg);
LOCAL_LOG.log(getLogPrefix() + "DBG: " + msg);
}
private void logDbg(String msg, Throwable tr) {
Slog.d(TAG, getLogPrefix() + msg, tr);
LOCAL_LOG.log(getLogPrefix() + "DBG: " + msg + tr);
}
private void logWarn(String msg) {
@@ -2238,7 +2246,7 @@ public class VcnGatewayConnection extends StateMachine {
+ (getCurrentState() == null
? null
: getCurrentState().getClass().getSimpleName()));
pw.println("mIsQuitting: " + mIsQuitting);
pw.println("mIsQuitting: " + mIsQuitting.getValue());
pw.println("mIsInSafeMode: " + mIsInSafeMode);
pw.println("mCurrentToken: " + mCurrentToken);
pw.println("mFailedAttempts: " + mFailedAttempts);
@@ -2275,12 +2283,12 @@ public class VcnGatewayConnection extends StateMachine {
@VisibleForTesting(visibility = Visibility.PRIVATE)
boolean isQuitting() {
return mIsQuitting;
return mIsQuitting.getValue();
}
@VisibleForTesting(visibility = Visibility.PRIVATE)
void setIsQuitting(boolean isQuitting) {
mIsQuitting = isQuitting;
void setQuitting() {
mIsQuitting.setTrue();
}
@VisibleForTesting(visibility = Visibility.PRIVATE)

39
services/core/java/com/android/server/vcn/util/OneWayBoolean.java Normal file
View File

@@ -0,0 +1,39 @@
/*
* Copyright (C) 2021 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.vcn.util;
/**
* OneWayBoolean is an abstraction for a boolean that MUST only ever be flipped from false to true
*
* <p>This class allows the providing of a guarantee that a flag will never be flipped back after
* being set.
*
* @hide
*/
public class OneWayBoolean {
private boolean mValue = false;
/** Get boolean value. */
public boolean getValue() {
return mValue;
}
/** Sets the value to true. */
public void setTrue() {
mValue = true;
}
}

4
tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java
View File

@@ -578,6 +578,10 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
mGatewayConnection.teardownAsynchronously();
mTestLooper.dispatchAll();
// Verify that sending a non-quitting disconnect request does not unset the isQuitting flag
mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false);
mTestLooper.dispatchAll();
assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());
assertTrue(mGatewayConnection.isQuitting());
}

4
tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectingStateTest.java
View File

@@ -127,6 +127,10 @@ public class VcnGatewayConnectionConnectingStateTest extends VcnGatewayConnectio
mGatewayConnection.teardownAsynchronously();
mTestLooper.dispatchAll();
// Verify that sending a non-quitting disconnect request does not unset the isQuitting flag
mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false);
mTestLooper.dispatchAll();
assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());
assertTrue(mGatewayConnection.isQuitting());
}

6
tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java
View File

@@ -68,7 +68,7 @@ public class VcnGatewayConnectionDisconnectedStateTest extends VcnGatewayConnect
true /* isMobileDataEnabled */,
mDeps);
vgc.setIsQuitting(true);
vgc.setQuitting();
vgc.transitionTo(vgc.mDisconnectedState);
mTestLooper.dispatchAll();
@@ -102,6 +102,10 @@ public class VcnGatewayConnectionDisconnectedStateTest extends VcnGatewayConnect
mGatewayConnection.teardownAsynchronously();
mTestLooper.dispatchAll();
// Verify that sending a non-quitting disconnect request does not unset the isQuitting flag
mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false);
mTestLooper.dispatchAll();
assertNull(mGatewayConnection.getCurrentState());
verify(mIpSecSvc).deleteTunnelInterface(eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), any());
verifySafeModeTimeoutAlarmAndGetCallback(true /* expectCanceled */);

4
tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionDisconnectingStateTest.java
View File

@@ -79,6 +79,10 @@ public class VcnGatewayConnectionDisconnectingStateTest extends VcnGatewayConnec
mGatewayConnection.teardownAsynchronously();
mTestLooper.dispatchAll();
// Verify that sending a non-quitting disconnect request does not unset the isQuitting flag
mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false);
mTestLooper.dispatchAll();
// Should do nothing; already tearing down.
assertEquals(mGatewayConnection.mDisconnectingState, mGatewayConnection.getCurrentState());
verifyTeardownTimeoutAlarmAndGetCallback(false /* expectCanceled */);

4
tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionRetryTimeoutStateTest.java
View File

@@ -90,6 +90,10 @@ public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnect
.onSelectedUnderlyingNetworkChanged(null);
mTestLooper.dispatchAll();
// Verify that sending a non-quitting disconnect request does not unset the isQuitting flag
mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false);
mTestLooper.dispatchAll();
assertEquals(mGatewayConnection.mDisconnectedState, mGatewayConnection.getCurrentState());
verifyRetryTimeoutAlarmAndGetCallback(mFirstRetryInterval, true /* expectCanceled */);