From df49541a0914ba34f0b0f5817739fdc2f9e84a12 Mon Sep 17 00:00:00 2001 From: Michael Wachenschwanz Date: Tue, 23 Aug 2022 16:23:46 -0700 Subject: [PATCH] Lock access to PowerStatsLogger data Also add exceptions to error logs in PowerStatsDataStorage. Bug: 237470734 Test: atest FrameworksServicesTests:PowerStatsServiceTest Change-Id: I37933fc44a3b85d2469f02bb39bad6d6dffbd818 --- .../powerstats/PowerStatsDataStorage.java | 33 ++++++++++++------- .../server/powerstats/PowerStatsLogger.java | 16 ++++----- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/powerstats/PowerStatsDataStorage.java b/services/core/java/com/android/server/powerstats/PowerStatsDataStorage.java index 06253a08d9377..8b30995404f0c 100644 --- a/services/core/java/com/android/server/powerstats/PowerStatsDataStorage.java +++ b/services/core/java/com/android/server/powerstats/PowerStatsDataStorage.java @@ -220,18 +220,17 @@ public class PowerStatsDataStorage { public void write(byte[] data) { if (data != null && data.length > 0) { mLock.lock(); - - long currentTimeMillis = System.currentTimeMillis(); try { + long currentTimeMillis = System.currentTimeMillis(); DataElement dataElement = new DataElement(data); mFileRotator.rewriteActive(new DataRewriter(dataElement.toByteArray()), currentTimeMillis); mFileRotator.maybeRotate(currentTimeMillis); } catch (IOException e) { Slog.e(TAG, "Failed to write to on-device storage: " + e); + } finally { + mLock.unlock(); } - - mLock.unlock(); } } @@ -240,21 +239,31 @@ public class PowerStatsDataStorage { * DataElement retrieved from on-device storage, callback is called. */ public void read(DataElementReadCallback callback) throws IOException { - mFileRotator.readMatching(new DataReader(callback), Long.MIN_VALUE, Long.MAX_VALUE); + mLock.lock(); + try { + mFileRotator.readMatching(new DataReader(callback), Long.MIN_VALUE, Long.MAX_VALUE); + } finally { + mLock.unlock(); + } } /** * Deletes all stored log data. */ public void deleteLogs() { - File[] files = mDataStorageDir.listFiles(); - for (int i = 0; i < files.length; i++) { - int versionDot = mDataStorageFilename.lastIndexOf('.'); - String beforeVersionDot = mDataStorageFilename.substring(0, versionDot); - // Check that the stems before the version match. - if (files[i].getName().startsWith(beforeVersionDot)) { - files[i].delete(); + mLock.lock(); + try { + File[] files = mDataStorageDir.listFiles(); + for (int i = 0; i < files.length; i++) { + int versionDot = mDataStorageFilename.lastIndexOf('.'); + String beforeVersionDot = mDataStorageFilename.substring(0, versionDot); + // Check that the stems before the version match. + if (files[i].getName().startsWith(beforeVersionDot)) { + files[i].delete(); + } } + } finally { + mLock.unlock(); } } } diff --git a/services/core/java/com/android/server/powerstats/PowerStatsLogger.java b/services/core/java/com/android/server/powerstats/PowerStatsLogger.java index ca675973b2fdd..39ead13b03fec 100644 --- a/services/core/java/com/android/server/powerstats/PowerStatsLogger.java +++ b/services/core/java/com/android/server/powerstats/PowerStatsLogger.java @@ -159,12 +159,12 @@ public final class PowerStatsLogger extends Handler { EnergyMeasurementUtils.packProtoMessage(energyMeasurement, pos); if (DEBUG) EnergyMeasurementUtils.print(energyMeasurement); } catch (IOException e) { - Slog.e(TAG, "Failed to write energy meter data to incident report."); + Slog.e(TAG, "Failed to write energy meter data to incident report.", e); } } }); } catch (IOException e) { - Slog.e(TAG, "Failed to write energy meter info to incident report."); + Slog.e(TAG, "Failed to write energy meter info to incident report.", e); } pos.flush(); @@ -200,12 +200,12 @@ public final class PowerStatsLogger extends Handler { EnergyConsumerResultUtils.packProtoMessage(energyConsumerResult, pos, true); if (DEBUG) EnergyConsumerResultUtils.print(energyConsumerResult); } catch (IOException e) { - Slog.e(TAG, "Failed to write energy model data to incident report."); + Slog.e(TAG, "Failed to write energy model data to incident report.", e); } } }); } catch (IOException e) { - Slog.e(TAG, "Failed to write energy model info to incident report."); + Slog.e(TAG, "Failed to write energy model info to incident report.", e); } pos.flush(); @@ -241,12 +241,12 @@ public final class PowerStatsLogger extends Handler { StateResidencyResultUtils.packProtoMessage(stateResidencyResult, pos); if (DEBUG) StateResidencyResultUtils.print(stateResidencyResult); } catch (IOException e) { - Slog.e(TAG, "Failed to write residency data to incident report."); + Slog.e(TAG, "Failed to write residency data to incident report.", e); } } }); } catch (IOException e) { - Slog.e(TAG, "Failed to write residency data to incident report."); + Slog.e(TAG, "Failed to write residency data to incident report.", e); } pos.flush(); @@ -267,7 +267,7 @@ public final class PowerStatsLogger extends Handler { final FileInputStream fis = new FileInputStream(cachedFile.getPath()); fis.read(dataCached); } catch (IOException e) { - Slog.e(TAG, "Failed to read cached data from file"); + Slog.e(TAG, "Failed to read cached data from file", e); } // If the cached and current data are different, delete the data store. @@ -291,7 +291,7 @@ public final class PowerStatsLogger extends Handler { fos.write(data); atomicCachedFile.finishWrite(fos); } catch (IOException e) { - Slog.e(TAG, "Failed to write current data to cached file"); + Slog.e(TAG, "Failed to write current data to cached file", e); } }