Fix crash and duplicated ethernet tethering request

This change fix two things:
1. Handle ethernet callback in internal thread to avoid crash. IpServer
should be created from tethering thread, otherwise mIpNeighborMonitor of
IpServer would throw
   IllegalStateException("start() called from off-thread")
2. Ethernet tethering request may be duplicated if multiple
startTethering is called but no stopTethering

Bug: 130840861
Bug: 148824036
Test: ON/OFF ethernet tehtering manually
      atest TetheringTests

Change-Id: I7c5127e96d80d077735010d2e62c7227805ccb10

Tethering/src/com/android/server/connectivity/tethering/Tethering.java

@@ -220,6 +220,7 @@ public class Tethering {
     private final UserRestrictionActionListener mTetheringRestriction;
     private final ActiveDataSubIdListener mActiveDataSubIdListener;
     private final ConnectedClientsTracker mConnectedClientsTracker;
+    private final TetheringThreadExecutor mExecutor;
     private int mActiveDataSubId = INVALID_SUBSCRIPTION_ID;
     // All the usage of mTetheringEventCallback should run in the same thread.
     private ITetheringEventCallback mTetheringEventCallback = null;
@@ -296,8 +297,8 @@ public class Tethering {
         final UserManager userManager = (UserManager) mContext.getSystemService(
                 Context.USER_SERVICE);
         mTetheringRestriction = new UserRestrictionActionListener(userManager, this);
-        final TetheringThreadExecutor executor = new TetheringThreadExecutor(mHandler);
+        mExecutor = new TetheringThreadExecutor(mHandler);
-        mActiveDataSubIdListener = new ActiveDataSubIdListener(executor);
+        mActiveDataSubIdListener = new ActiveDataSubIdListener(mExecutor);
 
         // Load tethering configuration.
         updateConfiguration();
@@ -315,9 +316,7 @@ public class Tethering {
 
         final WifiManager wifiManager = getWifiManager();
         if (wifiManager != null) {
-            wifiManager.registerSoftApCallback(
+            wifiManager.registerSoftApCallback(mExecutor, new TetheringSoftApCallback());
-                  mHandler::post /* executor */,
-                  new TetheringSoftApCallback());
         }
     }
 
@@ -606,14 +605,17 @@ public class Tethering {
                 Context.ETHERNET_SERVICE);
         synchronized (mPublicSync) {
             if (enable) {
+                if (mEthernetCallback != null) return TETHER_ERROR_NO_ERROR;
+
                 mEthernetCallback = new EthernetCallback();
-                mEthernetIfaceRequest = em.requestTetheredInterface(mEthernetCallback);
+                mEthernetIfaceRequest = em.requestTetheredInterface(mExecutor, mEthernetCallback);
             } else {
-                if (mConfiguredEthernetIface != null) {
                 stopEthernetTetheringLocked();
+                if (mEthernetCallback != null) {
                     mEthernetIfaceRequest.release();
-                }
                     mEthernetCallback = null;
+                    mEthernetIfaceRequest = null;
+                }
             }
         }
         return TETHER_ERROR_NO_ERROR;

Tethering/tests/unit/src/com/android/server/connectivity/tethering/TetheringTest.java

@@ -28,6 +28,7 @@ import static android.net.TetheringManager.ACTION_TETHER_STATE_CHANGED;
 import static android.net.TetheringManager.EXTRA_ACTIVE_LOCAL_ONLY;
 import static android.net.TetheringManager.EXTRA_ACTIVE_TETHER;
 import static android.net.TetheringManager.EXTRA_AVAILABLE_TETHER;
+import static android.net.TetheringManager.TETHERING_ETHERNET;
 import static android.net.TetheringManager.TETHERING_NCM;
 import static android.net.TetheringManager.TETHERING_USB;
 import static android.net.TetheringManager.TETHERING_WIFI;
@@ -75,6 +76,8 @@ import android.content.pm.ApplicationInfo;
 import android.content.res.Resources;
 import android.hardware.usb.UsbManager;
 import android.net.ConnectivityManager;
+import android.net.EthernetManager;
+import android.net.EthernetManager.TetheredInterfaceRequest;
 import android.net.INetd;
 import android.net.ITetheringEventCallback;
 import android.net.InetAddresses;
@@ -180,6 +183,7 @@ public class TetheringTest {
     @Mock private UserManager mUserManager;
     @Mock private NetworkRequest mNetworkRequest;
     @Mock private ConnectivityManager mCm;
+    @Mock private EthernetManager mEm;
 
     private final MockIpServerDependencies mIpServerDependencies =
             spy(new MockIpServerDependencies());
@@ -232,6 +236,7 @@ public class TetheringTest {
             if (Context.USER_SERVICE.equals(name)) return mUserManager;
             if (Context.NETWORK_STATS_SERVICE.equals(name)) return mStatsManager;
             if (Context.CONNECTIVITY_SERVICE.equals(name)) return mCm;
+            if (Context.ETHERNET_SERVICE.equals(name)) return mEm;
             return super.getSystemService(name);
         }
 
@@ -1316,6 +1321,24 @@ public class TetheringTest {
         assertEquals(fakeSubId, newConfig.activeDataSubId);
     }
 
+    @Test
+    public void testNoDuplicatedEthernetRequest() throws Exception {
+        final TetheredInterfaceRequest mockRequest = mock(TetheredInterfaceRequest.class);
+        when(mEm.requestTetheredInterface(any(), any())).thenReturn(mockRequest);
+        mTethering.startTethering(createTetheringRquestParcel(TETHERING_ETHERNET), null);
+        mLooper.dispatchAll();
+        verify(mEm, times(1)).requestTetheredInterface(any(), any());
+        mTethering.startTethering(createTetheringRquestParcel(TETHERING_ETHERNET), null);
+        mLooper.dispatchAll();
+        verifyNoMoreInteractions(mEm);
+        mTethering.stopTethering(TETHERING_ETHERNET);
+        mLooper.dispatchAll();
+        verify(mockRequest, times(1)).release();
+        mTethering.stopTethering(TETHERING_ETHERNET);
+        mLooper.dispatchAll();
+        verifyNoMoreInteractions(mEm);
+    }
+
     private void workingWifiP2pGroupOwner(
             boolean emulateInterfaceStatusChanged) throws Exception {
         if (emulateInterfaceStatusChanged) {