PackageWatchdog listens for NetworkStack failures

In addition to the NetworkStack app monitoring, have PackageWatchdog
register an observer to NetworkStackClient to receive severe failure
notifications, and attempt a rollback if available.

The callback is registered in onPackagesReady(), which is called in the
boot sequence just before starting the NetworkStack.

Test: installed new networkstack, killed it twice, observe rollback
Test: unit test in change on top
Bug: 133725814
Change-Id: I2cb4200b78c2482cacc4bfe2ace1581b869be512

3 files changed, 63 insertions, 11 deletions

diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java
index 32671144aee3..dee89e5d5c4d 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.annotation.Nullable;
 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 @@ public class PackageWatchdog {
     // 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 @@ public class PackageWatchdog {
                         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 @@ public class PackageWatchdog {
      */
     @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 @@ public class PackageWatchdog {
                     () -> syncRequestsAsync());
             setPropertyChangedListenerLocked();
             updateConfigs();
+            registerNetworkStackHealthListener();
         }
     }
 
@@ -630,29 +636,40 @@ public class PackageWatchdog {
             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 @@ public class PackageWatchdog {
         }
     }
 
+    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/tests/PackageWatchdog/Android.bp b/tests/PackageWatchdog/Android.bp
index b07996568839..88d92c4d94fe 100644
--- a/tests/PackageWatchdog/Android.bp
+++ b/tests/PackageWatchdog/Android.bp
@@ -18,11 +18,18 @@ android_test {
     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 eb19361d86a3..97716377ea2e 100644
--- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
+++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
@@ -27,6 +27,7 @@ import static org.junit.Assert.fail;
 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 com.android.server.PackageWatchdog.PackageHealthObserverImpact;
 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 @@ public class PackageWatchdogTest {
     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 @@ public class PackageWatchdogTest {
                 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) {