diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index d2e726e023..1311e7b461 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -128,7 +128,6 @@ import android.util.SparseArray; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.MessageUtils; @@ -218,9 +217,6 @@ public class Tethering { private final SparseArray mActiveTetheringRequests = new SparseArray<>(); - // used to synchronize public access to members - // TODO(b/153621704): remove mPublicSync to make Tethering lock free - private final Object mPublicSync; private final Context mContext; private final ArrayMap mTetherStates; private final BroadcastReceiver mStateReceiver; @@ -246,8 +242,6 @@ public class Tethering { private final BpfCoordinator mBpfCoordinator; private final PrivateAddressCoordinator mPrivateAddressCoordinator; private int mActiveDataSubId = INVALID_SUBSCRIPTION_ID; - // All the usage of mTetheringEventCallback should run in the same thread. - private ITetheringEventCallback mTetheringEventCallback = null; private volatile TetheringConfiguration mConfig; private InterfaceSet mCurrentUpstreamIfaceSet; @@ -261,11 +255,8 @@ public class Tethering { private String mWifiP2pTetherInterface = null; private int mOffloadStatus = TETHER_HARDWARE_OFFLOAD_STOPPED; - @GuardedBy("mPublicSync") private EthernetManager.TetheredInterfaceRequest mEthernetIfaceRequest; - @GuardedBy("mPublicSync") private String mConfiguredEthernetIface; - @GuardedBy("mPublicSync") private EthernetCallback mEthernetCallback; public Tethering(TetheringDependencies deps) { @@ -276,8 +267,6 @@ public class Tethering { mLooper = mDeps.getTetheringLooper(); mNotificationUpdater = mDeps.getNotificationUpdater(mContext, mLooper); - mPublicSync = new Object(); - mTetherStates = new ArrayMap<>(); mConnectedClientsTracker = new ConnectedClientsTracker(); @@ -504,20 +493,18 @@ public class Tethering { // Never called directly: only called from interfaceLinkStateChanged. // See NetlinkHandler.cpp: notifyInterfaceChanged. if (VDBG) Log.d(TAG, "interfaceStatusChanged " + iface + ", " + up); - synchronized (mPublicSync) { - if (up) { - maybeTrackNewInterfaceLocked(iface); + if (up) { + maybeTrackNewInterfaceLocked(iface); + } else { + if (ifaceNameToType(iface) == TETHERING_BLUETOOTH + || ifaceNameToType(iface) == TETHERING_WIGIG) { + stopTrackingInterfaceLocked(iface); } else { - if (ifaceNameToType(iface) == TETHERING_BLUETOOTH - || ifaceNameToType(iface) == TETHERING_WIGIG) { - stopTrackingInterfaceLocked(iface); - } else { - // Ignore usb0 down after enabling RNDIS. - // We will handle disconnect in interfaceRemoved. - // Similarly, ignore interface down for WiFi. We monitor WiFi AP status - // through the WifiManager.WIFI_AP_STATE_CHANGED_ACTION intent. - if (VDBG) Log.d(TAG, "ignore interface down for " + iface); - } + // Ignore usb0 down after enabling RNDIS. + // We will handle disconnect in interfaceRemoved. + // Similarly, ignore interface down for WiFi. We monitor WiFi AP status + // through the WifiManager.WIFI_AP_STATE_CHANGED_ACTION intent. + if (VDBG) Log.d(TAG, "ignore interface down for " + iface); } } } @@ -547,16 +534,12 @@ public class Tethering { void interfaceAdded(String iface) { if (VDBG) Log.d(TAG, "interfaceAdded " + iface); - synchronized (mPublicSync) { - maybeTrackNewInterfaceLocked(iface); - } + maybeTrackNewInterfaceLocked(iface); } void interfaceRemoved(String iface) { if (VDBG) Log.d(TAG, "interfaceRemoved " + iface); - synchronized (mPublicSync) { - stopTrackingInterfaceLocked(iface); - } + stopTrackingInterfaceLocked(iface); } void startTethering(final TetheringRequestParcel request, final IIntResultListener listener) { @@ -641,17 +624,15 @@ public class Tethering { private int setWifiTethering(final boolean enable) { final long ident = Binder.clearCallingIdentity(); try { - synchronized (mPublicSync) { - final WifiManager mgr = getWifiManager(); - if (mgr == null) { - mLog.e("setWifiTethering: failed to get WifiManager!"); - return TETHER_ERROR_SERVICE_UNAVAIL; - } - if ((enable && mgr.startTetheredHotspot(null /* use existing softap config */)) - || (!enable && mgr.stopSoftAp())) { - mWifiTetherRequested = enable; - return TETHER_ERROR_NO_ERROR; - } + final WifiManager mgr = getWifiManager(); + if (mgr == null) { + mLog.e("setWifiTethering: failed to get WifiManager!"); + return TETHER_ERROR_SERVICE_UNAVAIL; + } + if ((enable && mgr.startTetheredHotspot(null /* use existing softap config */)) + || (!enable && mgr.stopSoftAp())) { + mWifiTetherRequested = enable; + return TETHER_ERROR_NO_ERROR; } } finally { Binder.restoreCallingIdentity(ident); @@ -703,18 +684,16 @@ public class Tethering { private int setEthernetTethering(final boolean enable) { final EthernetManager em = (EthernetManager) mContext.getSystemService( Context.ETHERNET_SERVICE); - synchronized (mPublicSync) { - if (enable) { - if (mEthernetCallback != null) { - Log.d(TAG, "Ethernet tethering already started"); - return TETHER_ERROR_NO_ERROR; - } - - mEthernetCallback = new EthernetCallback(); - mEthernetIfaceRequest = em.requestTetheredInterface(mExecutor, mEthernetCallback); - } else { - stopEthernetTetheringLocked(); + if (enable) { + if (mEthernetCallback != null) { + Log.d(TAG, "Ethernet tethering already started"); + return TETHER_ERROR_NO_ERROR; } + + mEthernetCallback = new EthernetCallback(); + mEthernetIfaceRequest = em.requestTetheredInterface(mExecutor, mEthernetCallback); + } else { + stopEthernetTetheringLocked(); } return TETHER_ERROR_NO_ERROR; } @@ -734,26 +713,22 @@ public class Tethering { private class EthernetCallback implements EthernetManager.TetheredInterfaceCallback { @Override public void onAvailable(String iface) { - synchronized (mPublicSync) { - if (this != mEthernetCallback) { - // Ethernet callback arrived after Ethernet tethering stopped. Ignore. - return; - } - maybeTrackNewInterfaceLocked(iface, TETHERING_ETHERNET); - changeInterfaceState(iface, getRequestedState(TETHERING_ETHERNET)); - mConfiguredEthernetIface = iface; + if (this != mEthernetCallback) { + // Ethernet callback arrived after Ethernet tethering stopped. Ignore. + return; } + maybeTrackNewInterfaceLocked(iface, TETHERING_ETHERNET); + changeInterfaceState(iface, getRequestedState(TETHERING_ETHERNET)); + mConfiguredEthernetIface = iface; } @Override public void onUnavailable() { - synchronized (mPublicSync) { - if (this != mEthernetCallback) { - // onAvailable called after stopping Ethernet tethering. - return; - } - stopEthernetTetheringLocked(); + if (this != mEthernetCallback) { + // onAvailable called after stopping Ethernet tethering. + return; } + stopEthernetTetheringLocked(); } } @@ -767,57 +742,53 @@ public class Tethering { private int tether(String iface, int requestedState) { if (DBG) Log.d(TAG, "Tethering " + iface); - synchronized (mPublicSync) { - TetherState tetherState = mTetherStates.get(iface); - if (tetherState == null) { - Log.e(TAG, "Tried to Tether an unknown iface: " + iface + ", ignoring"); - return 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 (tetherState.lastState != IpServer.STATE_AVAILABLE) { - Log.e(TAG, "Tried to Tether an unavailable iface: " + iface + ", ignoring"); - return TETHER_ERROR_UNAVAIL_IFACE; - } - // NOTE: If a CMD_TETHER_REQUESTED message is already in the TISM's queue but not yet - // processed, this will be a no-op and it will not return an error. - // - // This code cannot race with untether() because they both synchronize on mPublicSync. - // TODO: reexamine the threading and messaging model to totally remove mPublicSync. - final int type = tetherState.ipServer.interfaceType(); - final TetheringRequestParcel request = mActiveTetheringRequests.get(type, null); - if (request != null) { - mActiveTetheringRequests.delete(type); - } - tetherState.ipServer.sendMessage(IpServer.CMD_TETHER_REQUESTED, requestedState, 0, - request); - return TETHER_ERROR_NO_ERROR; + TetherState tetherState = mTetherStates.get(iface); + if (tetherState == null) { + Log.e(TAG, "Tried to Tether an unknown iface: " + iface + ", ignoring"); + return 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 (tetherState.lastState != IpServer.STATE_AVAILABLE) { + Log.e(TAG, "Tried to Tether an unavailable iface: " + iface + ", ignoring"); + return TETHER_ERROR_UNAVAIL_IFACE; + } + // NOTE: If a CMD_TETHER_REQUESTED message is already in the TISM's queue but not yet + // processed, this will be a no-op and it will not return an error. + // + // This code cannot race with untether() because they both run on the handler thread. + final int type = tetherState.ipServer.interfaceType(); + final TetheringRequestParcel request = mActiveTetheringRequests.get(type, null); + if (request != null) { + mActiveTetheringRequests.delete(type); + } + tetherState.ipServer.sendMessage(IpServer.CMD_TETHER_REQUESTED, requestedState, 0, + request); + return TETHER_ERROR_NO_ERROR; } void untether(String iface, final IIntResultListener listener) { mHandler.post(() -> { try { listener.onResult(untether(iface)); - } catch (RemoteException e) { } + } catch (RemoteException e) { + } }); } int untether(String iface) { if (DBG) Log.d(TAG, "Untethering " + iface); - synchronized (mPublicSync) { - TetherState tetherState = mTetherStates.get(iface); - if (tetherState == null) { - Log.e(TAG, "Tried to Untether an unknown iface :" + iface + ", ignoring"); - return TETHER_ERROR_UNKNOWN_IFACE; - } - if (!tetherState.isCurrentlyServing()) { - Log.e(TAG, "Tried to untether an inactive iface :" + iface + ", ignoring"); - return TETHER_ERROR_UNAVAIL_IFACE; - } - tetherState.ipServer.sendMessage(IpServer.CMD_TETHER_UNREQUESTED); - return TETHER_ERROR_NO_ERROR; + TetherState tetherState = mTetherStates.get(iface); + if (tetherState == null) { + Log.e(TAG, "Tried to Untether an unknown iface :" + iface + ", ignoring"); + return TETHER_ERROR_UNKNOWN_IFACE; } + if (!tetherState.isCurrentlyServing()) { + Log.e(TAG, "Tried to untether an inactive iface :" + iface + ", ignoring"); + return TETHER_ERROR_UNAVAIL_IFACE; + } + tetherState.ipServer.sendMessage(IpServer.CMD_TETHER_UNREQUESTED); + return TETHER_ERROR_NO_ERROR; } void untetherAll() { @@ -828,16 +799,15 @@ public class Tethering { stopTethering(TETHERING_ETHERNET); } - int getLastTetherError(String iface) { - synchronized (mPublicSync) { - TetherState tetherState = mTetherStates.get(iface); - if (tetherState == null) { - Log.e(TAG, "Tried to getLastTetherError on an unknown iface :" + iface - + ", ignoring"); - return TETHER_ERROR_UNKNOWN_IFACE; - } - return tetherState.lastError; + @VisibleForTesting + int getLastErrorForTest(String iface) { + TetherState tetherState = mTetherStates.get(iface); + if (tetherState == null) { + Log.e(TAG, "Tried to getLastErrorForTest on an unknown iface :" + iface + + ", ignoring"); + return TETHER_ERROR_UNKNOWN_IFACE; } + return tetherState.lastError; } private boolean isProvisioningNeededButUnavailable() { @@ -894,27 +864,25 @@ public class Tethering { mTetherStatesParcel = new TetherStatesParcel(); int downstreamTypesMask = DOWNSTREAM_NONE; - synchronized (mPublicSync) { - for (int i = 0; i < mTetherStates.size(); i++) { - TetherState tetherState = mTetherStates.valueAt(i); - String iface = mTetherStates.keyAt(i); - if (tetherState.lastError != TETHER_ERROR_NO_ERROR) { - erroredList.add(iface); - lastErrorList.add(tetherState.lastError); - } else if (tetherState.lastState == IpServer.STATE_AVAILABLE) { - availableList.add(iface); - } else if (tetherState.lastState == IpServer.STATE_LOCAL_ONLY) { - localOnlyList.add(iface); - } else if (tetherState.lastState == IpServer.STATE_TETHERED) { - if (cfg.isUsb(iface)) { - downstreamTypesMask |= (1 << TETHERING_USB); - } else if (cfg.isWifi(iface)) { - downstreamTypesMask |= (1 << TETHERING_WIFI); - } else if (cfg.isBluetooth(iface)) { - downstreamTypesMask |= (1 << TETHERING_BLUETOOTH); - } - tetherList.add(iface); + for (int i = 0; i < mTetherStates.size(); i++) { + TetherState tetherState = mTetherStates.valueAt(i); + String iface = mTetherStates.keyAt(i); + if (tetherState.lastError != TETHER_ERROR_NO_ERROR) { + erroredList.add(iface); + lastErrorList.add(tetherState.lastError); + } else if (tetherState.lastState == IpServer.STATE_AVAILABLE) { + availableList.add(iface); + } else if (tetherState.lastState == IpServer.STATE_LOCAL_ONLY) { + localOnlyList.add(iface); + } else if (tetherState.lastState == IpServer.STATE_TETHERED) { + if (cfg.isUsb(iface)) { + downstreamTypesMask |= (1 << TETHERING_USB); + } else if (cfg.isWifi(iface)) { + downstreamTypesMask |= (1 << TETHERING_WIFI); + } else if (cfg.isBluetooth(iface)) { + downstreamTypesMask |= (1 << TETHERING_BLUETOOTH); } + tetherList.add(iface); } } @@ -1012,21 +980,19 @@ public class Tethering { // functions are ready to use. // // For more explanation, see b/62552150 . - synchronized (Tethering.this.mPublicSync) { - if (!usbConnected && mRndisEnabled) { - // Turn off tethering if it was enabled and there is a disconnect. - tetherMatchingInterfaces(IpServer.STATE_AVAILABLE, TETHERING_USB); - mEntitlementMgr.stopProvisioningIfNeeded(TETHERING_USB); - } else if (usbConfigured && rndisEnabled) { - // Tether if rndis is enabled and usb is configured. - final int state = getRequestedState(TETHERING_USB); - tetherMatchingInterfaces(state, TETHERING_USB); - } else if (usbConnected && ncmEnabled) { - final int state = getRequestedState(TETHERING_NCM); - tetherMatchingInterfaces(state, TETHERING_NCM); - } - mRndisEnabled = usbConfigured && rndisEnabled; + if (!usbConnected && mRndisEnabled) { + // Turn off tethering if it was enabled and there is a disconnect. + tetherMatchingInterfaces(IpServer.STATE_AVAILABLE, TETHERING_USB); + mEntitlementMgr.stopProvisioningIfNeeded(TETHERING_USB); + } else if (usbConfigured && rndisEnabled) { + // Tether if rndis is enabled and usb is configured. + final int state = getRequestedState(TETHERING_USB); + tetherMatchingInterfaces(state, TETHERING_USB); + } else if (usbConnected && ncmEnabled) { + final int state = getRequestedState(TETHERING_NCM); + tetherMatchingInterfaces(state, TETHERING_NCM); } + mRndisEnabled = usbConfigured && rndisEnabled; } private void handleWifiApAction(Intent intent) { @@ -1034,23 +1000,21 @@ public class Tethering { final String ifname = intent.getStringExtra(EXTRA_WIFI_AP_INTERFACE_NAME); final int ipmode = intent.getIntExtra(EXTRA_WIFI_AP_MODE, IFACE_IP_MODE_UNSPECIFIED); - synchronized (Tethering.this.mPublicSync) { - switch (curState) { - case WifiManager.WIFI_AP_STATE_ENABLING: - // We can see this state on the way to both enabled and failure states. - break; - case WifiManager.WIFI_AP_STATE_ENABLED: - enableWifiIpServingLocked(ifname, ipmode); - break; - case WifiManager.WIFI_AP_STATE_DISABLING: - // We can see this state on the way to disabled. - break; - case WifiManager.WIFI_AP_STATE_DISABLED: - case WifiManager.WIFI_AP_STATE_FAILED: - default: - disableWifiIpServingLocked(ifname, curState); - break; - } + switch (curState) { + case WifiManager.WIFI_AP_STATE_ENABLING: + // We can see this state on the way to both enabled and failure states. + break; + case WifiManager.WIFI_AP_STATE_ENABLED: + enableWifiIpServingLocked(ifname, ipmode); + break; + case WifiManager.WIFI_AP_STATE_DISABLING: + // We can see this state on the way to disabled. + break; + case WifiManager.WIFI_AP_STATE_DISABLED: + case WifiManager.WIFI_AP_STATE_FAILED: + default: + disableWifiIpServingLocked(ifname, curState); + break; } } @@ -1071,32 +1035,30 @@ public class Tethering { Log.d(TAG, "WifiP2pAction: P2pInfo: " + p2pInfo + " Group: " + group); } - synchronized (Tethering.this.mPublicSync) { - // if no group is formed, bring it down if needed. - if (p2pInfo == null || !p2pInfo.groupFormed) { - disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); - mWifiP2pTetherInterface = null; - return; - } - - // If there is a group but the device is not the owner, bail out. - if (!isGroupOwner(group)) return; - - // If already serving from the correct interface, nothing to do. - if (group.getInterface().equals(mWifiP2pTetherInterface)) return; - - // If already serving from another interface, turn it down first. - if (!TextUtils.isEmpty(mWifiP2pTetherInterface)) { - mLog.w("P2P tethered interface " + mWifiP2pTetherInterface - + "is different from current interface " - + group.getInterface() + ", re-tether it"); - disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); - } - - // Finally bring up serving on the new interface - mWifiP2pTetherInterface = group.getInterface(); - enableWifiIpServingLocked(mWifiP2pTetherInterface, IFACE_IP_MODE_LOCAL_ONLY); + // if no group is formed, bring it down if needed. + if (p2pInfo == null || !p2pInfo.groupFormed) { + disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); + mWifiP2pTetherInterface = null; + return; } + + // If there is a group but the device is not the owner, bail out. + if (!isGroupOwner(group)) return; + + // If already serving from the correct interface, nothing to do. + if (group.getInterface().equals(mWifiP2pTetherInterface)) return; + + // If already serving from another interface, turn it down first. + if (!TextUtils.isEmpty(mWifiP2pTetherInterface)) { + mLog.w("P2P tethered interface " + mWifiP2pTetherInterface + + "is different from current interface " + + group.getInterface() + ", re-tether it"); + disableWifiP2pIpServingLockedIfNeeded(mWifiP2pTetherInterface); + } + + // Finally bring up serving on the new interface + mWifiP2pTetherInterface = group.getInterface(); + enableWifiIpServingLocked(mWifiP2pTetherInterface, IFACE_IP_MODE_LOCAL_ONLY); } private void handleUserRestrictionAction() { @@ -1131,14 +1093,14 @@ public class Tethering { @VisibleForTesting protected static class UserRestrictionActionListener { private final UserManager mUserMgr; - private final Tethering mWrapper; + private final Tethering mTethering; private final TetheringNotificationUpdater mNotificationUpdater; public boolean mDisallowTethering; - public UserRestrictionActionListener(@NonNull UserManager um, @NonNull Tethering wrapper, + public UserRestrictionActionListener(@NonNull UserManager um, @NonNull Tethering tethering, @NonNull TetheringNotificationUpdater updater) { mUserMgr = um; - mWrapper = wrapper; + mTethering = tethering; mNotificationUpdater = updater; mDisallowTethering = false; } @@ -1165,13 +1127,13 @@ public class Tethering { return; } - if (mWrapper.isTetheringActive()) { + if (mTethering.isTetheringActive()) { // Restricted notification is shown when tethering function is disallowed on // user's device. mNotificationUpdater.notifyTetheringDisabledByRestriction(); // Untether from all downstreams since tethering is disallowed. - mWrapper.untetherAll(); + mTethering.untetherAll(); } // TODO(b/148139325): send tetheringSupported on restriction change } @@ -1332,53 +1294,51 @@ public class Tethering { return copy(mConfig.tetherableBluetoothRegexs); } - int setUsbTethering(boolean enable) { + void setUsbTethering(boolean enable, IIntResultListener listener) { + mHandler.post(() -> { + try { + listener.onResult(setUsbTethering(enable)); + } catch (RemoteException e) { } + }); + } + + private int setUsbTethering(boolean enable) { if (VDBG) Log.d(TAG, "setUsbTethering(" + enable + ")"); UsbManager usbManager = (UsbManager) mContext.getSystemService(Context.USB_SERVICE); if (usbManager == null) { mLog.e("setUsbTethering: failed to get UsbManager!"); return TETHER_ERROR_SERVICE_UNAVAIL; } - - synchronized (mPublicSync) { - usbManager.setCurrentFunctions(enable ? UsbManager.FUNCTION_RNDIS - : UsbManager.FUNCTION_NONE); - } + usbManager.setCurrentFunctions(enable ? UsbManager.FUNCTION_RNDIS + : UsbManager.FUNCTION_NONE); return TETHER_ERROR_NO_ERROR; } private int setNcmTethering(boolean enable) { if (VDBG) Log.d(TAG, "setNcmTethering(" + enable + ")"); UsbManager usbManager = (UsbManager) mContext.getSystemService(Context.USB_SERVICE); - synchronized (mPublicSync) { - usbManager.setCurrentFunctions(enable ? UsbManager.FUNCTION_NCM - : UsbManager.FUNCTION_NONE); - } + usbManager.setCurrentFunctions(enable ? UsbManager.FUNCTION_NCM : UsbManager.FUNCTION_NONE); return TETHER_ERROR_NO_ERROR; } // TODO review API - figure out how to delete these entirely. String[] getTetheredIfaces() { ArrayList list = new ArrayList(); - synchronized (mPublicSync) { - for (int i = 0; i < mTetherStates.size(); i++) { - TetherState tetherState = mTetherStates.valueAt(i); - if (tetherState.lastState == IpServer.STATE_TETHERED) { - list.add(mTetherStates.keyAt(i)); - } + for (int i = 0; i < mTetherStates.size(); i++) { + TetherState tetherState = mTetherStates.valueAt(i); + if (tetherState.lastState == IpServer.STATE_TETHERED) { + list.add(mTetherStates.keyAt(i)); } } return list.toArray(new String[list.size()]); } - String[] getTetherableIfaces() { + String[] getTetherableIfacesForTest() { ArrayList list = new ArrayList(); - synchronized (mPublicSync) { - for (int i = 0; i < mTetherStates.size(); i++) { - TetherState tetherState = mTetherStates.valueAt(i); - if (tetherState.lastState == IpServer.STATE_AVAILABLE) { - list.add(mTetherStates.keyAt(i)); - } + for (int i = 0; i < mTetherStates.size(); i++) { + TetherState tetherState = mTetherStates.valueAt(i); + if (tetherState.lastState == IpServer.STATE_AVAILABLE) { + list.add(mTetherStates.keyAt(i)); } } return list.toArray(new String[list.size()]); @@ -1390,29 +1350,13 @@ public class Tethering { return mConfig.legacyDhcpRanges; } - String[] getErroredIfaces() { - ArrayList list = new ArrayList(); - synchronized (mPublicSync) { - for (int i = 0; i < mTetherStates.size(); i++) { - TetherState tetherState = mTetherStates.valueAt(i); - if (tetherState.lastError != TETHER_ERROR_NO_ERROR) { - list.add(mTetherStates.keyAt(i)); - } - } - } - return list.toArray(new String[list.size()]); - } - private void logMessage(State state, int what) { mLog.log(state.getName() + " got " + sMagicDecoderRing.get(what, Integer.toString(what))); } private boolean upstreamWanted() { if (!mForwardedDownstreams.isEmpty()) return true; - - synchronized (mPublicSync) { - return mWifiTetherRequested; - } + return mWifiTetherRequested; } // Needed because the canonical source of upstream truth is just the @@ -2308,37 +2252,35 @@ public class Tethering { mEntitlementMgr.dump(pw); pw.decreaseIndent(); - synchronized (mPublicSync) { - pw.println("Tether state:"); - pw.increaseIndent(); - for (int i = 0; i < mTetherStates.size(); i++) { - final String iface = mTetherStates.keyAt(i); - final TetherState tetherState = mTetherStates.valueAt(i); - pw.print(iface + " - "); + pw.println("Tether state:"); + pw.increaseIndent(); + 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.lastState) { - case IpServer.STATE_UNAVAILABLE: - pw.print("UnavailableState"); - break; - case IpServer.STATE_AVAILABLE: - pw.print("AvailableState"); - break; - case IpServer.STATE_TETHERED: - pw.print("TetheredState"); - break; - case IpServer.STATE_LOCAL_ONLY: - pw.print("LocalHotspotState"); - break; - default: - pw.print("UnknownState"); - break; - } - pw.println(" - lastError = " + tetherState.lastError); + switch (tetherState.lastState) { + case IpServer.STATE_UNAVAILABLE: + pw.print("UnavailableState"); + break; + case IpServer.STATE_AVAILABLE: + pw.print("AvailableState"); + break; + case IpServer.STATE_TETHERED: + pw.print("TetheredState"); + break; + case IpServer.STATE_LOCAL_ONLY: + pw.print("LocalHotspotState"); + break; + default: + pw.print("UnknownState"); + break; } - pw.println("Upstream wanted: " + upstreamWanted()); - pw.println("Current upstream interface(s): " + mCurrentUpstreamIfaceSet); - pw.decreaseIndent(); + pw.println(" - lastError = " + tetherState.lastError); } + pw.println("Upstream wanted: " + upstreamWanted()); + pw.println("Current upstream interface(s): " + mCurrentUpstreamIfaceSet); + pw.decreaseIndent(); pw.println("Hardware offload:"); pw.increaseIndent(); @@ -2439,14 +2381,12 @@ public class Tethering { // TODO: Move into TetherMainSM. private void notifyInterfaceStateChange(IpServer who, int state, int error) { final String iface = who.interfaceName(); - synchronized (mPublicSync) { - final TetherState tetherState = mTetherStates.get(iface); - if (tetherState != null && tetherState.ipServer.equals(who)) { - tetherState.lastState = state; - tetherState.lastError = error; - } else { - if (DBG) Log.d(TAG, "got notification from stale iface " + iface); - } + final TetherState tetherState = mTetherStates.get(iface); + if (tetherState != null && tetherState.ipServer.equals(who)) { + tetherState.lastState = state; + tetherState.lastError = error; + } else { + if (DBG) Log.d(TAG, "got notification from stale iface " + iface); } mLog.log(String.format("OBSERVED iface=%s state=%s error=%s", iface, state, error)); @@ -2478,14 +2418,12 @@ public class Tethering { private void notifyLinkPropertiesChanged(IpServer who, LinkProperties newLp) { final String iface = who.interfaceName(); final int state; - synchronized (mPublicSync) { - final TetherState tetherState = mTetherStates.get(iface); - if (tetherState != null && tetherState.ipServer.equals(who)) { - state = tetherState.lastState; - } else { - mLog.log("got notification from stale iface " + iface); - return; - } + final TetherState tetherState = mTetherStates.get(iface); + if (tetherState != null && tetherState.ipServer.equals(who)) { + state = tetherState.lastState; + } else { + mLog.log("got notification from stale iface " + iface); + return; } mLog.log(String.format( diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringService.java b/Tethering/src/com/android/networkstack/tethering/TetheringService.java index e36df7fac2..745ebd0762 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringService.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringService.java @@ -121,9 +121,7 @@ public class TetheringService extends Service { IIntResultListener listener) { if (checkAndNotifyCommonError(callerPkg, callingAttributionTag, listener)) return; - try { - listener.onResult(mTethering.setUsbTethering(enable)); - } catch (RemoteException e) { } + mTethering.setUsbTethering(enable, listener); } @Override diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java index 7204ff6747..941cd78a62 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java @@ -25,7 +25,10 @@ import static android.net.TetheringManager.TETHER_ERROR_NO_CHANGE_TETHERING_PERM import static android.net.TetheringManager.TETHER_ERROR_NO_ERROR; import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -214,11 +217,15 @@ public final class TetheringServiceTest { } private void runSetUsbTethering(final TestTetheringResult result) throws Exception { - when(mTethering.setUsbTethering(true /* enable */)).thenReturn(TETHER_ERROR_NO_ERROR); + doAnswer((invocation) -> { + final IIntResultListener listener = invocation.getArgument(1); + listener.onResult(TETHER_ERROR_NO_ERROR); + return null; + }).when(mTethering).setUsbTethering(anyBoolean(), any(IIntResultListener.class)); mTetheringConnector.setUsbTethering(true /* enable */, TEST_CALLER_PKG, TEST_ATTRIBUTION_TAG, result); verify(mTethering).isTetheringSupported(); - verify(mTethering).setUsbTethering(true /* enable */); + verify(mTethering).setUsbTethering(eq(true) /* enable */, any(IIntResultListener.class)); result.assertResult(TETHER_ERROR_NO_ERROR); } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index faa5ac7039..fedeeeb609 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -941,7 +941,7 @@ public class TetheringTest { verifyNoMoreInteractions(mWifiManager); // Asking for the last error after the per-interface state machine // has been reaped yields an unknown interface error. - assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastTetherError(TEST_WLAN_IFNAME)); + assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastErrorForTest(TEST_WLAN_IFNAME)); } /** @@ -1474,7 +1474,7 @@ public class TetheringTest { verifyNoMoreInteractions(mWifiManager); // Asking for the last error after the per-interface state machine // has been reaped yields an unknown interface error. - assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastTetherError(TEST_WLAN_IFNAME)); + assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastErrorForTest(TEST_WLAN_IFNAME)); } // TODO: Test with and without interfaceStatusChanged(). @@ -1942,7 +1942,7 @@ public class TetheringTest { // There are 2 IpServer state change events: STATE_AVAILABLE -> STATE_LOCAL_ONLY verify(mNotificationUpdater, times(2)).onDownstreamChanged(DOWNSTREAM_NONE); - assertEquals(TETHER_ERROR_NO_ERROR, mTethering.getLastTetherError(TEST_P2P_IFNAME)); + assertEquals(TETHER_ERROR_NO_ERROR, mTethering.getLastErrorForTest(TEST_P2P_IFNAME)); // Emulate externally-visible WifiP2pManager effects, when wifi p2p group // is being removed. @@ -1961,7 +1961,7 @@ public class TetheringTest { verifyNoMoreInteractions(mNetd); // Asking for the last error after the per-interface state machine // has been reaped yields an unknown interface error. - assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastTetherError(TEST_P2P_IFNAME)); + assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastErrorForTest(TEST_P2P_IFNAME)); } private void workingWifiP2pGroupClient( @@ -1991,7 +1991,7 @@ public class TetheringTest { verifyNoMoreInteractions(mNetd); // Asking for the last error after the per-interface state machine // has been reaped yields an unknown interface error. - assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastTetherError(TEST_P2P_IFNAME)); + assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastErrorForTest(TEST_P2P_IFNAME)); } @Test @@ -2022,7 +2022,7 @@ public class TetheringTest { verify(mNetd, never()).networkAddInterface(INetd.LOCAL_NET_ID, TEST_P2P_IFNAME); verify(mNetd, never()).ipfwdEnableForwarding(TETHERING_NAME); verify(mNetd, never()).tetherStartWithConfiguration(any()); - assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastTetherError(TEST_P2P_IFNAME)); + assertEquals(TETHER_ERROR_UNKNOWN_IFACE, mTethering.getLastErrorForTest(TEST_P2P_IFNAME)); } @Test public void workingWifiP2pGroupOwnerLegacyModeWithIfaceChanged() throws Exception { @@ -2387,10 +2387,10 @@ public class TetheringTest { mTethering.interfaceStatusChanged(TEST_USB_IFNAME, true); sendUsbBroadcast(true, true, true, TETHERING_USB); - assertContains(Arrays.asList(mTethering.getTetherableIfaces()), TEST_USB_IFNAME); - assertContains(Arrays.asList(mTethering.getTetherableIfaces()), TEST_ETH_IFNAME); - assertEquals(TETHER_ERROR_IFACE_CFG_ERROR, mTethering.getLastTetherError(TEST_USB_IFNAME)); - assertEquals(TETHER_ERROR_IFACE_CFG_ERROR, mTethering.getLastTetherError(TEST_ETH_IFNAME)); + assertContains(Arrays.asList(mTethering.getTetherableIfacesForTest()), TEST_USB_IFNAME); + assertContains(Arrays.asList(mTethering.getTetherableIfacesForTest()), TEST_ETH_IFNAME); + assertEquals(TETHER_ERROR_IFACE_CFG_ERROR, mTethering.getLastErrorForTest(TEST_USB_IFNAME)); + assertEquals(TETHER_ERROR_IFACE_CFG_ERROR, mTethering.getLastErrorForTest(TEST_ETH_IFNAME)); } @Test