diff --git a/core/java/com/android/internal/os/AtomicDirectory.java b/core/java/com/android/internal/os/AtomicDirectory.java index ebcee15587c82..8ca7a7186cb48 100644 --- a/core/java/com/android/internal/os/AtomicDirectory.java +++ b/core/java/com/android/internal/os/AtomicDirectory.java @@ -17,15 +17,17 @@ package com.android.internal.os; import android.annotation.NonNull; -import android.annotation.Nullable; import android.os.FileUtils; +import android.system.ErrnoException; +import android.system.Os; +import android.system.OsConstants; import android.util.ArrayMap; +import android.util.Log; import com.android.internal.util.Preconditions; -import libcore.io.IoUtils; - import java.io.File; +import java.io.FileDescriptor; import java.io.FileOutputStream; import java.io.IOException; import java.util.Arrays; @@ -48,12 +50,13 @@ import java.util.Arrays; * backing directory when checking existence, making changes, and deleting. */ public final class AtomicDirectory { - private final @NonNull ArrayMap mOpenFiles = new ArrayMap<>(); + + private static final String LOG_TAG = AtomicDirectory.class.getSimpleName(); + private final @NonNull File mBaseDirectory; private final @NonNull File mBackupDirectory; - private int mBaseDirectoryFd = -1; - private int mBackupDirectoryFd = -1; + private final @NonNull ArrayMap mOpenFiles = new ArrayMap<>(); /** * Creates a new instance. @@ -67,15 +70,16 @@ public final class AtomicDirectory { } /** - * Gets the backup directory if present. This could be useful if you are - * writing new state to the dir but need to access the last persisted state - * at the same time. This means that this call is useful in between - * {@link #startWrite()} and {@link #finishWrite()} or {@link #failWrite()}. - * You should not modify the content returned by this method. + * Gets the backup directory which may or may not exist. This could be + * useful if you are writing new state to the directory but need to access + * the last persisted state at the same time. This means that this call is + * useful in between {@link #startWrite()} and {@link #finishWrite()} or + * {@link #failWrite()}. You should not modify the content returned by this + * method. * * @see #startRead() */ - public @Nullable File getBackupDirectory() { + public @NonNull File getBackupDirectory() { return mBackupDirectory; } @@ -90,7 +94,8 @@ public final class AtomicDirectory { */ public @NonNull File startRead() throws IOException { restore(); - return getOrCreateBaseDirectory(); + ensureBaseDirectory(); + return mBaseDirectory; } /** @@ -99,10 +104,7 @@ public final class AtomicDirectory { * @see #startRead() * @see #startWrite() */ - public void finishRead() { - mBaseDirectoryFd = -1; - mBackupDirectoryFd = -1; - } + public void finishRead() {} /** * Starts editing this directory. After calling this method you should @@ -121,7 +123,8 @@ public final class AtomicDirectory { */ public @NonNull File startWrite() throws IOException { backup(); - return getOrCreateBaseDirectory(); + ensureBaseDirectory(); + return mBaseDirectory; } /** @@ -135,11 +138,11 @@ public final class AtomicDirectory { * @see #closeWrite(FileOutputStream) */ public @NonNull FileOutputStream openWrite(@NonNull File file) throws IOException { - if (file.isDirectory() || !file.getParentFile().equals(getOrCreateBaseDirectory())) { - throw new IllegalArgumentException("Must be a file in " + getOrCreateBaseDirectory()); + if (file.isDirectory() || !file.getParentFile().equals(mBaseDirectory)) { + throw new IllegalArgumentException("Must be a file in " + mBaseDirectory); } if (mOpenFiles.containsKey(file)) { - throw new IllegalArgumentException("Already open file" + file.getCanonicalPath()); + throw new IllegalArgumentException("Already open file " + file.getAbsolutePath()); } final FileOutputStream destination = new FileOutputStream(file); mOpenFiles.put(file, destination); @@ -160,7 +163,7 @@ public final class AtomicDirectory { } mOpenFiles.removeAt(indexOfValue); FileUtils.sync(destination); - IoUtils.closeQuietly(destination); + FileUtils.closeQuietly(destination); } public void failWrite(@NonNull FileOutputStream destination) { @@ -169,7 +172,7 @@ public final class AtomicDirectory { throw new IllegalArgumentException("Unknown file stream " + destination); } mOpenFiles.removeAt(indexOfValue); - IoUtils.closeQuietly(destination); + FileUtils.closeQuietly(destination); } /** @@ -177,15 +180,15 @@ public final class AtomicDirectory { * * @see #startWrite() * - * @throws IllegalStateException is some files are not closed. + * @throws IllegalStateException if some files are not closed. */ public void finishWrite() { throwIfSomeFilesOpen(); - fsyncDirectoryFd(mBaseDirectoryFd); + + syncDirectory(mBaseDirectory); + syncParentDirectory(); deleteDirectory(mBackupDirectory); - fsyncDirectoryFd(mBackupDirectoryFd); - mBaseDirectoryFd = -1; - mBackupDirectoryFd = -1; + syncParentDirectory(); } /** @@ -195,11 +198,12 @@ public final class AtomicDirectory { */ public void failWrite() { throwIfSomeFilesOpen(); + try{ restore(); - } catch (IOException ignored) {} - mBaseDirectoryFd = -1; - mBackupDirectoryFd = -1; + } catch (IOException e) { + Log.e(LOG_TAG, "Failed to restore in failWrite()", e); + } } /** @@ -213,29 +217,28 @@ public final class AtomicDirectory { * Deletes this directory. */ public void delete() { + boolean deleted = false; if (mBaseDirectory.exists()) { - deleteDirectory(mBaseDirectory); - fsyncDirectoryFd(mBaseDirectoryFd); + deleted |= deleteDirectory(mBaseDirectory); } if (mBackupDirectory.exists()) { - deleteDirectory(mBackupDirectory); - fsyncDirectoryFd(mBackupDirectoryFd); + deleted |= deleteDirectory(mBackupDirectory); + } + if (deleted) { + syncParentDirectory(); } } - private @NonNull File getOrCreateBaseDirectory() throws IOException { - if (!mBaseDirectory.exists()) { - if (!mBaseDirectory.mkdirs()) { - throw new IOException("Couldn't create directory " + mBaseDirectory); - } - FileUtils.setPermissions(mBaseDirectory.getPath(), - FileUtils.S_IRWXU | FileUtils.S_IRWXG | FileUtils.S_IXOTH, - -1, -1); + private void ensureBaseDirectory() throws IOException { + if (mBaseDirectory.exists()) { + return; } - if (mBaseDirectoryFd < 0) { - mBaseDirectoryFd = getDirectoryFd(mBaseDirectory.getCanonicalPath()); + + if (!mBaseDirectory.mkdirs()) { + throw new IOException("Failed to create directory " + mBaseDirectory); } - return mBaseDirectory; + FileUtils.setPermissions(mBaseDirectory.getPath(), + FileUtils.S_IRWXU | FileUtils.S_IRWXG | FileUtils.S_IXOTH, -1, -1); } private void throwIfSomeFilesOpen() { @@ -249,50 +252,56 @@ public final class AtomicDirectory { if (!mBaseDirectory.exists()) { return; } - if (mBaseDirectoryFd < 0) { - mBaseDirectoryFd = getDirectoryFd(mBaseDirectory.getCanonicalPath()); - } + if (mBackupDirectory.exists()) { deleteDirectory(mBackupDirectory); } if (!mBaseDirectory.renameTo(mBackupDirectory)) { - throw new IOException("Couldn't backup " + mBaseDirectory - + " to " + mBackupDirectory); + throw new IOException("Failed to backup " + mBaseDirectory + " to " + mBackupDirectory); } - mBackupDirectoryFd = mBaseDirectoryFd; - mBaseDirectoryFd = -1; - fsyncDirectoryFd(mBackupDirectoryFd); + syncParentDirectory(); } private void restore() throws IOException { if (!mBackupDirectory.exists()) { return; } - if (mBackupDirectoryFd == -1) { - mBackupDirectoryFd = getDirectoryFd(mBackupDirectory.getCanonicalPath()); - } + if (mBaseDirectory.exists()) { deleteDirectory(mBaseDirectory); } if (!mBackupDirectory.renameTo(mBaseDirectory)) { - throw new IOException("Couldn't restore " + mBackupDirectory - + " to " + mBaseDirectory); + throw new IOException("Failed to restore " + mBackupDirectory + " to " + + mBaseDirectory); } - mBaseDirectoryFd = mBackupDirectoryFd; - mBackupDirectoryFd = -1; - fsyncDirectoryFd(mBaseDirectoryFd); + syncParentDirectory(); } - private static void deleteDirectory(@NonNull File file) { - final File[] children = file.listFiles(); - if (children != null) { - for (File child : children) { - deleteDirectory(child); - } - } - file.delete(); + private static boolean deleteDirectory(@NonNull File directory) { + return FileUtils.deleteContentsAndDir(directory); } - private static native int getDirectoryFd(String path); - private static native void fsyncDirectoryFd(int fd); + private void syncParentDirectory() { + syncDirectory(mBaseDirectory.getParentFile()); + } + + // Standard Java IO doesn't allow opening a directory (will throw a FileNotFoundException + // instead), so we have to do it manually. + private static void syncDirectory(@NonNull File directory) { + String path = directory.getAbsolutePath(); + FileDescriptor fd; + try { + fd = Os.open(path, OsConstants.O_RDONLY, 0); + } catch (ErrnoException e) { + Log.e(LOG_TAG, "Failed to open " + path, e); + return; + } + try { + Os.fsync(fd); + } catch (ErrnoException e) { + Log.e(LOG_TAG, "Failed to fsync " + path, e); + } finally { + FileUtils.closeQuietly(fd); + } + } } diff --git a/core/jni/Android.bp b/core/jni/Android.bp index 844a89862d5a1..9d99f58d5ddb1 100644 --- a/core/jni/Android.bp +++ b/core/jni/Android.bp @@ -194,7 +194,6 @@ cc_library_shared { "android_content_res_ObbScanner.cpp", "android_content_res_Configuration.cpp", "android_security_Scrypt.cpp", - "com_android_internal_os_AtomicDirectory.cpp", "com_android_internal_os_ClassLoaderFactory.cpp", "com_android_internal_os_FuseAppLoop.cpp", "com_android_internal_os_Zygote.cpp", diff --git a/core/jni/AndroidRuntime.cpp b/core/jni/AndroidRuntime.cpp index 533f403034a92..a3fd915c29b2b 100644 --- a/core/jni/AndroidRuntime.cpp +++ b/core/jni/AndroidRuntime.cpp @@ -227,7 +227,6 @@ extern int register_android_content_res_Configuration(JNIEnv* env); extern int register_android_animation_PropertyValuesHolder(JNIEnv *env); extern int register_android_security_Scrypt(JNIEnv *env); extern int register_com_android_internal_content_NativeLibraryHelper(JNIEnv *env); -extern int register_com_android_internal_os_AtomicDirectory(JNIEnv *env); extern int register_com_android_internal_os_ClassLoaderFactory(JNIEnv* env); extern int register_com_android_internal_os_FuseAppLoop(JNIEnv* env); extern int register_com_android_internal_os_Zygote(JNIEnv *env); @@ -1602,7 +1601,6 @@ static const RegJNIRec gRegJNI[] = { REG_JNI(register_android_animation_PropertyValuesHolder), REG_JNI(register_android_security_Scrypt), REG_JNI(register_com_android_internal_content_NativeLibraryHelper), - REG_JNI(register_com_android_internal_os_AtomicDirectory), REG_JNI(register_com_android_internal_os_FuseAppLoop), }; diff --git a/core/jni/com_android_internal_os_AtomicDirectory.cpp b/core/jni/com_android_internal_os_AtomicDirectory.cpp deleted file mode 100644 index 76b0fc1672641..0000000000000 --- a/core/jni/com_android_internal_os_AtomicDirectory.cpp +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright (C) 2018 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. - */ - -#include -#include "jni.h" - -#include "core_jni_helpers.h" - -namespace android { - -static jint com_android_internal_os_AtomicDirectory_getDirectoryFd(JNIEnv* env, - jobject /*clazz*/, jstring path) { - ScopedUtfChars path8(env, path); - if (path8.c_str() == NULL) { - ALOGE("Invalid path: %s", path8.c_str()); - return -1; - } - int fd; - if ((fd = TEMP_FAILURE_RETRY(open(path8.c_str(), O_DIRECTORY | O_RDONLY | O_CLOEXEC))) == -1) { - ALOGE("Cannot open directory %s, error: %s\n", path8.c_str(), strerror(errno)); - return -1; - } - return fd; -} - -static void com_android_internal_os_AtomicDirectory_fsyncDirectoryFd(JNIEnv* env, - jobject /*clazz*/, jint fd) { - if (TEMP_FAILURE_RETRY(fsync(fd)) == -1) { - ALOGE("Cannot fsync directory %d, error: %s\n", fd, strerror(errno)); - } -} - -/* - * JNI registration. - */ -static const JNINativeMethod gRegisterMethods[] = { - /* name, signature, funcPtr */ - { "fsyncDirectoryFd", - "(I)V", - (void*) com_android_internal_os_AtomicDirectory_fsyncDirectoryFd - }, - { "getDirectoryFd", - "(Ljava/lang/String;)I", - (void*) com_android_internal_os_AtomicDirectory_getDirectoryFd - }, -}; - -int register_com_android_internal_os_AtomicDirectory(JNIEnv* env) { - return RegisterMethodsOrDie(env, "com/android/internal/os/AtomicDirectory", - gRegisterMethods, NELEM(gRegisterMethods)); -} - -}; // namespace android