From c8585b974274a98fdaabeee9bcf0c7b1d56a65f1 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Mon, 12 Dec 2016 21:21:40 +0900 Subject: [PATCH] More correct startup and shutdown procedures. During IpManager startup, anything sending messages to the state machine must not begin doing so until after the state machine has been started. Reorder the constructor accordingly. During shutdown, AvoidBadWifiTracker needs to unregister the registered BroadcastReceiver and might as well also unregister the ContentObserver. Test: backport from internal Bug: 33388922 Change-Id: I58e07f7ccddaab160c153bcfb69fd45f50bb8710 --- .../android/server/ConnectivityService.java | 1 + .../net/java/android/net/ip/IpManager.java | 57 +++++++++++++++---- .../android/net/util/AvoidBadWifiTracker.java | 38 +++++++++---- 3 files changed, 74 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 41eaa6ad201fe..66f5c4d4ba712 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -856,6 +856,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mAvoidBadWifiTracker = createAvoidBadWifiTracker( mContext, mHandler, () -> rematchForAvoidBadWifiUpdate()); + mAvoidBadWifiTracker.start(); } private NetworkRequest createInternetRequestForTransport( diff --git a/services/net/java/android/net/ip/IpManager.java b/services/net/java/android/net/ip/IpManager.java index 39f14e5ee287f..58b2dec578fee 100644 --- a/services/net/java/android/net/ip/IpManager.java +++ b/services/net/java/android/net/ip/IpManager.java @@ -438,6 +438,9 @@ public class IpManager extends StateMachine { mCallback = new LoggingCallbackWrapper(callback); mNwService = nwService; + mLocalLog = new LocalLog(MAX_LOG_RECORDS); + mMsgStateLogger = new MessageHandlingLogger(); + mNetlinkTracker = new NetlinkTracker( mInterfaceName, new NetlinkTracker.Callback() { @@ -451,48 +454,79 @@ public class IpManager extends StateMachine { super.interfaceAdded(iface); if (mClatInterfaceName.equals(iface)) { mCallback.setNeighborDiscoveryOffload(false); + } else if (!mInterfaceName.equals(iface)) { + return; } + + final String msg = "interfaceAdded(" + iface +")"; + logMsg(msg); } @Override public void interfaceRemoved(String iface) { super.interfaceRemoved(iface); + // TODO: Also observe mInterfaceName going down and take some + // kind of appropriate action. if (mClatInterfaceName.equals(iface)) { // TODO: consider sending a message to the IpManager main // StateMachine thread, in case "NDO enabled" state becomes // tied to more things that 464xlat operation. mCallback.setNeighborDiscoveryOffload(true); + } else if (!mInterfaceName.equals(iface)) { + return; } + + final String msg = "interfaceRemoved(" + iface +")"; + logMsg(msg); + } + + private void logMsg(String msg) { + Log.d(mTag, msg); + getHandler().post(() -> { mLocalLog.log("OBSERVED " + msg); }); } }; - try { - mNwService.registerObserver(mNetlinkTracker); - } catch (RemoteException e) { - Log.e(mTag, "Couldn't register NetlinkTracker: " + e.toString()); - } + mLinkProperties = new LinkProperties(); + mLinkProperties.setInterfaceName(mInterfaceName); - mAvoidBadWifiTracker = new AvoidBadWifiTracker(mContext, getHandler()); - - resetLinkProperties(); + mAvoidBadWifiTracker = new AvoidBadWifiTracker(mContext, getHandler(), + () -> { mLocalLog.log("OBSERVED AvoidBadWifi changed"); }); mProvisioningTimeoutAlarm = new WakeupMessage(mContext, getHandler(), mTag + ".EVENT_PROVISIONING_TIMEOUT", EVENT_PROVISIONING_TIMEOUT); mDhcpActionTimeoutAlarm = new WakeupMessage(mContext, getHandler(), mTag + ".EVENT_DHCPACTION_TIMEOUT", EVENT_DHCPACTION_TIMEOUT); - // Super simple StateMachine. + // Anything the StateMachine may access must have been instantiated + // before this point. + configureAndStartStateMachine(); + + // Anything that may send messages to the StateMachine must only be + // configured to do so after the StateMachine has started (above). + startStateMachineUpdaters(); + } + + private void configureAndStartStateMachine() { addState(mStoppedState); addState(mStartedState); addState(mRunningState, mStartedState); addState(mStoppingState); setInitialState(mStoppedState); - mLocalLog = new LocalLog(MAX_LOG_RECORDS); - mMsgStateLogger = new MessageHandlingLogger(); + super.start(); } + private void startStateMachineUpdaters() { + try { + mNwService.registerObserver(mNetlinkTracker); + } catch (RemoteException e) { + Log.e(mTag, "Couldn't register NetlinkTracker: " + e.toString()); + } + + mAvoidBadWifiTracker.start(); + } + @Override protected void onQuitting() { mCallback.onQuit(); @@ -501,6 +535,7 @@ public class IpManager extends StateMachine { // Shut down this IpManager instance altogether. public void shutdown() { stop(); + mAvoidBadWifiTracker.shutdown(); quit(); } diff --git a/services/net/java/android/net/util/AvoidBadWifiTracker.java b/services/net/java/android/net/util/AvoidBadWifiTracker.java index c14e811e05844..2abaeb1ae35b6 100644 --- a/services/net/java/android/net/util/AvoidBadWifiTracker.java +++ b/services/net/java/android/net/util/AvoidBadWifiTracker.java @@ -57,7 +57,11 @@ public class AvoidBadWifiTracker { private final Context mContext; private final Handler mHandler; private final Runnable mReevaluateRunnable; + private final Uri mUri; + private final ContentResolver mResolver; private final SettingObserver mSettingObserver; + private final BroadcastReceiver mBroadcastReceiver; + private volatile boolean mAvoidBadWifi = true; public AvoidBadWifiTracker(Context ctx, Handler handler) { @@ -68,19 +72,36 @@ public class AvoidBadWifiTracker { mContext = ctx; mHandler = handler; mReevaluateRunnable = () -> { if (update() && cb != null) cb.run(); }; + mUri = Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI); + mResolver = mContext.getContentResolver(); mSettingObserver = new SettingObserver(); - - final IntentFilter intentFilter = new IntentFilter(); - intentFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED); - mContext.registerReceiverAsUser(new BroadcastReceiver() { + mBroadcastReceiver = new BroadcastReceiver() { + @Override public void onReceive(Context context, Intent intent) { reevaluate(); } - }, UserHandle.ALL, intentFilter, null, null); + }; update(); } + public void start() { + mResolver.registerContentObserver(mUri, false, mSettingObserver); + + final IntentFilter intentFilter = new IntentFilter(); + intentFilter.addAction(Intent.ACTION_CONFIGURATION_CHANGED); + mContext.registerReceiverAsUser( + mBroadcastReceiver, UserHandle.ALL, intentFilter, null, null); + + reevaluate(); + } + + public void shutdown() { + mResolver.unregisterContentObserver(mSettingObserver); + + mContext.unregisterReceiver(mBroadcastReceiver); + } + public boolean currentValue() { return mAvoidBadWifi; } @@ -100,8 +121,7 @@ public class AvoidBadWifiTracker { } public String getSettingsValue() { - final ContentResolver resolver = mContext.getContentResolver(); - return Settings.Global.getString(resolver, NETWORK_AVOID_BAD_WIFI); + return Settings.Global.getString(mResolver, NETWORK_AVOID_BAD_WIFI); } @VisibleForTesting @@ -117,12 +137,8 @@ public class AvoidBadWifiTracker { } private class SettingObserver extends ContentObserver { - private final Uri mUri = Settings.Global.getUriFor(NETWORK_AVOID_BAD_WIFI); - public SettingObserver() { super(null); - final ContentResolver resolver = mContext.getContentResolver(); - resolver.registerContentObserver(mUri, false, this); } @Override