Make Ethernet more robust.

am: 9effed6f9e

Change-Id: I1c531a142706d3cbe0da3adf810887cc221944ed

service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java

@@ -34,7 +34,6 @@ import android.net.NetworkInfo.DetailedState;
 import android.net.StaticIpConfiguration;
 import android.net.ip.IpManager;
 import android.net.ip.IpManager.ProvisioningConfiguration;
-import android.net.ip.IpManager.WaitForProvisioningCallback;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.INetworkManagementService;
@@ -50,6 +49,8 @@ import com.android.server.net.BaseNetworkObserver;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
 
 
 /**
@@ -84,6 +85,9 @@ class EthernetNetworkFactory {
     /** To set link state and configure IP addresses. */
     private INetworkManagementService mNMService;
 
+    /** All code runs here, including start(). */
+    private Handler mHandler;
+
     /* To communicate with ConnectivityManager */
     private NetworkCapabilities mNetworkCapabilities;
     private NetworkAgent mNetworkAgent;
@@ -96,19 +100,18 @@ class EthernetNetworkFactory {
     /** To notify Ethernet status. */
     private final RemoteCallbackList<IEthernetServiceListener> mListeners;
 
-    /** Data members. All accesses to these must be synchronized(this). */
+    /** Data members. All accesses to these must be on the handler thread. */
-    private static String mIface = "";
+    private String mIface = "";
     private String mHwAddr;
-    private static boolean mLinkUp;
+    private boolean mLinkUp;
     private NetworkInfo mNetworkInfo;
     private LinkProperties mLinkProperties;
     private IpManager mIpManager;
-    private Thread mIpProvisioningThread;
+    private boolean mNetworkRequested = false;
 
     EthernetNetworkFactory(RemoteCallbackList<IEthernetServiceListener> listeners) {
-        mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_ETHERNET, 0, NETWORK_TYPE, "");
-        mLinkProperties = new LinkProperties();
         initNetworkCapabilities();
+        clearInfo();
         mListeners = listeners;
     }
 
@@ -118,26 +121,40 @@ class EthernetNetworkFactory {
         }
 
         protected void startNetwork() {
-            onRequestNetwork();
+            if (!mNetworkRequested) {
-        }
+                mNetworkRequested = true;
-        protected void stopNetwork() {
+                maybeStartIpManager();
             }
         }
 
