Do not create EMPTY batterystats history file.
Previously when batterystats history buffer exceeds MAX_HISTORY_BUFFER
size, we create a new history file which is empty until batterystats
history buffer is written to the file. But the buffer is written to the
file every 30 minutes by default. When the file is empty, if the
system server process crashed or abrupt powered off, the file will remain
empty.
During the device bootup, batterystats reads history buffer from
the empty file and failed. The important mHistoryBaseTime variable is zero,
which causes all timestamps in subsequent history events are wrong, this
causes volta go/powerbug displays wrong, also Battery Usage in Settings
are wrong.
The fix is to not create new empty history file on disk until we actually
write history buffer to the file.
This problem can be manually reproduced by creating new empty file under
battery-history directory, then power off the device by long press power
button.
Bug: 133525277
Test: frameworks/base/core/tests/coretests/src/com/android/internal/os/BatteryStatsHistoryTest.java
Change-Id: I0c22881df6897e8832b472cc5e82fbf2727eb252
diff --git a/core/java/com/android/internal/os/BatteryStatsHistory.java b/core/java/com/android/internal/os/BatteryStatsHistory.java
index 24ad751..392f074 100644
--- a/core/java/com/android/internal/os/BatteryStatsHistory.java
+++ b/core/java/com/android/internal/os/BatteryStatsHistory.java
@@ -28,7 +28,6 @@
import java.io.File;
import java.io.FilenameFilter;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -138,11 +137,12 @@
if (!dedup.isEmpty()) {
mFileNumbers.addAll(dedup);
Collections.sort(mFileNumbers);
+ setActiveFile(mFileNumbers.get(mFileNumbers.size() - 1));
} else {
// No file found, default to have file 0.
mFileNumbers.add(0);
+ setActiveFile(0);
}
- createActiveFile();
}
/**
@@ -157,22 +157,15 @@
mHistoryBuffer = historyBuffer;
}
/**
- * The highest numbered history file is active file that mHistoryBuffer is backed up into.
- * If file does not exists, truncate() creates a empty file.
+ * Set the active file that mHistoryBuffer is backed up into.
+ *
+ * @param fileNumber the history file that mHistoryBuffer is backed up into.
*/
- private void createActiveFile() {
- final AtomicFile file = getFile(mFileNumbers.get(mFileNumbers.size() - 1));
+ private void setActiveFile(int fileNumber) {
+ mActiveFile = getFile(fileNumber);
if (DEBUG) {
- Slog.d(TAG, "activeHistoryFile:" + file.getBaseFile().getPath());
+ Slog.d(TAG, "activeHistoryFile:" + mActiveFile.getBaseFile().getPath());
}
- if (!file.exists()) {
- try {
- file.truncate();
- } catch (IOException e) {
- Slog.e(TAG, "Error creating history file "+ file.getBaseFile().getPath(), e);
- }
- }
- mActiveFile = file;
}
/**
@@ -189,7 +182,7 @@
* When {@link #mHistoryBuffer} reaches {@link BatteryStatsImpl.Constants#MAX_HISTORY_BUFFER},
* create next history file.
*/
- public void createNextFile() {
+ public void startNextFile() {
if (mFileNumbers.isEmpty()) {
Slog.wtf(TAG, "mFileNumbers should never be empty");
return;
@@ -198,7 +191,7 @@
// number plus one.
final int next = mFileNumbers.get(mFileNumbers.size() - 1) + 1;
mFileNumbers.add(next);
- createActiveFile();
+ setActiveFile(next);
// if free disk space is less than 100MB, delete oldest history file.
if (!hasFreeDiskSpace()) {
@@ -224,7 +217,7 @@
}
mFileNumbers.clear();
mFileNumbers.add(0);
- createActiveFile();
+ setActiveFile(0);
}
/**
diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java
index c04a249..e52f8e1 100644
--- a/core/java/com/android/internal/os/BatteryStatsImpl.java
+++ b/core/java/com/android/internal/os/BatteryStatsImpl.java
@@ -3685,7 +3685,7 @@
Slog.d(TAG, "addHistoryBufferLocked writeHistoryLocked takes ms:"
+ (SystemClock.uptimeMillis() - start));
}
- mBatteryStatsHistory.createNextFile();
+ mBatteryStatsHistory.startNextFile();
mHistoryBuffer.setDataSize(0);
mHistoryBuffer.setDataPosition(0);
mHistoryBuffer.setDataCapacity(mConstants.MAX_HISTORY_BUFFER / 2);
diff --git a/core/tests/coretests/src/com/android/internal/os/BatteryStatsHistoryTest.java b/core/tests/coretests/src/com/android/internal/os/BatteryStatsHistoryTest.java
index cc0ddb7..cd61011 100644
--- a/core/tests/coretests/src/com/android/internal/os/BatteryStatsHistoryTest.java
+++ b/core/tests/coretests/src/com/android/internal/os/BatteryStatsHistoryTest.java
@@ -22,25 +22,28 @@
import android.content.Context;
import android.os.Parcel;
+import android.util.Log;
import androidx.test.InstrumentationRegistry;
import androidx.test.runner.AndroidJUnit4;
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.MockitoAnnotations;
-import java.io.File;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
/**
* Test BatteryStatsHistory.
*/
@RunWith(AndroidJUnit4.class)
public class BatteryStatsHistoryTest {
+ private static final String TAG = "BatteryStatsHistoryTest";
private static final int MAX_HISTORY_FILES = 32;
private final BatteryStatsImpl mBatteryStatsImpl = new MockBatteryStatsImpl();
private final Parcel mHistoryBuffer = Parcel.obtain();
@@ -53,6 +56,12 @@
Context context = InstrumentationRegistry.getContext();
mSystemDir = context.getDataDir();
mHistoryDir = new File(mSystemDir, BatteryStatsHistory.HISTORY_DIR);
+ String[] files = mHistoryDir.list();
+ if (files != null) {
+ for (int i = 0; i < files.length; i++) {
+ new File(mHistoryDir, files[i]).delete();
+ }
+ }
mHistoryDir.delete();
}
@@ -60,28 +69,32 @@
public void testConstruct() {
BatteryStatsHistory history =
new BatteryStatsHistory(mBatteryStatsImpl, mSystemDir, mHistoryBuffer);
+ createActiveFile(history);
verifyFileNumbers(history, Arrays.asList(0));
verifyActiveFile(history, "0.bin");
}
@Test
- public void testCreateNextFile() {
+ public void testStartNextFile() {
BatteryStatsHistory history =
new BatteryStatsHistory(mBatteryStatsImpl, mSystemDir, mHistoryBuffer);
List<Integer> fileList = new ArrayList<>();
fileList.add(0);
+ createActiveFile(history);
// create file 1 to 31.
for (int i = 1; i < MAX_HISTORY_FILES; i++) {
fileList.add(i);
- history.createNextFile();
+ history.startNextFile();
+ createActiveFile(history);
verifyFileNumbers(history, fileList);
verifyActiveFile(history, i + ".bin");
}
// create file 32
- history.createNextFile();
+ history.startNextFile();
+ createActiveFile(history);
fileList.add(32);
fileList.remove(0);
// verify file 0 is deleted.
@@ -90,7 +103,8 @@
verifyActiveFile(history, "32.bin");
// create file 33
- history.createNextFile();
+ history.startNextFile();
+ createActiveFile(history);
// verify file 1 is deleted
fileList.add(33);
fileList.remove(0);
@@ -108,6 +122,7 @@
verifyActiveFile(history2, "33.bin");
history2.resetAllFiles();
+ createActiveFile(history2);
// verify all existing files are deleted.
for (int i = 2; i < 33; ++i) {
verifyFileDeleted(i + ".bin");
@@ -118,7 +133,8 @@
verifyActiveFile(history2, "0.bin");
// create file 1.
- history2.createNextFile();
+ history2.startNextFile();
+ createActiveFile(history2);
verifyFileNumbers(history2, Arrays.asList(0, 1));
verifyActiveFile(history2, "1.bin");
}
@@ -142,4 +158,13 @@
private void verifyFileDeleted(String file) {
assertFalse(new File(mHistoryDir, file).exists());
}
+
+ private void createActiveFile(BatteryStatsHistory history) {
+ final File file = history.getActiveFile().getBaseFile();
+ try {
+ file.createNewFile();
+ } catch (IOException e) {
+ Log.e(TAG, "Error creating history file " + file.getPath(), e);
+ }
+ }
}
\ No newline at end of file