From 5886ede6077b024f3dbc74fcd9c8c0c13aa400d8 Mon Sep 17 00:00:00 2001
From: Michael Sun <michaelfsun@google.com>
Date: Mon, 21 Dec 2020 07:29:39 +0000
Subject: [PATCH] KernelWakelockReader: provide class level lock when updates
 staleStats

As the wakelock version number is defined as static to provide
consistent versioning across objects, class level static lock should be
implemented to prevent racing conditions.

To trigger the racing condition, update statsd's stats pulling logic
locally to repeatably requesting wakelock stats then wakeup the phone
to trigger BatteryStats update routine. The racing condition is 100%
reproducible under the setup. The patch has been verified with the
setup, and the racing is no longer seen. See more reproduce details in
the linked bug.

Bug: 173539101
Test: manual
Change-Id: I386afa2f2ecd8678e71ece978da4a9950b21ca4d
---
 .../internal/os/KernelWakelockReader.java     | 32 +++++++++++--------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/core/java/com/android/internal/os/KernelWakelockReader.java b/core/java/com/android/internal/os/KernelWakelockReader.java
index e595db384cb98..c4168141f1841 100644
--- a/core/java/com/android/internal/os/KernelWakelockReader.java
+++ b/core/java/com/android/internal/os/KernelWakelockReader.java
@@ -78,13 +78,16 @@ public class KernelWakelockReader {
         boolean useSystemSuspend = (new File(sSysClassWakeupDir)).exists();
 
         if (useSystemSuspend) {
-            // Get both kernel and native wakelock stats from SystemSuspend
-            updateVersion(staleStats);
-            if (getWakelockStatsFromSystemSuspend(staleStats) == null) {
-                Slog.w(TAG, "Failed to get wakelock stats from SystemSuspend");
-                return null;
+            // static read/write lock protection for sKernelWakelockUpdateVersion
+            synchronized (KernelWakelockReader.class) {
+                // Get both kernel and native wakelock stats from SystemSuspend
+                updateVersion(staleStats);
+                if (getWakelockStatsFromSystemSuspend(staleStats) == null) {
+                    Slog.w(TAG, "Failed to get wakelock stats from SystemSuspend");
+                    return null;
+                }
+                return removeOldStats(staleStats);
             }
-            return removeOldStats(staleStats);
         } else {
             Arrays.fill(mKernelWakelockBuffer, (byte) 0);
             int len = 0;
@@ -141,14 +144,17 @@ public class KernelWakelockReader {
                 }
             }
 
-            updateVersion(staleStats);
-            // Get native wakelock stats from SystemSuspend
-            if (getWakelockStatsFromSystemSuspend(staleStats) == null) {
-                Slog.w(TAG, "Failed to get Native wakelock stats from SystemSuspend");
+            // static read/write lock protection for sKernelWakelockUpdateVersion
+            synchronized (KernelWakelockReader.class) {
+                updateVersion(staleStats);
+                // Get native wakelock stats from SystemSuspend
+                if (getWakelockStatsFromSystemSuspend(staleStats) == null) {
+                    Slog.w(TAG, "Failed to get Native wakelock stats from SystemSuspend");
+                }
+                // Get kernel wakelock stats
+                parseProcWakelocks(mKernelWakelockBuffer, len, wakeup_sources, staleStats);
+                return removeOldStats(staleStats);
             }
-            // Get kernel wakelock stats
-            parseProcWakelocks(mKernelWakelockBuffer, len, wakeup_sources, staleStats);
-            return removeOldStats(staleStats);
         }
     }