-    private void stopIpManagerLocked() {
+        protected void stopNetwork() {
+            mNetworkRequested = false;
+            stopIpManager();
+        }
+    }
+
+    private void clearInfo() {
+        mLinkProperties = new LinkProperties();
+        mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_ETHERNET, 0, NETWORK_TYPE, "");
+        mNetworkInfo.setExtraInfo(mHwAddr);
+        mNetworkInfo.setIsAvailable(isTrackingInterface());
+    }
+
+    private void stopIpManager() {
         if (mIpManager != null) {
             mIpManager.shutdown();
             mIpManager = null;
         }
+        // ConnectivityService will only forget our NetworkAgent if we send it a NetworkInfo object
+        // with a state of DISCONNECTED or SUSPENDED. So we can't simply clear our NetworkInfo here:
+        // that sets the state to IDLE, and ConnectivityService will still think we're connected.
+        //
+        mNetworkInfo.setDetailedState(DetailedState.DISCONNECTED, null, mHwAddr);
+        if (mNetworkAgent != null) {
+            updateAgent();
+            mNetworkAgent = null;
         }
-
+        clearInfo();
-    private void stopIpProvisioningThreadLocked() {
-        stopIpManagerLocked();
-
-        if (mIpProvisioningThread != null) {
-            mIpProvisioningThread.interrupt();
-            mIpProvisioningThread = null;
-        }
     }
 
     /**
@@ -150,36 +167,36 @@ class EthernetNetworkFactory {
         }
         Log.d(TAG, "updateInterface: " + iface + " link " + (up ? "up" : "down"));
 
-        synchronized(this) {
         mLinkUp = up;
-            mNetworkInfo.setIsAvailable(up);
+        if (up) {
-            if (!up) {
+            maybeStartIpManager();
-                // Tell the agent we're disconnected. It will call disconnect().
+        } else {
-                mNetworkInfo.setDetailedState(DetailedState.DISCONNECTED, null, mHwAddr);
+            stopIpManager();
-                stopIpProvisioningThreadLocked();
-            }
-            updateAgent();
-            // set our score lower than any network could go
-            // so we get dropped.  TODO - just unregister the factory
-            // when link goes down.
-            mFactory.setScoreFilter(up ? NETWORK_SCORE : -1);
         }
     }
 
     private class InterfaceObserver extends BaseNetworkObserver {
         @Override
         public void interfaceLinkStateChanged(String iface, boolean up) {
+            mHandler.post(() -> {
                 updateInterfaceState(iface, up);
+            });
         }
 
         @Override
         public void interfaceAdded(String iface) {
+            mHandler.post(() -> {
                 maybeTrackInterface(iface);
+            });
         }
 
         @Override
         public void interfaceRemoved(String iface) {
-            stopTrackingInterface(iface);
+            mHandler.post(() -> {
+                if (stopTrackingInterface(iface)) {
+                    trackFirstAvailableInterface();
+                }
+            });
         }
     }
 
@@ -195,16 +212,14 @@ class EthernetNetworkFactory {
                 return;
             }
 
-            synchronized (this) {
             if (!isTrackingInterface()) {
-                    setInterfaceInfoLocked(iface, config.getHardwareAddress());
+                setInterfaceInfo(iface, config.getHardwareAddress());
                 mNetworkInfo.setIsAvailable(true);
                 mNetworkInfo.setExtraInfo(mHwAddr);
             } else {
                 Log.e(TAG, "Interface unexpectedly changed from " + iface + " to " + mIface);
                 mNMService.setInterfaceDown(iface);
             }
-            }
         } catch (RemoteException e) {
             Log.e(TAG, "Error upping interface " + mIface + ": " + e);
         }
@@ -221,23 +236,14 @@ class EthernetNetworkFactory {
         return true;
     }
 
-    private void stopTrackingInterface(String iface) {
+    private boolean stopTrackingInterface(String iface) {
         if (!iface.equals(mIface))
-            return;
+            return false;
 
         Log.d(TAG, "Stopped tracking interface " + iface);
-        // TODO: Unify this codepath with stop().
+        setInterfaceInfo("", null);
-        synchronized (this) {
+        stopIpManager();
-            stopIpProvisioningThreadLocked();
+        return true;
-            setInterfaceInfoLocked("", null);
-            mNetworkInfo.setExtraInfo(null);
-            mLinkUp = false;
-            mNetworkInfo.setDetailedState(DetailedState.DISCONNECTED, null, mHwAddr);
-            updateAgent();
-            mNetworkAgent = null;
-            mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_ETHERNET, 0, NETWORK_TYPE, "");
-            mLinkProperties = new LinkProperties();
-        }
     }
 
     private boolean setStaticIpAddress(StaticIpConfiguration staticConfig) {
@@ -260,7 +266,6 @@ class EthernetNetworkFactory {
     }
 
     public void updateAgent() {
-        synchronized (EthernetNetworkFactory.this) {
         if (mNetworkAgent == null) return;
         if (DBG) {
             Log.i(TAG, "Updating mNetworkAgent with: " +
@@ -274,21 +279,55 @@ class EthernetNetworkFactory {
         // never set the network score below 0.
         mNetworkAgent.sendNetworkScore(mLinkUp? NETWORK_SCORE : 0);
     }
-    }
 
-    /* Called by the NetworkFactory on the handler thread. */
+    void onIpLayerStarted(LinkProperties linkProperties) {
-    public void onRequestNetwork() {
+        if (mNetworkAgent != null) {
-        synchronized(EthernetNetworkFactory.this) {
+            Log.e(TAG, "Already have a NetworkAgent - aborting new request");
-            if (mIpProvisioningThread != null) {
+            stopIpManager();
             return;
         }
+        mLinkProperties = linkProperties;
+        mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, mHwAddr);
+
+        // Create our NetworkAgent.
+        mNetworkAgent = new NetworkAgent(mHandler.getLooper(), mContext,
+                NETWORK_TYPE, mNetworkInfo, mNetworkCapabilities, mLinkProperties,
+                NETWORK_SCORE) {
+            public void unwanted() {
+                if (this == mNetworkAgent) {
+                    stopIpManager();
+                } else if (mNetworkAgent != null) {
+                    Log.d(TAG, "Ignoring unwanted as we have a more modern " +
+                            "instance");
+                }  // Otherwise, we've already called stopIpManager.
+            }
+        };
     }
 
-        final Thread ipProvisioningThread = new Thread(new Runnable() {
+    void onIpLayerStopped(LinkProperties linkProperties) {
-            public void run() {
+        // This cannot happen due to provisioning timeout, because our timeout is 0. It can only
+        // happen if we're provisioned and we lose provisioning.
+        stopIpManager();
+        maybeStartIpManager();
+    }
+
+    void updateLinkProperties(LinkProperties linkProperties) {
+        mLinkProperties = linkProperties;
+        if (mNetworkAgent != null) {
+            mNetworkAgent.sendLinkProperties(linkProperties);
+        }
+    }
+
+    public void maybeStartIpManager() {
+        if (mNetworkRequested && mIpManager == null && isTrackingInterface()) {
+            startIpManager();
+        }
+    }
+
+    public void startIpManager() {
         if (DBG) {
-                    Log.d(TAG, String.format("starting ipProvisioningThread(%s): mNetworkInfo=%s",
+            Log.d(TAG, String.format("starting IpManager(%s): mNetworkInfo=%s", mIface,
-                            mIface, mNetworkInfo));
+                    mNetworkInfo));
         }
 
         LinkProperties linkProperties;
@@ -303,20 +342,24 @@ class EthernetNetworkFactory {
             linkProperties = config.getStaticIpConfiguration().toLinkProperties(mIface);
         } else {
             mNetworkInfo.setDetailedState(DetailedState.OBTAINING_IPADDR, null, mHwAddr);
-                    WaitForProvisioningCallback ipmCallback = new WaitForProvisioningCallback() {
+            IpManager.Callback ipmCallback = new IpManager.Callback() {
+                @Override
+                public void onProvisioningSuccess(LinkProperties newLp) {
+                    mHandler.post(() -> onIpLayerStarted(newLp));
+                }
+
+                @Override
+                public void onProvisioningFailure(LinkProperties newLp) {
+                    mHandler.post(() -> onIpLayerStopped(newLp));
+                }
+
                 @Override
                 public void onLinkPropertiesChange(LinkProperties newLp) {
-                            synchronized(EthernetNetworkFactory.this) {
+                    mHandler.post(() -> updateLinkProperties(newLp));
-                                if (mNetworkAgent != null && mNetworkInfo.isConnected()) {
-                                    mLinkProperties = newLp;
-                                    mNetworkAgent.sendLinkProperties(newLp);
-                                }
-                            }
                 }
             };
 
-                    synchronized(EthernetNetworkFactory.this) {
+            stopIpManager();
-                        stopIpManagerLocked();
             mIpManager = new IpManager(mContext, mIface, ipmCallback);
 
             if (config.getProxySettings() == ProxySettings.STATIC ||
@@ -336,80 +379,14 @@ class EthernetNetworkFactory {
                             .build();
             mIpManager.startProvisioning(provisioningConfiguration);
         }
-
-                    linkProperties = ipmCallback.waitForProvisioning();
-                    if (linkProperties == null) {
-                        Log.e(TAG, "IP provisioning error");
-                        // set our score lower than any network could go
-                        // so we get dropped.
-                        mFactory.setScoreFilter(-1);
-                        synchronized(EthernetNetworkFactory.this) {
-                            stopIpManagerLocked();
-                        }
-                        return;
-                    }
-                }
-
-                synchronized(EthernetNetworkFactory.this) {
-                    if (mNetworkAgent != null) {
-                        Log.e(TAG, "Already have a NetworkAgent - aborting new request");
-                        stopIpManagerLocked();
-                        mIpProvisioningThread = null;
-                        return;
-                    }
-                    mLinkProperties = linkProperties;
-                    mNetworkInfo.setIsAvailable(true);
-                    mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, mHwAddr);
-
-                    // Create our NetworkAgent.
-                    mNetworkAgent = new NetworkAgent(mFactory.getLooper(), mContext,
-                            NETWORK_TYPE, mNetworkInfo, mNetworkCapabilities, mLinkProperties,
-                            NETWORK_SCORE) {
-                        public void unwanted() {
-                            synchronized(EthernetNetworkFactory.this) {
-                                if (this == mNetworkAgent) {
-                                    stopIpManagerLocked();
-
-                                    mLinkProperties.clear();
-                                    mNetworkInfo.setDetailedState(DetailedState.DISCONNECTED, null,
-                                            mHwAddr);
-                                    updateAgent();
-                                    mNetworkAgent = null;
-                                    try {
-                                        mNMService.clearInterfaceAddresses(mIface);
-                                    } catch (Exception e) {
-                                        Log.e(TAG, "Failed to clear addresses or disable ipv6" + e);
-                                    }
-                                } else {
-                                    Log.d(TAG, "Ignoring unwanted as we have a more modern " +
-                                            "instance");
-                                }
-                            }
-                        };
-                    };
-
-                    mIpProvisioningThread = null;
-                }
-
-                if (DBG) {
-                    Log.d(TAG, String.format("exiting ipProvisioningThread(%s): mNetworkInfo=%s",
-                            mIface, mNetworkInfo));
-                }
-            }
-        });
-
-        synchronized(EthernetNetworkFactory.this) {
-            if (mIpProvisioningThread == null) {
-                mIpProvisioningThread = ipProvisioningThread;
-                mIpProvisioningThread.start();
-            }
-        }
     }
 
     /**
      * Begin monitoring connectivity
      */
-    public synchronized void start(Context context, Handler target) {
+    public void start(Context context, Handler handler) {
+        mHandler = handler;
+
         // The services we use.
         IBinder b = ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE);
         mNMService = INetworkManagementService.Stub.asInterface(b);
@@ -420,9 +397,9 @@ class EthernetNetworkFactory {
                 com.android.internal.R.string.config_ethernet_iface_regex);
 
         // Create and register our NetworkFactory.
-        mFactory = new LocalNetworkFactory(NETWORK_TYPE, context, target.getLooper());
+        mFactory = new LocalNetworkFactory(NETWORK_TYPE, context, mHandler.getLooper());
         mFactory.setCapabilityFilter(mNetworkCapabilities);
-        mFactory.setScoreFilter(-1); // this set high when we have an iface
+        mFactory.setScoreFilter(NETWORK_SCORE);
         mFactory.register();
 
         mContext = context;
@@ -437,45 +414,32 @@ class EthernetNetworkFactory {
 
         // If an Ethernet interface is already connected, start tracking that.
         // Otherwise, the first Ethernet interface to appear will be tracked.
+        mHandler.post(() -> trackFirstAvailableInterface());
+    }
+
+    public void trackFirstAvailableInterface() {
         try {
             final String[] ifaces = mNMService.listInterfaces();
             for (String iface : ifaces) {
-                synchronized(this) {
                 if (maybeTrackInterface(iface)) {
                     // We have our interface. Track it.
-                        // Note: if the interface already has link (e.g., if we
+                    // Note: if the interface already has link (e.g., if we crashed and got
-                        // crashed and got restarted while it was running),
+                    // restarted while it was running), we need to fake a link up notification so we
-                        // we need to fake a link up notification so we start
+                    // start configuring it.
-                        // configuring it. Since we're already holding the lock,
-                        // any real link up/down notification will only arrive
-                        // after we've done this.
                     if (mNMService.getInterfaceConfig(iface).hasFlag("running")) {
                         updateInterfaceState(iface, true);
                     }
                     break;
                 }
             }
-            }
         } catch (RemoteException|IllegalStateException e) {
             Log.e(TAG, "Could not get list of interfaces " + e);
         }
     }
 
-    public synchronized void stop() {
+    public void stop() {
-        stopIpProvisioningThreadLocked();
+        stopIpManager();
-        // ConnectivityService will only forget our NetworkAgent if we send it a NetworkInfo object
+        setInterfaceInfo("", null);
-        // with a state of DISCONNECTED or SUSPENDED. So we can't simply clear our NetworkInfo here:
-        // that sets the state to IDLE, and ConnectivityService will still think we're connected.
-        //
-        // TODO: stop using explicit comparisons to DISCONNECTED / SUSPENDED in ConnectivityService,
-        // and instead use isConnectedOrConnecting().
-        mNetworkInfo.setDetailedState(DetailedState.DISCONNECTED, null, mHwAddr);
-        mLinkUp = false;
-        updateAgent();
-        mLinkProperties = new LinkProperties();
-        mNetworkAgent = null;
-        setInterfaceInfoLocked("", null);
-        mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_ETHERNET, 0, NETWORK_TYPE, "");
         mFactory.unregister();
     }
 
@@ -489,20 +453,22 @@ class EthernetNetworkFactory {
         mNetworkCapabilities.setLinkDownstreamBandwidthKbps(100 * 1000);
     }
 
-    public synchronized boolean isTrackingInterface() {
+    public boolean isTrackingInterface() {
         return !TextUtils.isEmpty(mIface);
     }
 
     /**
      * Set interface information and notify listeners if availability is changed.
-     * This should be called with the lock held.
      */
-    private void setInterfaceInfoLocked(String iface, String hwAddr) {
+    private void setInterfaceInfo(String iface, String hwAddr) {
         boolean oldAvailable = isTrackingInterface();
         mIface = iface;
         mHwAddr = hwAddr;
         boolean available = isTrackingInterface();
 
+        mNetworkInfo.setExtraInfo(mHwAddr);
+        mNetworkInfo.setIsAvailable(available);
+
         if (oldAvailable != available) {
             int n = mListeners.beginBroadcast();
             for (int i = 0; i < n; i++) {
@@ -516,7 +482,23 @@ class EthernetNetworkFactory {
         }
     }
 
-    synchronized void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) {
+    private void postAndWaitForRunnable(Runnable r) throws InterruptedException {
+        CountDownLatch latch = new CountDownLatch(1);
+        mHandler.post(() -> {
+            try {
+                r.run();
+            } finally {
+                latch.countDown();
+            }
+        });
+        latch.await();
+    }
+
+
+    void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) {
+        try {
+            postAndWaitForRunnable(() -> {
+                pw.println("Network Requested: " + mNetworkRequested);
                 if (isTrackingInterface()) {
                     pw.println("Tracking interface: " + mIface);
                     pw.increaseIndent();
@@ -537,5 +519,9 @@ class EthernetNetworkFactory {
                     mIpManager.dump(fd, pw, args);
                     pw.decreaseIndent();
                 }
+            });
+        } catch (InterruptedException e) {
+            throw new IllegalStateException("dump() interrupted");
+        }
     }
 }