From d23af5ed3004d08f92acf1f6f83b4b4b9d91c978 Mon Sep 17 00:00:00 2001
From: Andrei Onea <andreionea@google.com>
Date: Thu, 17 Jun 2021 14:15:03 +0000
Subject: [PATCH] Attempt to fix deadlock on saveOverrides

Currently, the lock on mChanges is held continuously while saving the
changes to disk, however if the serialization triggers an OomAdjustLSP,
that triggers yet another call to the compat framework - causing a
deadlock.
Currently, OomAdjust happens during XML serialisation, so releasing the
lock after we've collected all the data should fix the deadlock, however
there could be issues if OomAdjust happens while collecting the
overrides themselves. That seems somewhat unlikely, and would likely
signal a larger issue.

Test: atest CompatConfigTest
Bug: 190611974
Change-Id: I01e4802baeecf2757044609b91b7b7a40e728d4e
---
 .../android/server/compat/CompatConfig.java   | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/services/core/java/com/android/server/compat/CompatConfig.java b/services/core/java/com/android/server/compat/CompatConfig.java
index 6dca00191b24d..c2375351aee9f 100644
--- a/services/core/java/com/android/server/compat/CompatConfig.java
+++ b/services/core/java/com/android/server/compat/CompatConfig.java
@@ -257,8 +257,8 @@ final class CompatConfig {
                 addChange(c);
             }
             c.addPackageOverride(packageName, overrides, allowedState, versionCode);
-            invalidateCache();
         }
+        invalidateCache();
         return alreadyKnown;
     }
 
@@ -379,9 +379,9 @@ final class CompatConfig {
                 CompatChange change = mChanges.valueAt(i);
                 removeOverrideUnsafe(change, packageName, versionCode);
             }
-            saveOverrides();
-            invalidateCache();
         }
+        saveOverrides();
+        invalidateCache();
     }
 
     /**
@@ -626,7 +626,18 @@ final class CompatConfig {
         if (mOverridesFile == null) {
             return;
         }
+        Overrides overrides = new Overrides();
         synchronized (mChanges) {
+            List<ChangeOverrides> changeOverridesList = overrides.getChangeOverrides();
+            for (int idx = 0; idx < mChanges.size(); ++idx) {
+                CompatChange c = mChanges.valueAt(idx);
+                ChangeOverrides changeOverrides = c.saveOverrides();
+                if (changeOverrides != null) {
+                    changeOverridesList.add(changeOverrides);
+                }
+            }
+        }
+        synchronized (mOverridesFile) {
             // Create the file if it doesn't already exist
             try {
                 mOverridesFile.createNewFile();
@@ -636,15 +647,6 @@ final class CompatConfig {
             }
             try (PrintWriter out = new PrintWriter(mOverridesFile)) {
                 XmlWriter writer = new XmlWriter(out);
-                Overrides overrides = new Overrides();
-                List<ChangeOverrides> changeOverridesList = overrides.getChangeOverrides();
-                for (int idx = 0; idx < mChanges.size(); ++idx) {
-                    CompatChange c = mChanges.valueAt(idx);
-                    ChangeOverrides changeOverrides = c.saveOverrides();
-                    if (changeOverrides != null) {
-                        changeOverridesList.add(changeOverrides);
-                    }
-                }
                 XmlWriter.write(writer, overrides);
             } catch (IOException e) {
                 Slog.e(TAG, e.toString());