Merge changes I2cb4200b,If1fa00be into qt-dev
am: 6b2681521e
Change-Id: Ib279a80a19bcdaf53678921108c4932d04988474
diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java
index 3267114..dee89e5 100644
--- a/services/core/java/com/android/server/PackageWatchdog.java
+++ b/services/core/java/com/android/server/PackageWatchdog.java
@@ -25,6 +25,7 @@
import android.content.Context;
import android.content.pm.PackageManager;
import android.content.pm.VersionedPackage;
+import android.net.NetworkStackClient;
import android.os.Environment;
import android.os.Handler;
import android.os.Looper;
@@ -115,6 +116,7 @@
// File containing the XML data of monitored packages /data/system/package-watchdog.xml
private final AtomicFile mPolicyFile;
private final ExplicitHealthCheckController mHealthCheckController;
+ private final NetworkStackClient mNetworkStackClient;
@GuardedBy("mLock")
private boolean mIsPackagesReady;
// Flag to control whether explicit health checks are supported or not
@@ -135,7 +137,8 @@
new File(new File(Environment.getDataDirectory(), "system"),
"package-watchdog.xml")),
new Handler(Looper.myLooper()), BackgroundThread.getHandler(),
- new ExplicitHealthCheckController(context));
+ new ExplicitHealthCheckController(context),
+ NetworkStackClient.getInstance());
}
/**
@@ -143,12 +146,14 @@
*/
@VisibleForTesting
PackageWatchdog(Context context, AtomicFile policyFile, Handler shortTaskHandler,
- Handler longTaskHandler, ExplicitHealthCheckController controller) {
+ Handler longTaskHandler, ExplicitHealthCheckController controller,
+ NetworkStackClient networkStackClient) {
mContext = context;
mPolicyFile = policyFile;
mShortTaskHandler = shortTaskHandler;
mLongTaskHandler = longTaskHandler;
mHealthCheckController = controller;
+ mNetworkStackClient = networkStackClient;
loadFromFile();
}
@@ -174,6 +179,7 @@
() -> syncRequestsAsync());
setPropertyChangedListenerLocked();
updateConfigs();
+ registerNetworkStackHealthListener();
}
}
@@ -630,29 +636,40 @@
synchronized (mLock) {
PackageHealthObserver registeredObserver = observer.mRegisteredObserver;
if (registeredObserver != null) {
- PackageManager pm = mContext.getPackageManager();
Iterator<MonitoredPackage> it = failedPackages.iterator();
while (it.hasNext()) {
String failedPackage = it.next().getName();
- long versionCode = 0;
Slog.i(TAG, "Explicit health check failed for package " + failedPackage);
- try {
- versionCode = pm.getPackageInfo(
- failedPackage, 0 /* flags */).getLongVersionCode();
- } catch (PackageManager.NameNotFoundException e) {
+ VersionedPackage versionedPkg = getVersionedPackage(failedPackage);
+ if (versionedPkg == null) {
Slog.w(TAG, "Explicit health check failed but could not find package "
+ failedPackage);
// TODO(b/120598832): Skip. We only continue to pass tests for now since
// the tests don't install any packages
+ versionedPkg = new VersionedPackage(failedPackage, 0L);
}
- registeredObserver.execute(
- new VersionedPackage(failedPackage, versionCode));
+ registeredObserver.execute(versionedPkg);
}
}
}
});
}
+ @Nullable
+ private VersionedPackage getVersionedPackage(String packageName) {
+ final PackageManager pm = mContext.getPackageManager();
+ if (pm == null) {
+ return null;
+ }
+ try {
+ final long versionCode = pm.getPackageInfo(
+ packageName, 0 /* flags */).getLongVersionCode();
+ return new VersionedPackage(packageName, versionCode);
+ } catch (PackageManager.NameNotFoundException e) {
+ return null;
+ }
+ }
+
/**
* Loads mAllObservers from file.
*
@@ -726,6 +743,27 @@
}
}
+ private void registerNetworkStackHealthListener() {
+ // TODO: have an internal method to trigger a rollback by reporting high severity errors,
+ // and rely on ActivityManager to inform the watchdog of severe network stack crashes
+ // instead of having this listener in parallel.
+ mNetworkStackClient.registerHealthListener(
+ packageName -> {
+ final VersionedPackage pkg = getVersionedPackage(packageName);
+ if (pkg == null) {
+ Slog.wtf(TAG, "NetworkStack failed but could not find its package");
+ return;
+ }
+ // This is a severe failure and recovery should be attempted immediately.
+ // TODO: have a better way to handle such failures.
+ final List<VersionedPackage> pkgList = Collections.singletonList(pkg);
+ final long failureCount = getTriggerFailureCount();
+ for (int i = 0; i < failureCount; i++) {
+ onPackageFailure(pkgList);
+ }
+ });
+ }
+
/**
* Persists mAllObservers to file. Threshold information is ignored.
*/
diff --git a/services/net/java/android/net/NetworkStackClient.java b/services/net/java/android/net/NetworkStackClient.java
index 6b5842f..09c9b6d 100644
--- a/services/net/java/android/net/NetworkStackClient.java
+++ b/services/net/java/android/net/NetworkStackClient.java
@@ -26,22 +26,27 @@
import android.content.Context;
import android.content.Intent;
import android.content.ServiceConnection;
+import android.content.SharedPreferences;
import android.content.pm.PackageManager;
import android.net.dhcp.DhcpServingParamsParcel;
import android.net.dhcp.IDhcpServerCallbacks;
import android.net.ip.IIpClientCallbacks;
import android.net.util.SharedLog;
import android.os.Binder;
-import android.os.Build;
+import android.os.Environment;
import android.os.IBinder;
import android.os.Process;
import android.os.RemoteException;
import android.os.ServiceManager;
+import android.os.SystemClock;
import android.os.UserHandle;
+import android.provider.DeviceConfig;
+import android.util.ArraySet;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
+import java.io.File;
import java.io.PrintWriter;
import java.util.ArrayList;
@@ -54,6 +59,15 @@
private static final int NETWORKSTACK_TIMEOUT_MS = 10_000;
private static final String IN_PROCESS_SUFFIX = ".InProcess";
+ private static final String PREFS_FILE = "NetworkStackClientPrefs.xml";
+ private static final String PREF_KEY_LAST_CRASH_UPTIME = "lastcrash";
+ private static final String CONFIG_MIN_CRASH_INTERVAL_MS = "min_crash_interval";
+
+ // Even if the network stack is lost, do not crash the system more often than this.
+ // Connectivity would be broken, but if the user needs the device for something urgent
+ // (like calling emergency services) we should not bootloop the device.
+ // This is the default value: the actual value can be adjusted via device config.
+ private static final long DEFAULT_MIN_CRASH_INTERVAL_MS = 6 * 3_600_000L;
private static NetworkStackClient sInstance;
@@ -67,12 +81,34 @@
@GuardedBy("mLog")
private final SharedLog mLog = new SharedLog(TAG);
- private volatile boolean mNetworkStackStartRequested = false;
+ private volatile boolean mWasSystemServerInitialized = false;
+
+ /**
+ * If non-zero, indicates that the last framework start happened after a crash of the
+ * NetworkStack which was at the specified uptime.
+ */
+ private volatile long mLastCrashUptime = 0L;
+
+ @GuardedBy("mHealthListeners")
+ private final ArraySet<NetworkStackHealthListener> mHealthListeners = new ArraySet<>();
private interface NetworkStackCallback {
void onNetworkStackConnected(INetworkStackConnector connector);
}
+ /**
+ * Callback interface for severe failures of the NetworkStack.
+ *
+ * <p>Useful for health monitors such as PackageWatchdog.
+ */
+ public interface NetworkStackHealthListener {
+ /**
+ * Called when there is a severe failure of the network stack.
+ * @param packageName Package name of the network stack.
+ */
+ void onNetworkStackFailure(@NonNull String packageName);
+ }
+
private NetworkStackClient() { }
/**
@@ -86,6 +122,15 @@
}
/**
+ * Add a {@link NetworkStackHealthListener} to listen to network stack health events.
+ */
+ public void registerHealthListener(@NonNull NetworkStackHealthListener listener) {
+ synchronized (mHealthListeners) {
+ mHealthListeners.add(listener);
+ }
+ }
+
+ /**
* Create a DHCP server according to the specified parameters.
*
* <p>The server will be returned asynchronously through the provided callbacks.
@@ -147,6 +192,16 @@
}
private class NetworkStackConnection implements ServiceConnection {
+ @NonNull
+ private final Context mContext;
+ @NonNull
+ private final String mPackageName;
+
+ private NetworkStackConnection(@NonNull Context context, @NonNull String packageName) {
+ mContext = context;
+ mPackageName = packageName;
+ }
+
@Override
public void onServiceConnected(ComponentName name, IBinder service) {
logi("Network stack service connected");
@@ -155,14 +210,14 @@
@Override
public void onServiceDisconnected(ComponentName name) {
- // The system has lost its network stack (probably due to a crash in the
- // network stack process): better crash rather than stay in a bad state where all
- // networking is broken.
// onServiceDisconnected is not being called on device shutdown, so this method being
// called always indicates a bad state for the system server.
- maybeCrashWithTerribleFailure("Lost network stack");
+ // This code path is only run by the system server: only the system server binds
+ // to the NetworkStack as a service. Other processes get the NetworkStack from
+ // the ServiceManager.
+ maybeCrashWithTerribleFailure("Lost network stack", mContext, mPackageName);
}
- };
+ }
private void registerNetworkStackService(@NonNull IBinder service) {
final INetworkStackConnector connector = INetworkStackConnector.Stub.asInterface(service);
@@ -189,7 +244,7 @@
*/
public void init() {
log("Network stack init");
- mNetworkStackStartRequested = true;
+ mWasSystemServerInitialized = true;
}
/**
@@ -202,6 +257,13 @@
*/
public void start(Context context) {
log("Starting network stack");
+
+ final SharedPreferences prefs = getSharedPreferences(context);
+ mLastCrashUptime = prefs.getLong(PREF_KEY_LAST_CRASH_UPTIME, 0L);
+ // Remove the preference after getting the last crash uptime, so mLastCrashUptime always
+ // indicates this is the first start since the last crash.
+ prefs.edit().remove(PREF_KEY_LAST_CRASH_UPTIME).commit();
+
final PackageManager pm = context.getPackageManager();
// Try to bind in-process if the device was shipped with an in-process version
@@ -216,16 +278,19 @@
}
if (intent == null) {
- maybeCrashWithTerribleFailure("Could not resolve the network stack");
+ maybeCrashWithTerribleFailure("Could not resolve the network stack", context, null);
return;
}
+ final String packageName = intent.getComponent().getPackageName();
+
// Start the network stack. The service will be added to the service manager in
// NetworkStackConnection.onServiceConnected().
- if (!context.bindServiceAsUser(intent, new NetworkStackConnection(),
+ if (!context.bindServiceAsUser(intent, new NetworkStackConnection(context, packageName),
Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT, UserHandle.SYSTEM)) {
maybeCrashWithTerribleFailure(
- "Could not bind to network stack in-process, or in app with " + intent);
+ "Could not bind to network stack in-process, or in app with " + intent,
+ context, packageName);
return;
}
@@ -274,11 +339,47 @@
}
}
- private void maybeCrashWithTerribleFailure(@NonNull String message) {
+ private void maybeCrashWithTerribleFailure(@NonNull String message,
+ @NonNull Context context, @Nullable String packageName) {
logWtf(message, null);
- if (Build.IS_DEBUGGABLE) {
+ // uptime is monotonic even after a framework restart
+ final long uptime = SystemClock.elapsedRealtime();
+ final long minCrashIntervalMs = DeviceConfig.getLong(DeviceConfig.NAMESPACE_CONNECTIVITY,
+ CONFIG_MIN_CRASH_INTERVAL_MS, DEFAULT_MIN_CRASH_INTERVAL_MS);
+
+ // Either the framework was not restarted after a crash of the NetworkStack, or the min
+ // crash interval has passed since then.
+ if (mLastCrashUptime == 0L || uptime - mLastCrashUptime > minCrashIntervalMs) {
+ // The system is not bound to its network stack (for example due to a crash in the
+ // network stack process): better crash rather than stay in a bad state where all
+ // networking is broken.
+ // Using device-encrypted SharedPreferences as DeviceConfig does not have a synchronous
+ // API to persist settings before a crash.
+ final SharedPreferences prefs = getSharedPreferences(context);
+ if (!prefs.edit().putLong(PREF_KEY_LAST_CRASH_UPTIME, uptime).commit()) {
+ logWtf("Could not persist last crash uptime", null);
+ }
throw new IllegalStateException(message);
}
+
+ // Here the system crashed recently already. Inform listeners that something is
+ // definitely wrong.
+ if (packageName != null) {
+ final ArraySet<NetworkStackHealthListener> listeners;
+ synchronized (mHealthListeners) {
+ listeners = new ArraySet<>(mHealthListeners);
+ }
+ for (NetworkStackHealthListener listener : listeners) {
+ listener.onNetworkStackFailure(packageName);
+ }
+ }
+ }
+
+ private SharedPreferences getSharedPreferences(Context context) {
+ final File prefsFile = new File(
+ Environment.getDataSystemDeDirectory(UserHandle.USER_SYSTEM), PREFS_FILE);
+ return context.createDeviceProtectedStorageContext()
+ .getSharedPreferences(prefsFile, Context.MODE_PRIVATE);
}
/**
@@ -350,7 +451,7 @@
"Only the system server should try to bind to the network stack.");
}
- if (!mNetworkStackStartRequested) {
+ if (!mWasSystemServerInitialized) {
// The network stack is not being started in this process, e.g. this process is not
// the system server. Get a remote connector registered by the system server.
final INetworkStackConnector connector = getRemoteConnector();
diff --git a/tests/PackageWatchdog/Android.bp b/tests/PackageWatchdog/Android.bp
index b079965..88d92c4 100644
--- a/tests/PackageWatchdog/Android.bp
+++ b/tests/PackageWatchdog/Android.bp
@@ -18,11 +18,18 @@
srcs: ["src/**/*.java"],
static_libs: [
"junit",
+ "mockito-target-extended-minus-junit4",
"frameworks-base-testutils",
"androidx.test.rules",
"services.core",
+ "services.net",
],
libs: ["android.test.runner"],
+ jni_libs: [
+ // mockito-target-extended dependencies
+ "libdexmakerjvmtiagent",
+ "libstaticjvmtiagent",
+ ],
platform_apis: true,
test_suites: ["device-tests"],
}
diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
index eb19361..9771637 100644
--- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
+++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
@@ -27,6 +27,7 @@
import android.Manifest;
import android.content.Context;
import android.content.pm.VersionedPackage;
+import android.net.NetworkStackClient;
import android.os.Handler;
import android.os.test.TestLooper;
import android.provider.DeviceConfig;
@@ -41,6 +42,8 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
import java.io.File;
import java.util.ArrayList;
@@ -70,9 +73,12 @@
private static final long SHORT_DURATION = TimeUnit.SECONDS.toMillis(1);
private static final long LONG_DURATION = TimeUnit.SECONDS.toMillis(5);
private TestLooper mTestLooper;
+ @Mock
+ private NetworkStackClient mNetworkStackClient;
@Before
public void setUp() throws Exception {
+ MockitoAnnotations.initMocks(this);
new File(InstrumentationRegistry.getContext().getFilesDir(),
"package-watchdog.xml").delete();
adoptShellPermissions(Manifest.permission.READ_DEVICE_CONFIG);
@@ -732,7 +738,8 @@
new AtomicFile(new File(context.getFilesDir(), "package-watchdog.xml"));
Handler handler = new Handler(mTestLooper.getLooper());
PackageWatchdog watchdog =
- new PackageWatchdog(context, policyFile, handler, handler, controller);
+ new PackageWatchdog(context, policyFile, handler, handler, controller,
+ mNetworkStackClient);
// Verify controller is not automatically started
assertFalse(controller.mIsEnabled);
if (withPackagesReady) {