Fix race conditions between Tethering and TetherInterfaceStateMachine
( cherry-pick of de4819dc82b734036b22416228327d2712e01884 )
Previously, Tethering would answer requests for current tethering state
by calling methods on instances of TetherInterfaceStateMachine to build
up that state. This is incorrect, since state queries can come in on an
arbitrary thread, independent of updates to the state machines
instances.
Fix this by:
- Altering TetherInterfaceStateMachine to push consistent state
snapshots to Tethering via notifyInterfaceStateChange()
- Storing the last state snapshot in Tethering.mTetherStates
- Removing public methods to query TetherInterfaceStateMachine state.
- Consistently synchronizing access to Tethering.mTetherStates
Bug: 29009601
Test: WiFi Tethering continues to work, unittests continue to pass
Change-Id: Ied334f5e8739bc3aff1b08a1079095b9cc2a7958
diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java
index 4a0e81b..bef48d6 100644
--- a/services/core/java/com/android/server/connectivity/Tethering.java
+++ b/services/core/java/com/android/server/connectivity/Tethering.java
@@ -81,8 +81,6 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
-import java.util.Map;
-import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
@@ -126,7 +124,18 @@
private final INetworkStatsService mStatsService;
private final Looper mLooper;
- private Map<String, TetherInterfaceStateMachine> mIfaces; // all tethered/tetherable ifaces
+ private static class TetherState {
+ public final TetherInterfaceStateMachine mStateMachine;
+ public int mLastState;
+ public int mLastError;
+ public TetherState(TetherInterfaceStateMachine sm) {
+ mStateMachine = sm;
+ // Assume all state machines start out available and with no errors.
+ mLastState = IControlsTethering.STATE_AVAILABLE;
+ mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
+ }
+ }
+ private final ArrayMap<String, TetherState> mTetherStates;
private final BroadcastReceiver mStateReceiver;
@@ -174,7 +183,7 @@
mPublicSync = new Object();
- mIfaces = new ArrayMap<String, TetherInterfaceStateMachine>();
+ mTetherStates = new ArrayMap<>();
// make our own thread so we don't anr the system
mLooper = IoThread.get().getLooper();
@@ -255,22 +264,20 @@
return;
}
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
+ TetherState tetherState = mTetherStates.get(iface);
if (up) {
- if (sm == null) {
- sm = new TetherInterfaceStateMachine(iface, mLooper, interfaceType,
- mNMService, mStatsService, this);
- mIfaces.put(iface, sm);
- sm.start();
+ if (tetherState == null) {
+ trackNewTetherableInterface(iface, interfaceType);
}
} else {
if (interfaceType == ConnectivityManager.TETHERING_USB) {
// ignore usb0 down after enabling RNDIS
// we will handle disconnect in interfaceRemoved instead
if (VDBG) Log.d(TAG, "ignore interface down for " + iface);
- } else if (sm != null) {
- sm.sendMessage(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
- mIfaces.remove(iface);
+ } else if (tetherState != null) {
+ tetherState.mStateMachine.sendMessage(
+ TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
+ mTetherStates.remove(iface);
}
}
}
@@ -329,15 +336,12 @@
return;
}
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm != null) {
+ TetherState tetherState = mTetherStates.get(iface);
+ if (tetherState == null) {
+ trackNewTetherableInterface(iface, interfaceType);
+ } else {
if (VDBG) Log.d(TAG, "active iface (" + iface + ") reported as added, ignoring");
- return;
}
- sm = new TetherInterfaceStateMachine(iface, mLooper, interfaceType,
- mNMService, mStatsService, this);
- mIfaces.put(iface, sm);
- sm.start();
}
}
@@ -345,15 +349,15 @@
public void interfaceRemoved(String iface) {
if (VDBG) Log.d(TAG, "interfaceRemoved " + iface);
synchronized (mPublicSync) {
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm == null) {
+ TetherState tetherState = mTetherStates.get(iface);
+ if (tetherState == null) {
if (VDBG) {
Log.e(TAG, "attempting to remove unknown iface (" + iface + "), ignoring");
}
return;
}
- sm.sendMessage(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
- mIfaces.remove(iface);
+ tetherState.mStateMachine.sendMessage(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
+ mTetherStates.remove(iface);
}
}
@@ -582,19 +586,18 @@
public int tether(String iface) {
if (DBG) Log.d(TAG, "Tethering " + iface);
synchronized (mPublicSync) {
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm == null) {
+ TetherState tetherState = mTetherStates.get(iface);
+ if (tetherState == null) {
Log.e(TAG, "Tried to Tether an unknown iface :" + iface + ", ignoring");
return ConnectivityManager.TETHER_ERROR_UNKNOWN_IFACE;
}
// Ignore the error status of the interface. If the interface is available,
// the errors are referring to past tethering attempts anyway.
- if (!sm.isAvailable()) {
+ if (tetherState.mLastState != IControlsTethering.STATE_AVAILABLE) {
Log.e(TAG, "Tried to Tether an unavailable iface :" + iface + ", ignoring");
return ConnectivityManager.TETHER_ERROR_UNAVAIL_IFACE;
-
}
- sm.sendMessage(TetherInterfaceStateMachine.CMD_TETHER_REQUESTED);
+ tetherState.mStateMachine.sendMessage(TetherInterfaceStateMachine.CMD_TETHER_REQUESTED);
return ConnectivityManager.TETHER_ERROR_NO_ERROR;
}
}
@@ -602,43 +605,43 @@
public int untether(String iface) {
if (DBG) Log.d(TAG, "Untethering " + iface);
synchronized (mPublicSync) {
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm == null) {
+ TetherState tetherState = mTetherStates.get(iface);
+ if (tetherState == null) {
Log.e(TAG, "Tried to Untether an unknown iface :" + iface + ", ignoring");
return ConnectivityManager.TETHER_ERROR_UNKNOWN_IFACE;
}
- if (!sm.isTethered()) {
- Log.e(TAG, "Tried to Untethered an errored iface :" + iface + ", ignoring");
+ if (tetherState.mLastState != IControlsTethering.STATE_TETHERED) {
+ Log.e(TAG, "Tried to untether an untethered iface :" + iface + ", ignoring");
return ConnectivityManager.TETHER_ERROR_UNAVAIL_IFACE;
}
- sm.sendMessage(TetherInterfaceStateMachine.CMD_TETHER_UNREQUESTED);
+ tetherState.mStateMachine.sendMessage(
+ TetherInterfaceStateMachine.CMD_TETHER_UNREQUESTED);
return ConnectivityManager.TETHER_ERROR_NO_ERROR;
}
}
public void untetherAll() {
- if (DBG) Log.d(TAG, "Untethering " + mIfaces);
- for (String iface : mIfaces.keySet()) {
- untether(iface);
+ synchronized (mPublicSync) {
+ if (DBG) Log.d(TAG, "Untethering " + mTetherStates.keySet());
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ untether(mTetherStates.keyAt(i));
+ }
}
}
public int getLastTetherError(String iface) {
synchronized (mPublicSync) {
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm == null) {
+ TetherState tetherState = mTetherStates.get(iface);
+ if (tetherState == null) {
Log.e(TAG, "Tried to getLastTetherError on an unknown iface :" + iface +
", ignoring");
return ConnectivityManager.TETHER_ERROR_UNKNOWN_IFACE;
}
- return sm.getLastError();
+ return tetherState.mLastError;
}
}
- // TODO - move all private methods used only by the state machine into the state machine
- // to clarify what needs synchronized protection.
- @Override
- public void sendTetherStateChangedBroadcast() {
+ private void sendTetherStateChangedBroadcast() {
if (!getConnectivityManager().isTetheringSupported()) return;
ArrayList<String> availableList = new ArrayList<String>();
@@ -650,24 +653,22 @@
boolean bluetoothTethered = false;
synchronized (mPublicSync) {
- Set<String> ifaces = mIfaces.keySet();
- for (String iface : ifaces) {
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm != null) {
- if (sm.isErrored()) {
- erroredList.add(iface);
- } else if (sm.isAvailable()) {
- availableList.add(iface);
- } else if (sm.isTethered()) {
- if (isUsb(iface)) {
- usbTethered = true;
- } else if (isWifi(iface)) {
- wifiTethered = true;
- } else if (isBluetooth(iface)) {
- bluetoothTethered = true;
- }
- activeList.add(iface);
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ TetherState tetherState = mTetherStates.valueAt(i);
+ String iface = mTetherStates.keyAt(i);
+ if (tetherState.mLastError != ConnectivityManager.TETHER_ERROR_NO_ERROR) {
+ erroredList.add(iface);
+ } else if (tetherState.mLastState == IControlsTethering.STATE_AVAILABLE) {
+ availableList.add(iface);
+ } else if (tetherState.mLastState == IControlsTethering.STATE_TETHERED) {
+ if (isUsb(iface)) {
+ usbTethered = true;
+ } else if (isWifi(iface)) {
+ wifiTethered = true;
+ } else if (isBluetooth(iface)) {
+ bluetoothTethered = true;
}
+ activeList.add(iface);
}
}
}
@@ -961,37 +962,27 @@
public String[] getTetheredIfaces() {
ArrayList<String> list = new ArrayList<String>();
synchronized (mPublicSync) {
- Set<String> keys = mIfaces.keySet();
- for (String key : keys) {
- TetherInterfaceStateMachine sm = mIfaces.get(key);
- if (sm.isTethered()) {
- list.add(key);
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ TetherState tetherState = mTetherStates.valueAt(i);
+ if (tetherState.mLastState == IControlsTethering.STATE_TETHERED) {
+ list.add(mTetherStates.keyAt(i));
}
}
}
- String[] retVal = new String[list.size()];
- for (int i=0; i < list.size(); i++) {
- retVal[i] = list.get(i);
- }
- return retVal;
+ return list.toArray(new String[list.size()]);
}
public String[] getTetherableIfaces() {
ArrayList<String> list = new ArrayList<String>();
synchronized (mPublicSync) {
- Set<String> keys = mIfaces.keySet();
- for (String key : keys) {
- TetherInterfaceStateMachine sm = mIfaces.get(key);
- if (sm.isAvailable()) {
- list.add(key);
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ TetherState tetherState = mTetherStates.valueAt(i);
+ if (tetherState.mLastState == IControlsTethering.STATE_AVAILABLE) {
+ list.add(mTetherStates.keyAt(i));
}
}
}
- String[] retVal = new String[list.size()];
- for (int i=0; i < list.size(); i++) {
- retVal[i] = list.get(i);
- }
- return retVal;
+ return list.toArray(new String[list.size()]);
}
public String[] getTetheredDhcpRanges() {
@@ -1001,19 +992,14 @@
public String[] getErroredIfaces() {
ArrayList<String> list = new ArrayList<String>();
synchronized (mPublicSync) {
- Set<String> keys = mIfaces.keySet();
- for (String key : keys) {
- TetherInterfaceStateMachine sm = mIfaces.get(key);
- if (sm.isErrored()) {
- list.add(key);
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ TetherState tetherState = mTetherStates.valueAt(i);
+ if (tetherState.mLastError != ConnectivityManager.TETHER_ERROR_NO_ERROR) {
+ list.add(mTetherStates.keyAt(i));
}
}
}
- String[] retVal = new String[list.size()];
- for (int i= 0; i< list.size(); i++) {
- retVal[i] = list.get(i);
- }
- return retVal;
+ return list.toArray(new String[list.size()]);
}
private void maybeLogMessage(State state, int what) {
@@ -1143,6 +1129,18 @@
private State mStopTetheringErrorState;
private State mSetDnsForwardersErrorState;
+ // This list is a little subtle. It contains all the interfaces that currently are
+ // requesting tethering, regardless of whether these interfaces are still members of
+ // mTetherStates. This allows us to maintain the following predicates:
+ //
+ // 1) mTetherStates contains the set of all currently existing, tetherable, link state up
+ // interfaces.
+ // 2) mNotifyList contains all state machines that may have outstanding tethering state
+ // that needs to be torn down.
+ //
+ // Because we excise interfaces immediately from mTetherStates, we must maintain mNotifyList
+ // so that the garbage collector does not clean up the state machine before it has a chance
+ // to tear itself down.
private ArrayList<TetherInterfaceStateMachine> mNotifyList;
private int mMobileApnReserved = ConnectivityManager.TYPE_NONE;
@@ -1453,15 +1451,16 @@
config_mobile_hotspot_provision_app_no_ui).isEmpty() == false) {
ArrayList<Integer> tethered = new ArrayList<Integer>();
synchronized (mPublicSync) {
- Set<String> ifaces = mIfaces.keySet();
- for (String iface : ifaces) {
- TetherInterfaceStateMachine sm = mIfaces.get(iface);
- if (sm != null && sm.isTethered()) {
- int interfaceType = ifaceNameToType(iface);
- if (interfaceType !=
- ConnectivityManager.TETHERING_INVALID) {
- tethered.add(new Integer(interfaceType));
- }
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ TetherState tetherState = mTetherStates.valueAt(i);
+ if (tetherState.mLastState !=
+ IControlsTethering.STATE_TETHERED) {
+ continue; // Skip interfaces that aren't tethered.
+ }
+ String iface = mTetherStates.keyAt(i);
+ int interfaceType = ifaceNameToType(iface);
+ if (interfaceType != ConnectivityManager.TETHERING_INVALID) {
+ tethered.add(new Integer(interfaceType));
}
}
}
@@ -1488,9 +1487,6 @@
class InitialState extends TetherMasterUtilState {
@Override
- public void enter() {
- }
- @Override
public boolean processMessage(Message message) {
maybeLogMessage(this, message.what);
boolean retValue = true;
@@ -1498,16 +1494,15 @@
case CMD_TETHER_MODE_REQUESTED:
TetherInterfaceStateMachine who = (TetherInterfaceStateMachine)message.obj;
if (VDBG) Log.d(TAG, "Tether Mode requested by " + who);
- mNotifyList.add(who);
+ if (mNotifyList.indexOf(who) < 0) {
+ mNotifyList.add(who);
+ }
transitionTo(mTetherModeAliveState);
break;
case CMD_TETHER_MODE_UNREQUESTED:
who = (TetherInterfaceStateMachine)message.obj;
if (VDBG) Log.d(TAG, "Tether Mode unrequested by " + who);
- int index = mNotifyList.indexOf(who);
- if (index != -1) {
- mNotifyList.remove(who);
- }
+ mNotifyList.remove(who);
break;
default:
retValue = false;
@@ -1546,24 +1541,26 @@
case CMD_TETHER_MODE_REQUESTED:
TetherInterfaceStateMachine who = (TetherInterfaceStateMachine)message.obj;
if (VDBG) Log.d(TAG, "Tether Mode requested by " + who);
- mNotifyList.add(who);
+ if (mNotifyList.indexOf(who) < 0) {
+ mNotifyList.add(who);
+ }
who.sendMessage(TetherInterfaceStateMachine.CMD_TETHER_CONNECTION_CHANGED,
mCurrentUpstreamIface);
break;
case CMD_TETHER_MODE_UNREQUESTED:
who = (TetherInterfaceStateMachine)message.obj;
if (VDBG) Log.d(TAG, "Tether Mode unrequested by " + who);
- int index = mNotifyList.indexOf(who);
- if (index != -1) {
+ if (mNotifyList.remove(who)) {
if (DBG) Log.d(TAG, "TetherModeAlive removing notifyee " + who);
- mNotifyList.remove(index);
if (mNotifyList.isEmpty()) {
turnOffMasterTetherSettings(); // transitions appropriately
} else {
if (DBG) {
Log.d(TAG, "TetherModeAlive still has " + mNotifyList.size() +
" live requests:");
- for (Object o : mNotifyList) Log.d(TAG, " " + o);
+ for (TetherInterfaceStateMachine o : mNotifyList) {
+ Log.d(TAG, " " + o);
+ }
}
}
} else {
@@ -1623,8 +1620,7 @@
}
void notify(int msgType) {
mErrorNotification = msgType;
- for (Object o : mNotifyList) {
- TetherInterfaceStateMachine sm = (TetherInterfaceStateMachine)o;
+ for (TetherInterfaceStateMachine sm : mNotifyList) {
sm.sendMessage(msgType);
}
}
@@ -1707,8 +1703,26 @@
pw.println("Tether state:");
pw.increaseIndent();
- for (Object o : mIfaces.values()) {
- pw.println(o);
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ final String iface = mTetherStates.keyAt(i);
+ final TetherState tetherState = mTetherStates.valueAt(i);
+ pw.print(iface + " - ");
+
+ switch (tetherState.mLastState) {
+ case IControlsTethering.STATE_UNAVAILABLE:
+ pw.print("UnavailableState");
+ break;
+ case IControlsTethering.STATE_AVAILABLE:
+ pw.print("AvailableState");
+ break;
+ case IControlsTethering.STATE_TETHERED:
+ pw.print("TetheredState");
+ break;
+ default:
+ pw.print("UnknownState");
+ break;
+ }
+ pw.println(" - lastError = " + tetherState.mLastError);
}
pw.decreaseIndent();
}
@@ -1716,9 +1730,40 @@
}
@Override
- public void notifyInterfaceTetheringReadiness(boolean isReady,
- TetherInterfaceStateMachine who) {
- mTetherMasterSM.sendMessage((isReady) ? TetherMasterSM.CMD_TETHER_MODE_REQUESTED
- : TetherMasterSM.CMD_TETHER_MODE_UNREQUESTED, who);
+ public void notifyInterfaceStateChange(String iface, TetherInterfaceStateMachine who,
+ int state, int error) {
+ synchronized (mPublicSync) {
+ TetherState tetherState = mTetherStates.get(iface);
+ if (tetherState != null && tetherState.mStateMachine.equals(who)) {
+ tetherState.mLastState = state;
+ tetherState.mLastError = error;
+ } else {
+ if (DBG) Log.d(TAG, "got notification from stale iface " + iface);
+ }
+ }
+
+ if (DBG) {
+ Log.d(TAG, "iface " + iface + " notified that it was in state " + state +
+ " with error " + error);
+ }
+
+ switch (state) {
+ case IControlsTethering.STATE_UNAVAILABLE:
+ case IControlsTethering.STATE_AVAILABLE:
+ mTetherMasterSM.sendMessage(TetherMasterSM.CMD_TETHER_MODE_UNREQUESTED, who);
+ break;
+ case IControlsTethering.STATE_TETHERED:
+ mTetherMasterSM.sendMessage(TetherMasterSM.CMD_TETHER_MODE_REQUESTED, who);
+ break;
+ }
+ sendTetherStateChangedBroadcast();
+ }
+
+ private void trackNewTetherableInterface(String iface, int interfaceType) {
+ TetherState tetherState;
+ tetherState = new TetherState(new TetherInterfaceStateMachine(iface, mLooper,
+ interfaceType, mNMService, mStatsService, this));
+ mTetherStates.put(iface, tetherState);
+ tetherState.mStateMachine.start();
}
}
diff --git a/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java b/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
index 7677daf..449b8a8 100644
--- a/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
+++ b/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
@@ -22,6 +22,18 @@
* Interface with methods necessary to notify that a given interface is ready for tethering.
*/
public interface IControlsTethering {
- void sendTetherStateChangedBroadcast();
- void notifyInterfaceTetheringReadiness(boolean isReady, TetherInterfaceStateMachine who);
+ public final int STATE_UNAVAILABLE = 0;
+ public final int STATE_AVAILABLE = 1;
+ public final int STATE_TETHERED = 2;
+
+ /**
+ * Notify that |who| has changed its tethering state. This may be called from any thread.
+ *
+ * @param iface a network interface (e.g. "wlan0")
+ * @param who corresponding instance of a TetherInterfaceStateMachine
+ * @param state one of IControlsTethering.STATE_*
+ * @param lastError one of ConnectivityManager.TETHER_ERROR_*
+ */
+ void notifyInterfaceStateChange(String iface, TetherInterfaceStateMachine who,
+ int state, int lastError);
}
diff --git a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
index b8bea60..aebeb69 100644
--- a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
+++ b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
@@ -27,7 +27,6 @@
import android.util.Log;
import android.util.SparseArray;
-import com.android.internal.util.IState;
import com.android.internal.util.MessageUtils;
import com.android.internal.util.Protocol;
import com.android.internal.util.State;
@@ -98,7 +97,7 @@
mTetherController = tetherController;
mIfaceName = ifaceName;
mInterfaceType = interfaceType;
- setLastError(ConnectivityManager.TETHER_ERROR_NO_ERROR);
+ mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
mInitialState = new InitialState();
addState(mInitialState);
@@ -110,40 +109,6 @@
setInitialState(mInitialState);
}
- @Override
- public String toString() {
- String res = new String();
- res += mIfaceName + " - ";
- IState current = getCurrentState();
- if (current == mInitialState) res += "InitialState";
- if (current == mTetheredState) res += "TetheredState";
- if (current == mUnavailableState) res += "UnavailableState";
- if (isAvailable()) res += " - Available";
- if (isTethered()) res += " - Tethered";
- res += " - lastError =" + getLastError();
- return res;
- }
-
- public int getLastError() {
- return mLastError;
- }
-
- private void setLastError(int error) {
- mLastError = error;
- }
-
- public boolean isAvailable() {
- return getCurrentState() == mInitialState;
- }
-
- public boolean isTethered() {
- return getCurrentState() == mTetheredState;
- }
-
- public boolean isErrored() {
- return (mLastError != ConnectivityManager.TETHER_ERROR_NO_ERROR);
- }
-
// configured when we start tethering and unconfig'd on error or conclusion
private boolean configureIfaceIp(boolean enabled) {
if (VDBG) Log.d(TAG, "configureIfaceIp(" + enabled + ")");
@@ -193,7 +158,9 @@
class InitialState extends State {
@Override
public void enter() {
- mTetherController.sendTetherStateChangedBroadcast();
+ mTetherController.notifyInterfaceStateChange(
+ mIfaceName, TetherInterfaceStateMachine.this,
+ IControlsTethering.STATE_AVAILABLE, mLastError);
}
@Override
@@ -202,8 +169,7 @@
boolean retValue = true;
switch (message.what) {
case CMD_TETHER_REQUESTED:
- setLastError(ConnectivityManager.TETHER_ERROR_NO_ERROR);
- mTetherController.notifyInterfaceTetheringReadiness(true, TetherInterfaceStateMachine.this);
+ mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
transitionTo(mTetheredState);
break;
case CMD_INTERFACE_DOWN:
@@ -221,7 +187,7 @@
@Override
public void enter() {
if (!configureIfaceIp(true)) {
- setLastError(ConnectivityManager.TETHER_ERROR_IFACE_CFG_ERROR);
+ mLastError = ConnectivityManager.TETHER_ERROR_IFACE_CFG_ERROR;
transitionTo(mInitialState);
return;
}
@@ -230,19 +196,18 @@
mNMService.tetherInterface(mIfaceName);
} catch (Exception e) {
Log.e(TAG, "Error Tethering: " + e.toString());
- setLastError(ConnectivityManager.TETHER_ERROR_TETHER_IFACE_ERROR);
+ mLastError = ConnectivityManager.TETHER_ERROR_TETHER_IFACE_ERROR;
transitionTo(mInitialState);
return;
}
if (DBG) Log.d(TAG, "Tethered " + mIfaceName);
- mTetherController.sendTetherStateChangedBroadcast();
+ mTetherController.notifyInterfaceStateChange(
+ mIfaceName, TetherInterfaceStateMachine.this,
+ IControlsTethering.STATE_TETHERED, mLastError);
}
@Override
public void exit() {
- mTetherController.notifyInterfaceTetheringReadiness(false,
- TetherInterfaceStateMachine.this);
-
// Note that at this point, we're leaving the tethered state. We can fail any
// of these operations, but it doesn't really change that we have to try them
// all in sequence.
@@ -251,7 +216,7 @@
try {
mNMService.untetherInterface(mIfaceName);
} catch (Exception ee) {
- setLastError(ConnectivityManager.TETHER_ERROR_UNTETHER_IFACE_ERROR);
+ mLastError = ConnectivityManager.TETHER_ERROR_UNTETHER_IFACE_ERROR;
Log.e(TAG, "Failed to untether interface: " + ee.toString());
}
@@ -315,7 +280,7 @@
newUpstreamIfaceName);
} catch (Exception e) {
Log.e(TAG, "Exception enabling Nat: " + e.toString());
- setLastError(ConnectivityManager.TETHER_ERROR_ENABLE_NAT_ERROR);
+ mLastError = ConnectivityManager.TETHER_ERROR_ENABLE_NAT_ERROR;
transitionTo(mInitialState);
return true;
}
@@ -327,8 +292,8 @@
case CMD_START_TETHERING_ERROR:
case CMD_STOP_TETHERING_ERROR:
case CMD_SET_DNS_FORWARDERS_ERROR:
- setLastErrorAndTransitionToInitialState(
- ConnectivityManager.TETHER_ERROR_MASTER_ERROR);
+ mLastError = ConnectivityManager.TETHER_ERROR_MASTER_ERROR;
+ transitionTo(mInitialState);
break;
default:
retValue = false;
@@ -348,13 +313,10 @@
class UnavailableState extends State {
@Override
public void enter() {
- setLastError(ConnectivityManager.TETHER_ERROR_NO_ERROR);
- mTetherController.sendTetherStateChangedBroadcast();
+ mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
+ mTetherController.notifyInterfaceStateChange(
+ mIfaceName, TetherInterfaceStateMachine.this,
+ IControlsTethering.STATE_UNAVAILABLE, mLastError);
}
}
-
- void setLastErrorAndTransitionToInitialState(int error) {
- setLastError(error);
- transitionTo(mInitialState);
- }
}
diff --git a/services/tests/servicestests/src/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java b/services/tests/servicestests/src/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java
index 30a7dbc..a30b362 100644
--- a/services/tests/servicestests/src/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java
+++ b/services/tests/servicestests/src/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java
@@ -16,8 +16,6 @@
package com.android.server.connectivity.tethering;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.inOrder;
@@ -26,6 +24,14 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
+import static android.net.ConnectivityManager.TETHER_ERROR_ENABLE_NAT_ERROR;
+import static android.net.ConnectivityManager.TETHER_ERROR_NO_ERROR;
+import static android.net.ConnectivityManager.TETHER_ERROR_TETHER_IFACE_ERROR;
+import static android.net.ConnectivityManager.TETHER_ERROR_UNTETHER_IFACE_ERROR;
+import static com.android.server.connectivity.tethering.IControlsTethering.STATE_AVAILABLE;
+import static com.android.server.connectivity.tethering.IControlsTethering.STATE_TETHERED;
+import static com.android.server.connectivity.tethering.IControlsTethering.STATE_UNAVAILABLE;
+
import android.net.ConnectivityManager;
import android.net.INetworkStatsService;
import android.net.InterfaceConfiguration;
@@ -78,8 +84,7 @@
when(mNMService.getInterfaceConfig(IFACE_NAME)).thenReturn(mInterfaceConfiguration);
}
- @Before
- public void setUp() throws Exception {
+ @Before public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
}
@@ -89,10 +94,8 @@
ConnectivityManager.TETHERING_BLUETOOTH, mNMService, mStatsService, mTetherHelper);
mTestedSm.start();
mLooper.dispatchAll();
- assertTrue("Should start out available for tethering", mTestedSm.isAvailable());
- assertFalse("Should not be tethered initially", mTestedSm.isTethered());
- assertFalse("Should have no errors initially", mTestedSm.isErrored());
- verify(mTetherHelper).sendTetherStateChangedBroadcast();
+ verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
verifyNoMoreInteractions(mTetherHelper, mNMService, mStatsService);
}
@@ -119,28 +122,23 @@
@Test
public void handlesImmediateInterfaceDown() throws Exception {
initStateMachine(ConnectivityManager.TETHERING_BLUETOOTH);
+
dispatchCommand(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
- verify(mTetherHelper).sendTetherStateChangedBroadcast();
+ verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertFalse("Should not be tetherable when the interface is down", mTestedSm.isAvailable());
- assertFalse("Should not be tethered when the interface is down", mTestedSm.isTethered());
- assertFalse("Should have no errors when the interface goes immediately down",
- mTestedSm.isErrored());
}
@Test
public void canBeTethered() throws Exception {
initStateMachine(ConnectivityManager.TETHERING_BLUETOOTH);
+
dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_REQUESTED);
InOrder inOrder = inOrder(mTetherHelper, mNMService);
- inOrder.verify(mTetherHelper).notifyInterfaceTetheringReadiness(true, mTestedSm);
inOrder.verify(mNMService).tetherInterface(IFACE_NAME);
- inOrder.verify(mTetherHelper).sendTetherStateChangedBroadcast();
-
+ inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertFalse("Should not be tetherable when tethered", mTestedSm.isAvailable());
- assertTrue("Should be in a tethered state", mTestedSm.isTethered());
- assertFalse("Should have no errors when tethered", mTestedSm.isErrored());
}
@Test
@@ -149,13 +147,10 @@
dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_UNREQUESTED);
InOrder inOrder = inOrder(mNMService, mStatsService, mTetherHelper);
- inOrder.verify(mTetherHelper).notifyInterfaceTetheringReadiness(false, mTestedSm);
inOrder.verify(mNMService).untetherInterface(IFACE_NAME);
- inOrder.verify(mTetherHelper).sendTetherStateChangedBroadcast();
+ inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertTrue("Should be ready for tethering again", mTestedSm.isAvailable());
- assertFalse("Should not be tethered", mTestedSm.isTethered());
- assertFalse("Should have no errors", mTestedSm.isErrored());
}
@Test
@@ -164,16 +159,12 @@
dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_REQUESTED);
InOrder inOrder = inOrder(mTetherHelper, mNMService);
- inOrder.verify(mTetherHelper).notifyInterfaceTetheringReadiness(true, mTestedSm);
inOrder.verify(mNMService).getInterfaceConfig(IFACE_NAME);
inOrder.verify(mNMService).setInterfaceConfig(IFACE_NAME, mInterfaceConfiguration);
inOrder.verify(mNMService).tetherInterface(IFACE_NAME);
- inOrder.verify(mTetherHelper).sendTetherStateChangedBroadcast();
-
+ inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertFalse("Should not be tetherable when tethered", mTestedSm.isAvailable());
- assertTrue("Should be in a tethered state", mTestedSm.isTethered());
- assertFalse("Should have no errors when tethered", mTestedSm.isErrored());
}
@Test
@@ -186,9 +177,6 @@
inOrder.verify(mNMService).enableNat(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNMService).startInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertFalse("Should not be tetherable when tethered", mTestedSm.isAvailable());
- assertTrue("Should be in a tethered state", mTestedSm.isTethered());
- assertFalse("Should have no errors when tethered", mTestedSm.isErrored());
}
@Test
@@ -203,9 +191,6 @@
inOrder.verify(mNMService).enableNat(IFACE_NAME, UPSTREAM_IFACE2);
inOrder.verify(mNMService).startInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE2);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertFalse("Should not be tetherable when tethered", mTestedSm.isAvailable());
- assertTrue("Should be in a tethered state", mTestedSm.isTethered());
- assertFalse("Should have no errors when tethered", mTestedSm.isErrored());
}
@Test
@@ -214,16 +199,13 @@
dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_UNREQUESTED);
InOrder inOrder = inOrder(mNMService, mStatsService, mTetherHelper);
- inOrder.verify(mTetherHelper).notifyInterfaceTetheringReadiness(false, mTestedSm);
inOrder.verify(mStatsService).forceUpdate();
inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE);
inOrder.verify(mNMService).untetherInterface(IFACE_NAME);
- inOrder.verify(mTetherHelper).sendTetherStateChangedBroadcast();
+ inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
- assertTrue("Should be ready for tethering again", mTestedSm.isAvailable());
- assertFalse("Should not be tethered", mTestedSm.isTethered());
- assertFalse("Should have no errors", mTestedSm.isErrored());
}
@Test
@@ -235,13 +217,12 @@
doThrow(RemoteException.class).when(mNMService).untetherInterface(IFACE_NAME);
}
dispatchCommand(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
- InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration);
+ InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration, mTetherHelper);
usbTeardownOrder.verify(mInterfaceConfiguration).setInterfaceDown();
usbTeardownOrder.verify(mNMService).setInterfaceConfig(
IFACE_NAME, mInterfaceConfiguration);
- verify(mTetherHelper).notifyInterfaceTetheringReadiness(false, mTestedSm);
- assertFalse("Should not be available", mTestedSm.isAvailable());
- assertFalse("Should not be tethered", mTestedSm.isTethered());
+ usbTeardownOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
}
}
@@ -251,15 +232,12 @@
doThrow(RemoteException.class).when(mNMService).tetherInterface(IFACE_NAME);
dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_REQUESTED);
- InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration);
+ InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration, mTetherHelper);
usbTeardownOrder.verify(mInterfaceConfiguration).setInterfaceDown();
usbTeardownOrder.verify(mNMService).setInterfaceConfig(
IFACE_NAME, mInterfaceConfiguration);
- // Initial call is when we transition to the tethered state on request.
- verify(mTetherHelper).notifyInterfaceTetheringReadiness(true, mTestedSm);
- // And this call is to notify that we really aren't requested tethering.
- verify(mTetherHelper).notifyInterfaceTetheringReadiness(false, mTestedSm);
- assertTrue("Expected to see an error reported", mTestedSm.isErrored());
+ usbTeardownOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_TETHER_IFACE_ERROR);
}
@Test
@@ -268,10 +246,11 @@
doThrow(RemoteException.class).when(mNMService).enableNat(anyString(), anyString());
dispatchTetherConnectionChanged(UPSTREAM_IFACE);
- InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration);
+ InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration, mTetherHelper);
usbTeardownOrder.verify(mInterfaceConfiguration).setInterfaceDown();
usbTeardownOrder.verify(mNMService).setInterfaceConfig(IFACE_NAME, mInterfaceConfiguration);
- verify(mTetherHelper).notifyInterfaceTetheringReadiness(false, mTestedSm);
+ usbTeardownOrder.verify(mTetherHelper).notifyInterfaceStateChange(
+ IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_ENABLE_NAT_ERROR);
}
/**