diff --git a/core/java/android/util/AtomicFile.java b/core/java/android/util/AtomicFile.java index cf7ed9b0566dd..da7503d014289 100644 --- a/core/java/android/util/AtomicFile.java +++ b/core/java/android/util/AtomicFile.java @@ -29,31 +29,32 @@ import java.io.IOException; import java.util.function.Consumer; /** - * Helper class for performing atomic operations on a file by creating a - * backup file until a write has successfully completed. If you need this - * on older versions of the platform you can use - * {@link android.support.v4.util.AtomicFile} in the v4 support library. + * Helper class for performing atomic operations on a file by writing to a new file and renaming it + * into the place of the original file after the write has successfully completed. If you need this + * on older versions of the platform you can use {@link androidx.core.util.AtomicFile} in AndroidX. *
- * Atomic file guarantees file integrity by ensuring that a file has - * been completely written and sync'd to disk before removing its backup. - * As long as the backup file exists, the original file is considered - * to be invalid (left over from a previous attempt to write the file). - *
- * Atomic file does not confer any file locking semantics. - * Do not use this class when the file may be accessed or modified concurrently - * by multiple threads or processes. The caller is responsible for ensuring - * appropriate mutual exclusion invariants whenever it accesses the file. - *
+ * Atomic file guarantees file integrity by ensuring that a file has been completely written and + * sync'd to disk before renaming it to the original file. Previously this is done by renaming the + * original file to a backup file beforehand, but this approach couldn't handle the case where the + * file is created for the first time. This class will also handle the backup file created by the + * old implementation properly. + *+ * Atomic file does not confer any file locking semantics. Do not use this class when the file may + * be accessed or modified concurrently by multiple threads or processes. The caller is responsible + * for ensuring appropriate mutual exclusion invariants whenever it accesses the file. */ public class AtomicFile { + private static final String LOG_TAG = "AtomicFile"; + private final File mBaseName; - private final File mBackupName; + private final File mNewName; + private final File mLegacyBackupName; private final String mCommitTag; private long mStartTime; /** * Create a new AtomicFile for a file located at the given File path. - * The secondary backup file will be the same file path with ".bak" appended. + * The new file created when writing will be the same file path with ".new" appended. */ public AtomicFile(File baseName) { this(baseName, null); @@ -65,7 +66,8 @@ public class AtomicFile { */ public AtomicFile(File baseName, String commitTag) { mBaseName = baseName; - mBackupName = new File(baseName.getPath() + ".bak"); + mNewName = new File(baseName.getPath() + ".new"); + mLegacyBackupName = new File(baseName.getPath() + ".bak"); mCommitTag = commitTag; } @@ -78,11 +80,12 @@ public class AtomicFile { } /** - * Delete the atomic file. This deletes both the base and backup files. + * Delete the atomic file. This deletes both the base and new files. */ public void delete() { mBaseName.delete(); - mBackupName.delete(); + mNewName.delete(); + mLegacyBackupName.delete(); } /** @@ -112,36 +115,28 @@ public class AtomicFile { public FileOutputStream startWrite(long startTime) throws IOException { mStartTime = startTime; - // Rename the current file so it may be used as a backup during the next read - if (mBaseName.exists()) { - if (!mBackupName.exists()) { - if (!mBaseName.renameTo(mBackupName)) { - Log.w("AtomicFile", "Couldn't rename file " + mBaseName - + " to backup file " + mBackupName); - } - } else { - mBaseName.delete(); + if (mLegacyBackupName.exists()) { + if (!mLegacyBackupName.renameTo(mBaseName)) { + Log.e(LOG_TAG, "Failed to rename legacy backup file " + mLegacyBackupName + + " to base file " + mBaseName); } } - FileOutputStream str = null; + try { - str = new FileOutputStream(mBaseName); + return new FileOutputStream(mNewName); } catch (FileNotFoundException e) { - File parent = mBaseName.getParentFile(); + File parent = mNewName.getParentFile(); if (!parent.mkdirs()) { - throw new IOException("Couldn't create directory " + mBaseName); + throw new IOException("Failed to create directory for " + mNewName); } - FileUtils.setPermissions( - parent.getPath(), - FileUtils.S_IRWXU|FileUtils.S_IRWXG|FileUtils.S_IXOTH, - -1, -1); + FileUtils.setPermissions(parent.getPath(), FileUtils.S_IRWXU | FileUtils.S_IRWXG + | FileUtils.S_IXOTH, -1, -1); try { - str = new FileOutputStream(mBaseName); + return new FileOutputStream(mNewName); } catch (FileNotFoundException e2) { - throw new IOException("Couldn't create " + mBaseName); + throw new IOException("Failed to create new file " + mNewName, e2); } } - return str; } /** @@ -151,36 +146,45 @@ public class AtomicFile { * will return the new file stream. */ public void finishWrite(FileOutputStream str) { - if (str != null) { - FileUtils.sync(str); - try { - str.close(); - mBackupName.delete(); - } catch (IOException e) { - Log.w("AtomicFile", "finishWrite: Got exception:", e); - } - if (mCommitTag != null) { - com.android.internal.logging.EventLogTags.writeCommitSysConfigFile( - mCommitTag, SystemClock.uptimeMillis() - mStartTime); - } + if (str == null) { + return; + } + if (!FileUtils.sync(str)) { + Log.e(LOG_TAG, "Failed to sync file output stream"); + } + try { + str.close(); + } catch (IOException e) { + Log.e(LOG_TAG, "Failed to close file output stream", e); + } + if (!mNewName.renameTo(mBaseName)) { + Log.e(LOG_TAG, "Failed to rename new file " + mNewName + " to base file " + mBaseName); + } + if (mCommitTag != null) { + com.android.internal.logging.EventLogTags.writeCommitSysConfigFile( + mCommitTag, SystemClock.uptimeMillis() - mStartTime); } } /** * Call when you have failed for some reason at writing to the stream * returned by {@link #startWrite()}. This will close the current - * write stream, and roll back to the previous state of the file. + * write stream, and delete the new file. */ public void failWrite(FileOutputStream str) { - if (str != null) { - FileUtils.sync(str); - try { - str.close(); - mBaseName.delete(); - mBackupName.renameTo(mBaseName); - } catch (IOException e) { - Log.w("AtomicFile", "failWrite: Got exception:", e); - } + if (str == null) { + return; + } + if (!FileUtils.sync(str)) { + Log.e(LOG_TAG, "Failed to sync file output stream"); + } + try { + str.close(); + } catch (IOException e) { + Log.e(LOG_TAG, "Failed to close file output stream", e); + } + if (!mNewName.delete()) { + Log.e(LOG_TAG, "Failed to delete new file " + mNewName); } } @@ -210,32 +214,34 @@ public class AtomicFile { } /** - * Open the atomic file for reading. If there previously was an - * incomplete write, this will roll back to the last good data before - * opening for read. You should call close() on the FileInputStream when - * you are done reading from it. - * - *
Note that if another thread is currently performing - * a write, this will incorrectly consider it to be in the state of a bad - * write and roll back, causing the new data currently being written to - * be dropped. You must do your own threading protection for access to - * AtomicFile. + * Open the atomic file for reading. You should call close() on the FileInputStream when you are + * done reading from it. + *
+ * You must do your own threading protection for access to AtomicFile.
*/
public FileInputStream openRead() throws FileNotFoundException {
- if (mBackupName.exists()) {
- mBaseName.delete();
- mBackupName.renameTo(mBaseName);
+ if (mLegacyBackupName.exists()) {
+ if (!mLegacyBackupName.renameTo(mBaseName)) {
+ Log.e(LOG_TAG, "Failed to rename legacy backup file " + mLegacyBackupName
+ + " to base file " + mBaseName);
+ }
+ }
+
+ if (mNewName.exists()) {
+ if (!mNewName.delete()) {
+ Log.e(LOG_TAG, "Failed to delete outdated new file " + mNewName);
+ }
}
return new FileInputStream(mBaseName);
}
/**
* @hide
- * Checks if the original or backup file exists.
- * @return whether the original or backup file exists.
+ * Checks if the original or legacy backup file exists.
+ * @return whether the original or legacy backup file exists.
*/
public boolean exists() {
- return mBaseName.exists() || mBackupName.exists();
+ return mBaseName.exists() || mLegacyBackupName.exists();
}
/**
@@ -246,8 +252,8 @@ public class AtomicFile {
* the file does not exist or an I/O error is encountered.
*/
public long getLastModifiedTime() {
- if (mBackupName.exists()) {
- return mBackupName.lastModified();
+ if (mLegacyBackupName.exists()) {
+ return mLegacyBackupName.lastModified();
}
return mBaseName.lastModified();
}
diff --git a/core/tests/utiltests/src/android/util/AtomicFileTest.java b/core/tests/utiltests/src/android/util/AtomicFileTest.java
new file mode 100644
index 0000000000000..547d4d844181a
--- /dev/null
+++ b/core/tests/utiltests/src/android/util/AtomicFileTest.java
@@ -0,0 +1,245 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.util;
+
+import static org.junit.Assert.assertArrayEquals;
+
+import android.app.Instrumentation;
+import android.content.Context;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+import androidx.test.platform.app.InstrumentationRegistry;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+
+@RunWith(Parameterized.class)
+public class AtomicFileTest {
+ private static final String BASE_NAME = "base";
+ private static final String NEW_NAME = BASE_NAME + ".new";
+ private static final String LEGACY_BACKUP_NAME = BASE_NAME + ".bak";
+
+ private enum WriteAction {
+ FINISH,
+ FAIL,
+ ABORT
+ }
+
+ private static final byte[] BASE_BYTES = "base".getBytes(StandardCharsets.UTF_8);
+ private static final byte[] EXISTING_NEW_BYTES = "unnew".getBytes(StandardCharsets.UTF_8);
+ private static final byte[] NEW_BYTES = "new".getBytes(StandardCharsets.UTF_8);
+ private static final byte[] LEGACY_BACKUP_BYTES = "bak".getBytes(StandardCharsets.UTF_8);
+
+ // JUnit wants every parameter to be used so make it happy.
+ @Parameterized.Parameter()
+ public String mUnusedTestName;
+ @Nullable
+ @Parameterized.Parameter(1)
+ public String[] mExistingFileNames;
+ @Nullable
+ @Parameterized.Parameter(2)
+ public WriteAction mWriteAction;
+ @Nullable
+ @Parameterized.Parameter(3)
+ public byte[] mExpectedBytes;
+
+ private final Instrumentation mInstrumentation =
+ InstrumentationRegistry.getInstrumentation();
+ private final Context mContext = mInstrumentation.getContext();
+
+ private final File mDirectory = mContext.getFilesDir();
+ private final File mBaseFile = new File(mDirectory, BASE_NAME);
+ private final File mNewFile = new File(mDirectory, NEW_NAME);
+ private final File mLegacyBackupFile = new File(mDirectory, LEGACY_BACKUP_NAME);
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Object[][] data() {
+ return new Object[][] {
+ { "none + none = none", null, null, null },
+ { "none + finish = new", null, WriteAction.FINISH, NEW_BYTES },
+ { "none + fail = none", null, WriteAction.FAIL, null },
+ { "none + abort = none", null, WriteAction.ABORT, null },
+ { "base + none = base", new String[] { BASE_NAME }, null, BASE_BYTES },
+ { "base + finish = new", new String[] { BASE_NAME }, WriteAction.FINISH,
+ NEW_BYTES },
+ { "base + fail = base", new String[] { BASE_NAME }, WriteAction.FAIL, BASE_BYTES },
+ { "base + abort = base", new String[] { BASE_NAME }, WriteAction.ABORT,
+ BASE_BYTES },
+ { "new + none = none", new String[] { NEW_NAME }, null, null },
+ { "new + finish = new", new String[] { NEW_NAME }, WriteAction.FINISH, NEW_BYTES },
+ { "new + fail = none", new String[] { NEW_NAME }, WriteAction.FAIL, null },
+ { "new + abort = none", new String[] { NEW_NAME }, WriteAction.ABORT, null },
+ { "bak + none = bak", new String[] { LEGACY_BACKUP_NAME }, null,
+ LEGACY_BACKUP_BYTES },
+ { "bak + finish = new", new String[] { LEGACY_BACKUP_NAME }, WriteAction.FINISH,
+ NEW_BYTES },
+ { "bak + fail = bak", new String[] { LEGACY_BACKUP_NAME }, WriteAction.FAIL,
+ LEGACY_BACKUP_BYTES },
+ { "bak + abort = bak", new String[] { LEGACY_BACKUP_NAME }, WriteAction.ABORT,
+ LEGACY_BACKUP_BYTES },
+ { "base & new + none = base", new String[] { BASE_NAME, NEW_NAME }, null,
+ BASE_BYTES },
+ { "base & new + finish = new", new String[] { BASE_NAME, NEW_NAME },
+ WriteAction.FINISH, NEW_BYTES },
+ { "base & new + fail = base", new String[] { BASE_NAME, NEW_NAME },
+ WriteAction.FAIL, BASE_BYTES },
+ { "base & new + abort = base", new String[] { BASE_NAME, NEW_NAME },
+ WriteAction.ABORT, BASE_BYTES },
+ { "base & bak + none = bak", new String[] { BASE_NAME, LEGACY_BACKUP_NAME }, null,
+ LEGACY_BACKUP_BYTES },
+ { "base & bak + finish = new", new String[] { BASE_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.FINISH, NEW_BYTES },
+ { "base & bak + fail = bak", new String[] { BASE_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.FAIL, LEGACY_BACKUP_BYTES },
+ { "base & bak + abort = bak", new String[] { BASE_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.ABORT, LEGACY_BACKUP_BYTES },
+ { "new & bak + none = bak", new String[] { NEW_NAME, LEGACY_BACKUP_NAME }, null,
+ LEGACY_BACKUP_BYTES },
+ { "new & bak + finish = new", new String[] { NEW_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.FINISH, NEW_BYTES },
+ { "new & bak + fail = bak", new String[] { NEW_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.FAIL, LEGACY_BACKUP_BYTES },
+ { "new & bak + abort = bak", new String[] { NEW_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.ABORT, LEGACY_BACKUP_BYTES },
+ { "base & new & bak + none = bak",
+ new String[] { BASE_NAME, NEW_NAME, LEGACY_BACKUP_NAME }, null,
+ LEGACY_BACKUP_BYTES },
+ { "base & new & bak + finish = new",
+ new String[] { BASE_NAME, NEW_NAME, LEGACY_BACKUP_NAME },
+ WriteAction.FINISH, NEW_BYTES },
+ { "base & new & bak + fail = bak",
+ new String[] { BASE_NAME, NEW_NAME, LEGACY_BACKUP_NAME }, WriteAction.FAIL,
+ LEGACY_BACKUP_BYTES },
+ { "base & new & bak + abort = bak",
+ new String[] { BASE_NAME, NEW_NAME, LEGACY_BACKUP_NAME }, WriteAction.ABORT,
+ LEGACY_BACKUP_BYTES },
+ };
+ }
+
+ @Before
+ @After
+ public void deleteFiles() {
+ mBaseFile.delete();
+ mNewFile.delete();
+ mLegacyBackupFile.delete();
+ }
+
+ @Test
+ public void testAtomicFile() throws Exception {
+ if (mExistingFileNames != null) {
+ for (String fileName : mExistingFileNames) {
+ switch (fileName) {
+ case BASE_NAME:
+ writeBytes(mBaseFile, BASE_BYTES);
+ break;
+ case NEW_NAME:
+ writeBytes(mNewFile, EXISTING_NEW_BYTES);
+ break;
+ case LEGACY_BACKUP_NAME:
+ writeBytes(mLegacyBackupFile, LEGACY_BACKUP_BYTES);
+ break;
+ default:
+ throw new AssertionError(fileName);
+ }
+ }
+ }
+
+ AtomicFile atomicFile = new AtomicFile(mBaseFile);
+ if (mWriteAction != null) {
+ try (FileOutputStream outputStream = atomicFile.startWrite()) {
+ outputStream.write(NEW_BYTES);
+ switch (mWriteAction) {
+ case FINISH:
+ atomicFile.finishWrite(outputStream);
+ break;
+ case FAIL:
+ atomicFile.failWrite(outputStream);
+ break;
+ case ABORT:
+ // Neither finishing nor failing is called upon abort.
+ break;
+ default:
+ throw new AssertionError(mWriteAction);
+ }
+ }
+ }
+
+ if (mExpectedBytes != null) {
+ try (FileInputStream inputStream = atomicFile.openRead()) {
+ assertArrayEquals(mExpectedBytes, readAllBytes(inputStream));
+ }
+ } else {
+ assertThrows(FileNotFoundException.class, () -> atomicFile.openRead());
+ }
+ }
+
+ private static void writeBytes(@NonNull File file, @NonNull byte[] bytes) throws IOException {
+ try (FileOutputStream outputStream = new FileOutputStream(file)) {
+ outputStream.write(bytes);
+ }
+ }
+
+ // InputStream.readAllBytes() is introduced in Java 9. Our files are small enough so that a
+ // naive implementation is okay.
+ private static byte[] readAllBytes(@NonNull InputStream inputStream) throws IOException {
+ try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
+ int b;
+ while ((b = inputStream.read()) != -1) {
+ outputStream.write(b);
+ }
+ return outputStream.toByteArray();
+ }
+ }
+
+ @NonNull
+ public static