Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e40781ee authored by Hui Yu's avatar Hui Yu
Browse files

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
parent be8a41a6

core/java/com/android/internal/os/BatteryStatsHistory.java

+11 −18
Original line number Original line Diff line number Diff line
@@ -28,7 +28,6 @@ import com.android.internal.util.ParseUtils; 




import java.io.File;

import java.io.File;

import java.io.FilenameFilter;

import java.io.FilenameFilter;

import java.io.IOException;

import java.util.ArrayList;

import java.util.ArrayList;

import java.util.Collections;

import java.util.Collections;

import java.util.List;

import java.util.List;
@@ -138,11 +137,12 @@ public class BatteryStatsHistory { 
        if (!dedup.isEmpty()) {

        if (!dedup.isEmpty()) {

            mFileNumbers.addAll(dedup);

            mFileNumbers.addAll(dedup);

            Collections.sort(mFileNumbers);

            Collections.sort(mFileNumbers);

            setActiveFile(mFileNumbers.get(mFileNumbers.size() - 1));

        } else {

        } else {

            // No file found, default to have file 0.

            // No file found, default to have file 0.

            mFileNumbers.add(0);

            mFileNumbers.add(0);

            setActiveFile(0);

        }

        }

        createActiveFile();

    }

    }





    /**

    /**
@@ -157,22 +157,15 @@ public class BatteryStatsHistory { 
        mHistoryBuffer = historyBuffer;

        mHistoryBuffer = historyBuffer;

    }

    }

    /**

    /**

     * The highest numbered history file is active file that mHistoryBuffer is backed up into.

     * Set the active file that mHistoryBuffer is backed up into.

     * If file does not exists, truncate() creates a empty file.

     *

     * @param fileNumber the history file that mHistoryBuffer is backed up into.

     */

     */

    private void createActiveFile() {

    private void setActiveFile(int fileNumber) {

        final AtomicFile file = getFile(mFileNumbers.get(mFileNumbers.size() - 1));

        mActiveFile = getFile(fileNumber);

        if (DEBUG) {

        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 @@ public class BatteryStatsHistory { 
     * When {@link #mHistoryBuffer} reaches {@link BatteryStatsImpl.Constants#MAX_HISTORY_BUFFER},

     * When {@link #mHistoryBuffer} reaches {@link BatteryStatsImpl.Constants#MAX_HISTORY_BUFFER},

     * create next history file.

     * create next history file.

     */

     */

    public void createNextFile() {

    public void startNextFile() {

        if (mFileNumbers.isEmpty()) {

        if (mFileNumbers.isEmpty()) {

            Slog.wtf(TAG, "mFileNumbers should never be empty");

            Slog.wtf(TAG, "mFileNumbers should never be empty");

            return;

            return;
@@ -198,7 +191,7 @@ public class BatteryStatsHistory { 
        // number plus one.

        // number plus one.

        final int next = mFileNumbers.get(mFileNumbers.size() - 1) + 1;

        final int next = mFileNumbers.get(mFileNumbers.size() - 1) + 1;

        mFileNumbers.add(next);

        mFileNumbers.add(next);

        createActiveFile();

        setActiveFile(next);





        // if free disk space is less than 100MB, delete oldest history file.

        // if free disk space is less than 100MB, delete oldest history file.

        if (!hasFreeDiskSpace()) {

        if (!hasFreeDiskSpace()) {
@@ -224,7 +217,7 @@ public class BatteryStatsHistory { 
        }

        }

        mFileNumbers.clear();

        mFileNumbers.clear();

        mFileNumbers.add(0);

        mFileNumbers.add(0);

        createActiveFile();

        setActiveFile(0);

    }

    }





    /**

    /**

core/java/com/android/internal/os/BatteryStatsImpl.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -3685,7 +3685,7 @@ public class BatteryStatsImpl extends BatteryStats { 
                Slog.d(TAG, "addHistoryBufferLocked writeHistoryLocked takes ms:"

                Slog.d(TAG, "addHistoryBufferLocked writeHistoryLocked takes ms:"

                        + (SystemClock.uptimeMillis() - start));

                        + (SystemClock.uptimeMillis() - start));

            }

            }

            mBatteryStatsHistory.createNextFile();

            mBatteryStatsHistory.startNextFile();

            mHistoryBuffer.setDataSize(0);

            mHistoryBuffer.setDataSize(0);

            mHistoryBuffer.setDataPosition(0);

            mHistoryBuffer.setDataPosition(0);

            mHistoryBuffer.setDataCapacity(mConstants.MAX_HISTORY_BUFFER / 2);

            mHistoryBuffer.setDataCapacity(mConstants.MAX_HISTORY_BUFFER / 2);

core/tests/coretests/src/com/android/internal/os/BatteryStatsHistoryTest.java

+35 −10
Original line number Original line Diff line number Diff line
@@ -22,25 +22,28 @@ import static org.junit.Assert.assertTrue; 




import android.content.Context;

import android.content.Context;

import android.os.Parcel;

import android.os.Parcel;

import android.util.Log;





import androidx.test.InstrumentationRegistry;

import androidx.test.InstrumentationRegistry;

import androidx.test.runner.AndroidJUnit4;

import androidx.test.runner.AndroidJUnit4;





import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.MockitoAnnotations;



import java.io.File;

import java.io.File;

import java.io.IOException;

import java.util.ArrayList;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Arrays;

import java.util.List;

import java.util.List;





import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.MockitoAnnotations;



/**

/**

 * Test BatteryStatsHistory.

 * Test BatteryStatsHistory.

 */

 */

@RunWith(AndroidJUnit4.class)

@RunWith(AndroidJUnit4.class)

public class BatteryStatsHistoryTest {

public class BatteryStatsHistoryTest {

    private static final String TAG = "BatteryStatsHistoryTest";

    private static final int MAX_HISTORY_FILES = 32;

    private static final int MAX_HISTORY_FILES = 32;

    private final BatteryStatsImpl mBatteryStatsImpl = new MockBatteryStatsImpl();

    private final BatteryStatsImpl mBatteryStatsImpl = new MockBatteryStatsImpl();

    private final Parcel mHistoryBuffer = Parcel.obtain();

    private final Parcel mHistoryBuffer = Parcel.obtain();
@@ -53,6 +56,12 @@ public class BatteryStatsHistoryTest { 
        Context context = InstrumentationRegistry.getContext();

        Context context = InstrumentationRegistry.getContext();

        mSystemDir = context.getDataDir();

        mSystemDir = context.getDataDir();

        mHistoryDir = new File(mSystemDir, BatteryStatsHistory.HISTORY_DIR);

        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();

        mHistoryDir.delete();

    }

    }
@@ -60,28 +69,32 @@ public class BatteryStatsHistoryTest { 
    public void testConstruct() {

    public void testConstruct() {

        BatteryStatsHistory history =

        BatteryStatsHistory history =

                new BatteryStatsHistory(mBatteryStatsImpl, mSystemDir, mHistoryBuffer);

                new BatteryStatsHistory(mBatteryStatsImpl, mSystemDir, mHistoryBuffer);

        createActiveFile(history);

        verifyFileNumbers(history, Arrays.asList(0));

        verifyFileNumbers(history, Arrays.asList(0));

        verifyActiveFile(history, "0.bin");

        verifyActiveFile(history, "0.bin");

    }

    }





    @Test

    @Test

    public void testCreateNextFile() {

    public void testStartNextFile() {

        BatteryStatsHistory history =

        BatteryStatsHistory history =

                new BatteryStatsHistory(mBatteryStatsImpl, mSystemDir, mHistoryBuffer);

                new BatteryStatsHistory(mBatteryStatsImpl, mSystemDir, mHistoryBuffer);





        List<Integer> fileList = new ArrayList<>();

        List<Integer> fileList = new ArrayList<>();

        fileList.add(0);

        fileList.add(0);

        createActiveFile(history);





        // create file 1 to 31.

        // create file 1 to 31.

        for (int i = 1; i < MAX_HISTORY_FILES; i++) {

        for (int i = 1; i < MAX_HISTORY_FILES; i++) {

            fileList.add(i);

            fileList.add(i);

            history.createNextFile();

            history.startNextFile();

            createActiveFile(history);

            verifyFileNumbers(history, fileList);

            verifyFileNumbers(history, fileList);

            verifyActiveFile(history, i + ".bin");

            verifyActiveFile(history, i + ".bin");

        }

        }





        // create file 32

        // create file 32

        history.createNextFile();

        history.startNextFile();

        createActiveFile(history);

        fileList.add(32);

        fileList.add(32);

        fileList.remove(0);

        fileList.remove(0);

        // verify file 0 is deleted.

        // verify file 0 is deleted.
@@ -90,7 +103,8 @@ public class BatteryStatsHistoryTest { 
        verifyActiveFile(history, "32.bin");

        verifyActiveFile(history, "32.bin");





        // create file 33

        // create file 33

        history.createNextFile();

        history.startNextFile();

        createActiveFile(history);

        // verify file 1 is deleted

        // verify file 1 is deleted

        fileList.add(33);

        fileList.add(33);

        fileList.remove(0);

        fileList.remove(0);
@@ -108,6 +122,7 @@ public class BatteryStatsHistoryTest { 
        verifyActiveFile(history2, "33.bin");

        verifyActiveFile(history2, "33.bin");





        history2.resetAllFiles();

        history2.resetAllFiles();

        createActiveFile(history2);

        // verify all existing files are deleted.

        // verify all existing files are deleted.

        for (int i = 2; i < 33; ++i) {

        for (int i = 2; i < 33; ++i) {

            verifyFileDeleted(i + ".bin");

            verifyFileDeleted(i + ".bin");
@@ -118,7 +133,8 @@ public class BatteryStatsHistoryTest { 
        verifyActiveFile(history2, "0.bin");

        verifyActiveFile(history2, "0.bin");





        // create file 1.

        // create file 1.

        history2.createNextFile();

        history2.startNextFile();

        createActiveFile(history2);

        verifyFileNumbers(history2, Arrays.asList(0, 1));

        verifyFileNumbers(history2, Arrays.asList(0, 1));

        verifyActiveFile(history2, "1.bin");

        verifyActiveFile(history2, "1.bin");

    }

    }
@@ -142,4 +158,13 @@ public class BatteryStatsHistoryTest { 
    private void verifyFileDeleted(String file) {

    private void verifyFileDeleted(String file) {

        assertFalse(new File(mHistoryDir, file).exists());

        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