From 8550f255232eb4e4852466c5297fdc125887f5af Mon Sep 17 00:00:00 2001 From: Suchi Amalapurapu Date: Tue, 29 Sep 2009 15:20:32 -0700 Subject: [PATCH] Check if rename of backed up file fails before persisting new changes. If not these system services will end up with inconsistent settings files when the device runs out of storage. Delete mangled settings file in PackageManager if the current write fails so that we don't end up overwriting the backed up version with the mangled version Include null check when retrieving fwd locked resource for an existing package --- core/java/android/app/ApplicationContext.java | 1 + .../com/android/internal/os/BatteryStatsImpl.java | 13 +++++++++++-- .../java/com/android/server/AppWidgetService.java | 9 +++++++-- .../com/android/server/PackageManagerService.java | 12 ++++++++---- .../com/android/server/am/UsageStatsService.java | 5 ++++- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/core/java/android/app/ApplicationContext.java b/core/java/android/app/ApplicationContext.java index 0582e34ae5ba2..8ba7c017e1e41 100644 --- a/core/java/android/app/ApplicationContext.java +++ b/core/java/android/app/ApplicationContext.java @@ -2760,6 +2760,7 @@ class ApplicationContext extends Context { if (mFile.exists()) { if (!mFile.renameTo(mBackupFile)) { Log.e(TAG, "Couldn't rename file " + mFile + " to backup file " + mBackupFile); + return false; } } diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 7a8a3bec8defb..4b26b8f5552f8 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -2988,7 +2988,10 @@ public final class BatteryStatsImpl extends BatteryStats { if (mBackupFile.exists()) { mBackupFile.delete(); } - mFile.renameTo(mBackupFile); + if (!mFile.renameTo(mBackupFile)) { + Log.w("BatteryStats", "Failed to back up file before writing new stats"); + return; + } } try { @@ -3003,8 +3006,14 @@ public final class BatteryStatsImpl extends BatteryStats { mBackupFile.delete(); mLastWriteTime = SystemClock.elapsedRealtime(); + return; } catch (IOException e) { - Log.e("BatteryStats", "Error writing battery statistics", e); + Log.w("BatteryStats", "Error writing battery statistics", e); + } + if (mFile.exists()) { + if (!mFile.delete()) { + Log.w(TAG, "Failed to delete mangled file " + mFile); + } } } diff --git a/services/java/com/android/server/AppWidgetService.java b/services/java/com/android/server/AppWidgetService.java index 3c469546081db..f8b8ecc91a0a6 100644 --- a/services/java/com/android/server/AppWidgetService.java +++ b/services/java/com/android/server/AppWidgetService.java @@ -816,7 +816,10 @@ class AppWidgetService extends IAppWidgetService.Stub temp.delete(); } - writeStateToFileLocked(temp); + if (!writeStateToFileLocked(temp)) { + Log.w(TAG, "Failed to persist new settings"); + return; + } //noinspection ResultOfMethodCallIgnored real.delete(); @@ -824,7 +827,7 @@ class AppWidgetService extends IAppWidgetService.Stub temp.renameTo(real); } - void writeStateToFileLocked(File file) { + boolean writeStateToFileLocked(File file) { FileOutputStream stream = null; int N; @@ -877,6 +880,7 @@ class AppWidgetService extends IAppWidgetService.Stub out.endDocument(); stream.close(); + return true; } catch (IOException e) { try { if (stream != null) { @@ -889,6 +893,7 @@ class AppWidgetService extends IAppWidgetService.Stub //noinspection ResultOfMethodCallIgnored file.delete(); } + return false; } } diff --git a/services/java/com/android/server/PackageManagerService.java b/services/java/com/android/server/PackageManagerService.java index 2f16b8491687e..5eb78c3dc6dad 100644 --- a/services/java/com/android/server/PackageManagerService.java +++ b/services/java/com/android/server/PackageManagerService.java @@ -2049,7 +2049,7 @@ class PackageManagerService extends IPackageManager.Stub { scanMode |= SCAN_FORWARD_LOCKED; } File resFile = destResourceFile; - if ((scanMode & SCAN_FORWARD_LOCKED) != 0) { + if (ps != null && ((scanMode & SCAN_FORWARD_LOCKED) != 0)) { resFile = getFwdLockedResource(ps.name); } // Note that we invoke the following method only if we are about to unpack an application @@ -6529,15 +6529,19 @@ class PackageManagerService extends IPackageManager.Stub { |FileUtils.S_IRGRP|FileUtils.S_IWGRP |FileUtils.S_IROTH, -1, -1); + return; } catch(XmlPullParserException e) { Log.w(TAG, "Unable to write package manager settings, current changes will be lost at reboot", e); - } catch(java.io.IOException e) { Log.w(TAG, "Unable to write package manager settings, current changes will be lost at reboot", e); - } - + // Clean up partially written file + if (mSettingsFilename.exists()) { + if (!mSettingsFilename.delete()) { + Log.i(TAG, "Failed to clean up mangled file: " + mSettingsFilename); + } + } //Debug.stopMethodTracing(); } diff --git a/services/java/com/android/server/am/UsageStatsService.java b/services/java/com/android/server/am/UsageStatsService.java index 66868a387bca4..373b44e4aa26c 100644 --- a/services/java/com/android/server/am/UsageStatsService.java +++ b/services/java/com/android/server/am/UsageStatsService.java @@ -381,7 +381,10 @@ public final class UsageStatsService extends IUsageStats.Stub { mFileLeaf = getCurrentDateStr(FILE_PREFIX); // Copy current file to back up File backupFile = new File(mFile.getPath() + ".bak"); - mFile.renameTo(backupFile); + if (!mFile.renameTo(backupFile)) { + Log.w(TAG, "Failed to persist new stats"); + return; + } try { // Write mStats to file writeStatsFLOCK();