From 04812b614baebeabbe8e556669b78ada609c7fce Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Sun, 4 Oct 2026 19:49:00 -0700 Subject: [PATCH 1/6] [CoreAI] Add atomic cache storage primitives Summary: Add the filesystem layer the Core AI runtime uses for its asset and bookmark cache (`runtime/coreai_storage.{h,mm}`, `runtime/coreai_file.h`). - Prepare an absolute storage root chosen by the application. Creating a root also excludes it from backup; inspecting an existing root changes nothing. - Enumerate directories through descriptors and return sorted names, failing on operational errors rather than returning partial listings. - Write new files relative to a directory descriptor and `fsync` each file and its parent directory. - Publish data atomically: write a temporary file, `fsync` and `F_FULLFSYNC` it, `renameat` it into place and sync the directory. Readers see either the old or the new complete bytes. A failure before the rename leaves the old bytes and removes the temporary file. - Tree removal is not included here; it lands with cache clearing. - README: the "Asset storage" root, durability and backup-exclusion text. Trust model: the application owns the cache directory. The helpers do not defend against other processes of the same user renaming or replacing paths inside it. Fault injection points use `storage_fault()`, which compiles to a constant 0 unless `COREAI_ASSETS_TESTING` is defined. Only the `coreai_host_test` target defines it. No changes outside `backends/apple/coreai`. Test Plan: ``` cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_OSX_DEPLOYMENT_TARGET=27.0 -DEXECUTORCH_BUILD_COREAI=ON \ -DEXECUTORCH_BUILD_TESTS=ON -DEXECUTORCH_BUILD_EXTENSION_DATA_LOADER=ON cmake --build build --target backends/apple/coreai/all ctest --test-dir build/backends/apple/coreai --output-on-failure --no-tests=error ``` CTest runs `coreai_host_test`, which adds the storage suite. AtomicPublicationPreservesOldBytesOnFailure checks that a failed rename leaves the old bytes and no `.tmp-*` entry. --- backends/apple/coreai/CMakeLists.txt | 5 +- backends/apple/coreai/README.md | 13 + backends/apple/coreai/runtime/coreai_file.h | 53 ++++ .../apple/coreai/runtime/coreai_storage.h | 60 ++++ .../apple/coreai/runtime/coreai_storage.mm | 298 ++++++++++++++++++ .../coreai/runtime/test/coreai_fault_scope.h | 57 ++++ .../runtime/test/coreai_filesystem_fixture.h | 27 ++ .../runtime/test/coreai_filesystem_fixture.mm | 61 ++++ .../runtime/test/coreai_storage_test.mm | 113 +++++++ 9 files changed, 685 insertions(+), 2 deletions(-) create mode 100644 backends/apple/coreai/runtime/coreai_file.h create mode 100644 backends/apple/coreai/runtime/coreai_storage.h create mode 100644 backends/apple/coreai/runtime/coreai_storage.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_fault_scope.h create mode 100644 backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h create mode 100644 backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_storage_test.mm diff --git a/backends/apple/coreai/CMakeLists.txt b/backends/apple/coreai/CMakeLists.txt index 800a9f098a4..6a01341c31b 100644 --- a/backends/apple/coreai/CMakeLists.txt +++ b/backends/apple/coreai/CMakeLists.txt @@ -11,7 +11,7 @@ endif() enable_language(OBJCXX) set(_coreai_runtime_sources runtime/coreai_manifest.mm - runtime/coreai_validation.mm + runtime/coreai_validation.mm runtime/coreai_storage.mm ) function(coreai_configure_objc_target target) @@ -31,7 +31,8 @@ if(EXECUTORCH_BUILD_TESTS AND CMAKE_SYSTEM_NAME STREQUAL "Darwin") add_executable( coreai_host_test runtime/test/coreai_host_test.mm runtime/test/coreai_manifest_test.mm - ${_coreai_runtime_sources} + runtime/test/coreai_storage_test.mm + runtime/test/coreai_filesystem_fixture.mm ${_coreai_runtime_sources} ) coreai_configure_objc_target(coreai_host_test) find_library(COREAI_FOUNDATION_FRAMEWORK Foundation REQUIRED) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 4b5115e1a21..623a96a14a6 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -50,6 +50,19 @@ architectures=["h17p"])` requests that compiler architecture; omitting the list lets the compiler emit its supported architectures for the target platform. These are Core AI architecture names, not CPU names such as `arm64`. +### Asset storage + +The assets root must be an absolute path. The application chooses it, should +reserve it for this backend, and is trusted not to rename, replace or modify its +contents while the backend uses it; the backend does not defend against other +processes of the same user rebinding paths. Written files and changed directories +are synced with `fsync`; atomic publication also uses `F_FULLFSYNC` before its +rename. + +Preparing the assets root sets `NSURLIsExcludedFromBackupKey` on it, which covers +everything beneath it. Ancestors are not modified. This is backup exclusion, not +a control for iCloud Drive synchronization. + ## Host Tests On macOS, `EXECUTORCH_BUILD_COREAI=ON` with `EXECUTORCH_BUILD_TESTS=ON` diff --git a/backends/apple/coreai/runtime/coreai_file.h b/backends/apple/coreai/runtime/coreai_file.h new file mode 100644 index 00000000000..478f03a7b91 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_file.h @@ -0,0 +1,53 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace executorch::backends::coreai { + +class FileDescriptor { + public: + explicit FileDescriptor(int fd) : fd_(fd) {} + ~FileDescriptor() { + reset(-1); + } + FileDescriptor(const FileDescriptor&) = delete; + FileDescriptor& operator=(const FileDescriptor&) = delete; + int get() const { + return fd_; + } + int release() { + const int fd = fd_; + fd_ = -1; + return fd; + } + void reset(int fd) { + if (fd_ >= 0) { + // Retrying close can close a descriptor reused by another thread. + ::close(fd_); + } + fd_ = fd; + } + + private: + int fd_; +}; + +template +auto retry_eintr(F call) { + decltype(call()) result; + do { + result = call(); + } while (result == -1 && errno == EINTR); + return result; +} + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_storage.h b/backends/apple/coreai/runtime/coreai_storage.h new file mode 100644 index 00000000000..f5bd377dd70 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_storage.h @@ -0,0 +1,60 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#import +#include +#include + +namespace executorch::backends::coreai { + +// Resolves the default location without creating or modifying storage. +runtime::Result default_coreai_assets_root(); +// Requires an absolute directory path. With create, makes the directory and +// excludes it from backup; otherwise a missing directory is InvalidArgument. +runtime::Result prepare_storage_root(NSString* path, bool create); +runtime::Error ensure_excluded_from_backup(NSURL* directory); + +runtime::Error sync_storage_directory(int fd); +// Returns an independently owned descriptor for a stable coordination file. +int open_storage_shared_file(int parent_fd, const char* name); +// Creates a new file and missing parents, then syncs the file and its parent. +runtime::Error write_storage_file( + int root_fd, + NSString* relative_path, + const void* bytes, + size_t size); +// Atomically replaces name with data. On error, readers see either the old or +// the new complete bytes. +runtime::Error publish_storage_data(int root_fd, NSString* name, NSData* data); +runtime::Result*> storage_children( + int fd, + bool skip_invalid_names = false); + +enum class StorageOperation { + Rename, + BeforeSDK, + AfterSDK, + BeforeEvict, + AfterEvict, + Remove, +}; +#if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING +int storage_fault(StorageOperation operation); +namespace testing { +using StorageFaultCallback = int (*)(StorageOperation); +extern thread_local StorageFaultCallback storage_fault_callback; +} // namespace testing +#else +constexpr int storage_fault(StorageOperation) { + return 0; +} +#endif + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_storage.mm b/backends/apple/coreai/runtime/coreai_storage.mm new file mode 100644 index 00000000000..66fea0e8833 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_storage.mm @@ -0,0 +1,298 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "coreai_storage.h" +#include "coreai_file.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace executorch::backends::coreai { +namespace { +using runtime::Error; +using runtime::Result; +} // namespace + +Error ensure_excluded_from_backup(NSURL* directory) { + NSURL* fresh = [NSURL fileURLWithPath:directory.path isDirectory:YES]; + NSNumber* excluded = nil; + NSError* error = nil; + ET_CHECK_OR_RETURN_ERROR( + [fresh getResourceValue:&excluded + forKey:NSURLIsExcludedFromBackupKey + error:&error], + AccessFailed, + "Cannot read Core AI backup exclusion: %s", + error.localizedDescription.UTF8String); + if (excluded.boolValue) { + return Error::Ok; + } + ET_CHECK_OR_RETURN_ERROR( + [fresh setResourceValue:@YES + forKey:NSURLIsExcludedFromBackupKey + error:&error], + AccessFailed, + "Cannot exclude Core AI storage from backup: %s", + error.localizedDescription.UTF8String); + return Error::Ok; +} + +Result default_coreai_assets_root() { + NSURL* caches = [NSFileManager.defaultManager + URLsForDirectory:NSCachesDirectory + inDomains:NSUserDomainMask].firstObject; + ET_CHECK_OR_RETURN_ERROR(caches.isFileURL, AccessFailed, + "Cannot resolve Core AI cache directory"); + return [caches.path stringByAppendingPathComponent:@"executorch_coreai"]; +} + +Result prepare_storage_root(NSString* path, bool create) { + ET_CHECK_OR_RETURN_ERROR( + [path isKindOfClass:NSString.class] && path.isAbsolutePath && + [path rangeOfString:@"\0"].location == NSNotFound, + InvalidArgument, + "Core AI storage requires an absolute path"); + if (create) { + NSError* error = nil; + ET_CHECK_OR_RETURN_ERROR( + [NSFileManager.defaultManager + createDirectoryAtPath:path + withIntermediateDirectories:YES + attributes:@{ + NSFilePosixPermissions : @0700 + } + error:&error], + AccessFailed, + "Cannot create Core AI storage directory: %s", + error.localizedDescription.UTF8String); + } + struct stat info; + const int result = + retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); + ET_CHECK_OR_RETURN_ERROR( + result == 0 || errno == ENOENT || errno == ENOTDIR, + AccessFailed, + "Cannot inspect Core AI storage directory"); + ET_CHECK_OR_RETURN_ERROR( + result == 0 && S_ISDIR(info.st_mode), + InvalidArgument, + "Core AI storage must be an existing directory"); + if (create) { + ET_CHECK_OK_OR_RETURN_ERROR(ensure_excluded_from_backup( + [NSURL fileURLWithPath:path isDirectory:YES])); + } + return path; +} + +#if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING +namespace testing { +thread_local StorageFaultCallback storage_fault_callback = nullptr; +} + +int storage_fault(StorageOperation operation) { + return testing::storage_fault_callback == nullptr + ? 0 + : testing::storage_fault_callback(operation); +} +#endif + +namespace { +template +int storage_call(StorageOperation operation, F call) { + return retry_eintr([&] { + const int fault = storage_fault(operation); + if (fault != 0) { + errno = fault; + return -1; + } + return call(); + }); +} + +bool storage_component(NSString* name) { + return [name isKindOfClass:NSString.class] && name.length > 0 && + ![name isEqualToString:@"."] && ![name isEqualToString:@".."] && + ![name containsString:@"/"] && ![name containsString:@"\0"]; +} + +Error write_file( + int root_fd, + NSString* relative_path, + const void* data, + size_t size, + bool full_sync) { + NSArray* components = + [relative_path componentsSeparatedByString:@"/"]; + ET_CHECK_OR_RETURN_ERROR( + components.count > 0 && (data != nullptr || size == 0), + InvalidArgument, + "Invalid Core AI storage write"); + for (NSString* component in components) { + ET_CHECK_OR_RETURN_ERROR( + storage_component(component), InvalidArgument, "Invalid storage path"); + } + FileDescriptor parent(retry_eintr([&] { return dup(root_fd); })); + ET_CHECK_OR_RETURN_ERROR( + parent.get() >= 0, AccessFailed, "Cannot open storage parent"); + for (NSUInteger i = 0; i + 1 < components.count; ++i) { + const char* component = components[i].fileSystemRepresentation; + ET_CHECK_OR_RETURN_ERROR( + retry_eintr( + [&] { return mkdirat(parent.get(), component, 0700); }) == 0 || + errno == EEXIST, + AccessFailed, + "Cannot create Core AI source directory"); + FileDescriptor next(retry_eintr([&] { + return openat( + parent.get(), + component, + O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + })); + ET_CHECK_OR_RETURN_ERROR( + next.get() >= 0, AccessFailed, "Cannot open source directory"); + ET_CHECK_OK_OR_RETURN_ERROR(sync_storage_directory(parent.get())); + parent.reset(next.release()); + } + FileDescriptor file(retry_eintr([&] { + return openat( + parent.get(), + components.lastObject.fileSystemRepresentation, + O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, + 0600); + })); + ET_CHECK_OR_RETURN_ERROR( + file.get() >= 0, AccessFailed, "Cannot create Core AI storage file"); + const auto* bytes = static_cast(data); + while (size > 0) { + const ssize_t count = + write(file.get(), bytes, std::min(size, size_t(1024 * 1024))); + if (count < 0 && errno == EINTR) continue; + ET_CHECK_OR_RETURN_ERROR( + count > 0, AccessFailed, "Cannot write Core AI storage file"); + bytes += count; + size -= count; + } + ET_CHECK_OR_RETURN_ERROR( + retry_eintr([&] { return fsync(file.get()); }) == 0 && + (!full_sync || + retry_eintr([&] { return fcntl(file.get(), F_FULLFSYNC); }) == 0), + AccessFailed, + "Cannot synchronize Core AI file"); + ET_CHECK_OR_RETURN_ERROR(::close(file.release()) == 0, AccessFailed, + "Cannot close Core AI storage file"); + return sync_storage_directory(parent.get()); +} +} // namespace + +Result*> storage_children(int fd, bool skip_invalid_names) { + FileDescriptor copy( + retry_eintr([&] { return fcntl(fd, F_DUPFD_CLOEXEC, 0); })); + ET_CHECK_OR_RETURN_ERROR(copy.get() >= 0, AccessFailed, + "Cannot duplicate storage directory"); + DIR* directory = fdopendir(copy.get()); + ET_CHECK_OR_RETURN_ERROR(directory != nullptr, AccessFailed, + "Cannot open storage directory stream"); + copy.release(); + // dup shares the enumeration offset with the original descriptor. + rewinddir(directory); + NSMutableArray* names = [NSMutableArray array]; + Error result = Error::Ok; + while (true) { + errno = 0; + dirent* entry = readdir(directory); + if (entry == nullptr) { + if (errno == EINTR) continue; + if (errno != 0) result = Error::AccessFailed; + break; + } + if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { + continue; + } + NSString* name = [[NSString alloc] initWithBytes:entry->d_name + length:strlen(entry->d_name) + encoding:NSUTF8StringEncoding]; + if (name == nil || + strcmp(name.fileSystemRepresentation, entry->d_name) != 0) { + if (skip_invalid_names) { + continue; + } + result = Error::InvalidExternalData; + break; + } + [names addObject:name]; + } + const int closed = closedir(directory); + ET_CHECK_OK_OR_RETURN_ERROR(result); + ET_CHECK_OR_RETURN_ERROR(closed == 0, AccessFailed, + "Cannot close storage directory stream"); + return [names sortedArrayUsingSelector:@selector(compare:)]; +} + +int open_storage_shared_file(int parent_fd, const char* name) { + // APFS can return ENOENT to a concurrent O_CREAT loser without O_EXCL. + int fd = retry_eintr([&] { + return openat(parent_fd, name, + O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, 0600); + }); + if (fd < 0 && errno == EEXIST) { + fd = retry_eintr([&] { + return openat(parent_fd, name, O_RDWR | O_NOFOLLOW | O_CLOEXEC); + }); + } + return fd; +} + +Error sync_storage_directory(int fd) { + ET_CHECK_OR_RETURN_ERROR( + retry_eintr([&] { return fsync(fd); }) == 0, + AccessFailed, + "Cannot synchronize Core AI directory"); + return Error::Ok; +} + +Error write_storage_file( + int root_fd, + NSString* relative_path, + const void* data, + size_t size) { + return write_file(root_fd, relative_path, data, size, false); +} + +Error publish_storage_data(int root_fd, NSString* name, NSData* data) { + ET_CHECK_OR_RETURN_ERROR( + storage_component(name) && data != nil, + InvalidArgument, + "Invalid Core AI storage publication"); + NSString* temporary = + [@".tmp-" stringByAppendingString:NSUUID.UUID.UUIDString]; + // Bookmarks are small and costly to lose, so they get a full device flush. + auto result = + write_file(root_fd, temporary, data.bytes, data.length, true); + if (result == Error::Ok && storage_call(StorageOperation::Rename, [&] { + return renameat( + root_fd, + temporary.fileSystemRepresentation, + root_fd, + name.fileSystemRepresentation); + }) != 0) + result = Error::AccessFailed; + if (result == Error::Ok) + return sync_storage_directory(root_fd); + if (unlinkat(root_fd, temporary.fileSystemRepresentation, 0) == 0) { + (void)sync_storage_directory(root_fd); + } + return result; +} + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/test/coreai_fault_scope.h b/backends/apple/coreai/runtime/test/coreai_fault_scope.h new file mode 100644 index 00000000000..e55de88579a --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_fault_scope.h @@ -0,0 +1,57 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include "coreai_storage.h" + +namespace executorch::backends::coreai::testing { + +struct StorageFaultScope { + StorageOperation operation; + int error = EIO; + int match_index = 1; + int matches = 0; + int hits = 0; + bool armed = true; + void (^observe)(StorageOperation) = nil; + + explicit StorageFaultScope(StorageOperation op) + : operation(op), + previous_(current_), + previous_callback_(storage_fault_callback) { + current_ = this; + storage_fault_callback = inject; + } + ~StorageFaultScope() { + storage_fault_callback = previous_callback_; + current_ = previous_; + } + StorageFaultScope(const StorageFaultScope&) = delete; + StorageFaultScope& operator=(const StorageFaultScope&) = delete; + + private: + inline static thread_local StorageFaultScope* current_ = nullptr; + StorageFaultScope* previous_; + StorageFaultCallback previous_callback_; + + static int inject(StorageOperation operation) { + auto& scope = *current_; + if (scope.observe != nil) + scope.observe(operation); + if (scope.armed && scope.operation == operation && + ++scope.matches == scope.match_index) { + ++scope.hits; + return scope.error; + } + return 0; + } +}; + +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h new file mode 100644 index 00000000000..625f1f48a8a --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h @@ -0,0 +1,27 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#import +#include + +namespace executorch::backends::coreai::testing { + +struct TestDirectory { + NSURL* url = nil; + TestDirectory(); + ~TestDirectory(); + TestDirectory(const TestDirectory&) = delete; + TestDirectory& operator=(const TestDirectory&) = delete; +}; + +::testing::AssertionResult backup_excluded(NSURL* url); +::testing::AssertionResult set_backup_excluded(NSURL* url, bool excluded); + +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm new file mode 100644 index 00000000000..f9810e02aad --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm @@ -0,0 +1,61 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "coreai_filesystem_fixture.h" +#include + +namespace executorch::backends::coreai::testing { + +TestDirectory::TestDirectory() { + char pattern[] = "/private/tmp/coreai-host-test-XXXXXX"; + if (mkdtemp(pattern) == nullptr) { + ADD_FAILURE() << "mkdtemp: " << strerror(errno); + return; + } + url = [NSURL fileURLWithPath:[NSString stringWithUTF8String:pattern] isDirectory:YES] + .URLByResolvingSymlinksInPath; +} + +TestDirectory::~TestDirectory() { + if (url == nil) return; + EXPECT_TRUE([url.lastPathComponent hasPrefix:@"coreai-host-test-"]); + if (![url.lastPathComponent hasPrefix:@"coreai-host-test-"]) return; + EXPECT_TRUE([NSFileManager.defaultManager removeItemAtURL:url error:nil]); +} + +::testing::AssertionResult backup_excluded(NSURL* url) { + NSURL* fresh = [NSURL fileURLWithPath:url.path]; + [fresh removeAllCachedResourceValues]; + NSNumber* value = nil; + NSError* error = nil; + if (![fresh getResourceValue:&value forKey:NSURLIsExcludedFromBackupKey error:&error] || + error != nil || value == nil) { + ADD_FAILURE() << "Cannot read backup flag: " << url.path.UTF8String; + return ::testing::AssertionFailure() << "Backup flag unavailable"; + } + return value.boolValue ? ::testing::AssertionSuccess() + : ::testing::AssertionFailure() << "Not excluded: " << url.path.UTF8String; +} + +::testing::AssertionResult set_backup_excluded(NSURL* url, bool excluded) { + // Foundation can defer metadata writes; settle deliberate fixture changes. + for (int attempt = 0; attempt < 50; ++attempt) { + NSURL* fresh = [NSURL fileURLWithPath:url.path]; + [fresh removeAllCachedResourceValues]; + if (![fresh setResourceValue:@(excluded) forKey:NSURLIsExcludedFromBackupKey error:nil]) { + return ::testing::AssertionFailure() << "Cannot set backup flag: " << url.path.UTF8String; + } + usleep(20000); + if (static_cast(backup_excluded(url)) == excluded) { + return ::testing::AssertionSuccess(); + } + } + return ::testing::AssertionFailure() << "Backup flag did not settle: " << url.path.UTF8String; +} + +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_storage_test.mm b/backends/apple/coreai/runtime/test/coreai_storage_test.mm new file mode 100644 index 00000000000..9b1ae57c5a1 --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_storage_test.mm @@ -0,0 +1,113 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include +#include "coreai_fault_scope.h" +#include "coreai_file.h" +#include "coreai_filesystem_fixture.h" + +using namespace executorch::runtime; +using namespace executorch::backends::coreai; +using namespace executorch::backends::coreai::testing; + +TEST(CoreAIStorageTest, PreparesRootWithRootOnlyBackupPolicy) { + NSFileManager* manager = NSFileManager.defaultManager; + TestDirectory sandbox; + ASSERT_NE(sandbox.url, nil); + NSURL* parent = [sandbox.url URLByAppendingPathComponent:@"parent"]; + ASSERT_TRUE(([manager createDirectoryAtURL:parent + withIntermediateDirectories:NO + attributes:nil + error:nil])); + ASSERT_TRUE(set_backup_excluded(sandbox.url, false)); + ASSERT_TRUE(set_backup_excluded(parent, false)); + NSURL* root = [parent URLByAppendingPathComponent:@"nested/models"]; + auto prepared = prepare_storage_root(root.path, true); + ASSERT_TRUE((prepared.ok() && [prepared.get() isEqualToString:root.path])); + ASSERT_TRUE((backup_excluded(root))); + ASSERT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); + ASSERT_TRUE((!backup_excluded(root.URLByDeletingLastPathComponent))); + + ASSERT_TRUE(set_backup_excluded(root, false)); + auto inspected = prepare_storage_root(root.path, false); + ASSERT_TRUE((inspected.ok() && [inspected.get() isEqualToString:root.path])); + ASSERT_TRUE((!backup_excluded(root))); + ASSERT_TRUE((prepare_storage_root(root.path, true).ok() && backup_excluded(root))); + + NSURL* missing = [sandbox.url URLByAppendingPathComponent:@"missing"]; + ASSERT_EQ(prepare_storage_root(missing.path, false).error(), Error::InvalidArgument); + EXPECT_FALSE([manager fileExistsAtPath:missing.path]); + EXPECT_EQ(prepare_storage_root(@"relative", true).error(), Error::InvalidArgument); + EXPECT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); +} + +TEST(CoreAIStorageTest, ListsChildrenRepeatedlyInSortedOrder) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root( + open(storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_TRUE((root.get() >= 0)); + NSMutableArray* expected = [NSMutableArray array]; + for (int i = 7; i >= 0; --i) { + NSString* name = [NSString stringWithFormat:@"entry-%d", i]; + FileDescriptor file(openat(root.get(), name.fileSystemRepresentation, + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); + ASSERT_TRUE((file.get() >= 0)); + [expected insertObject:name atIndex:0]; + } + for (int pass = 0; pass < 3; ++pass) { + errno = EIO; + auto names = storage_children(root.get()); + ASSERT_TRUE((names.ok() && [names.get() isEqual:expected])); + ASSERT_TRUE((fcntl(root.get(), F_GETFD) >= 0)); + } +} + +TEST(CoreAIStorageTest, RetriesEintr) { + int calls = 0; + const ssize_t count = retry_eintr([&]() -> ssize_t { + if (++calls < 3) { + errno = EINTR; + return -1; + } + return 7; + }); + ASSERT_TRUE((calls == 3 && count == 7)); +} + +static ::testing::AssertionResult no_temporary_files(int root) { + auto children = storage_children(root); + if (!children.ok()) return ::testing::AssertionFailure() << "Cannot list storage"; + for (NSString* child in children.get()) { + if ([child hasPrefix:@".tmp-"]) return ::testing::AssertionFailure() << child.UTF8String; + } + return ::testing::AssertionSuccess(); +} + +TEST(CoreAIStorageTest, AtomicPublicationPreservesOldBytesOnFailure) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root( + open(storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_TRUE((root.get() >= 0)); + NSData* old = [@"old" dataUsingEncoding:NSUTF8StringEncoding]; + NSData* next = [@"next" dataUsingEncoding:NSUTF8StringEncoding]; + NSURL* published = [storage.url URLByAppendingPathComponent:@"published"]; + ASSERT_EQ(publish_storage_data(root.get(), @"published", old), Error::Ok); + { + StorageFaultScope fault(StorageOperation::Rename); + EXPECT_EQ(publish_storage_data(root.get(), @"published", next), Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + } + EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:old])); + EXPECT_TRUE(no_temporary_files(root.get())); + + ASSERT_EQ(publish_storage_data(root.get(), @"published", next), Error::Ok); + EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:next])); + EXPECT_TRUE(no_temporary_files(root.get())); +} From 50f36345f4aac872dfa8e47cc79f661aa3916c10 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Mon, 26 Oct 2026 13:22:15 -0700 Subject: [PATCH 2/6] [CoreAI] Enable filesystem test fault injection The filesystem tests now own the fault-injection build setting that was removed from the parser-only base, where it had no consumers. Test Plan: Built and ran coreai_host_test locally on macOS 27. All 28 parser and filesystem tests passed, including injected atomic-publication failure. Authored with assistance from Devmate (OpenAI GPT-6-Astra). --- backends/apple/coreai/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/backends/apple/coreai/CMakeLists.txt b/backends/apple/coreai/CMakeLists.txt index 6a01341c31b..85d742adf3d 100644 --- a/backends/apple/coreai/CMakeLists.txt +++ b/backends/apple/coreai/CMakeLists.txt @@ -35,6 +35,7 @@ if(EXECUTORCH_BUILD_TESTS AND CMAKE_SYSTEM_NAME STREQUAL "Darwin") runtime/test/coreai_filesystem_fixture.mm ${_coreai_runtime_sources} ) coreai_configure_objc_target(coreai_host_test) + target_compile_definitions(coreai_host_test PRIVATE COREAI_ASSETS_TESTING=1) find_library(COREAI_FOUNDATION_FRAMEWORK Foundation REQUIRED) target_link_libraries( coreai_host_test PRIVATE executorch GTest::gtest From d3501fe87564e338b9b27fdd593f11897780581c Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Mon, 26 Oct 2026 14:23:10 -0700 Subject: [PATCH 3/6] up --- backends/apple/coreai/README.md | 11 +- .../apple/coreai/runtime/coreai_storage.h | 7 +- .../apple/coreai/runtime/coreai_storage.mm | 38 ++- .../runtime/test/coreai_storage_test.mm | 225 +++++++++++++++++- 4 files changed, 257 insertions(+), 24 deletions(-) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 623a96a14a6..b61bc86e6f1 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -56,8 +56,10 @@ The assets root must be an absolute path. The application chooses it, should reserve it for this backend, and is trusted not to rename, replace or modify its contents while the backend uses it; the backend does not defend against other processes of the same user rebinding paths. Written files and changed directories -are synced with `fsync`; atomic publication also uses `F_FULLFSYNC` before its -rename. +are synced with `fsync`. Atomic publication uses `F_FULLFSYNC` on the temporary +file before rename and on the directory afterward. A failure before rename +preserves the old contents; a failure of the final flush reports an error even +though the new complete contents are already visible. Preparing the assets root sets `NSURLIsExcludedFromBackupKey` on it, which covers everything beneath it. Ancestors are not modified. This is backup exclusion, not @@ -67,8 +69,9 @@ a control for iCloud Drive synchronization. On macOS, `EXECUTORCH_BUILD_COREAI=ON` with `EXECUTORCH_BUILD_TESTS=ON` registers the standalone `coreai_host_test` executable with CTest. It tests -manifest parsing and AOT asset selection using Foundation, without Swift or -CoreAI SDK calls. The host test target is omitted for Apple cross-builds. +manifest parsing, AOT asset selection, and filesystem storage using Foundation, +without Swift or CoreAI SDK calls. The host test target is omitted for Apple +cross-builds. CI builds and runs these tests on macOS 27. To use the same configuration from the repository root: diff --git a/backends/apple/coreai/runtime/coreai_storage.h b/backends/apple/coreai/runtime/coreai_storage.h index f5bd377dd70..e3e1eb5b3a8 100644 --- a/backends/apple/coreai/runtime/coreai_storage.h +++ b/backends/apple/coreai/runtime/coreai_storage.h @@ -30,14 +30,17 @@ runtime::Error write_storage_file( NSString* relative_path, const void* bytes, size_t size); -// Atomically replaces name with data. On error, readers see either the old or -// the new complete bytes. +// Atomically replaces name with data and fully flushes the file and renamed +// entry. On error, readers see either the old or the new complete bytes. runtime::Error publish_storage_data(int root_fd, NSString* name, NSData* data); runtime::Result*> storage_children( int fd, bool skip_invalid_names = false); enum class StorageOperation { + Write, + SyncFile, + FullSync, Rename, BeforeSDK, AfterSDK, diff --git a/backends/apple/coreai/runtime/coreai_storage.mm b/backends/apple/coreai/runtime/coreai_storage.mm index 66fea0e8833..8eed9099f63 100644 --- a/backends/apple/coreai/runtime/coreai_storage.mm +++ b/backends/apple/coreai/runtime/coreai_storage.mm @@ -109,8 +109,8 @@ int storage_fault(StorageOperation operation) { namespace { template -int storage_call(StorageOperation operation, F call) { - return retry_eintr([&] { +auto storage_call(StorageOperation operation, F call) { + return retry_eintr([&]() -> decltype(call()) { const int fault = storage_fault(operation); if (fault != 0) { errno = fault; @@ -175,22 +175,27 @@ FileDescriptor file(retry_eintr([&] { file.get() >= 0, AccessFailed, "Cannot create Core AI storage file"); const auto* bytes = static_cast(data); while (size > 0) { - const ssize_t count = - write(file.get(), bytes, std::min(size, size_t(1024 * 1024))); - if (count < 0 && errno == EINTR) continue; + const ssize_t count = storage_call(StorageOperation::Write, [&] { + return write(file.get(), bytes, std::min(size, size_t(1024 * 1024))); + }); ET_CHECK_OR_RETURN_ERROR( count > 0, AccessFailed, "Cannot write Core AI storage file"); bytes += count; size -= count; } ET_CHECK_OR_RETURN_ERROR( - retry_eintr([&] { return fsync(file.get()); }) == 0 && + storage_call( + StorageOperation::SyncFile, [&] { return fsync(file.get()); }) == 0 && (!full_sync || - retry_eintr([&] { return fcntl(file.get(), F_FULLFSYNC); }) == 0), + storage_call( + StorageOperation::FullSync, + [&] { return fcntl(file.get(), F_FULLFSYNC); }) == 0), AccessFailed, "Cannot synchronize Core AI file"); - ET_CHECK_OR_RETURN_ERROR(::close(file.release()) == 0, AccessFailed, - "Cannot close Core AI storage file"); + ET_CHECK_OR_RETURN_ERROR( + ::close(file.release()) == 0, + AccessFailed, + "Cannot close Core AI storage file"); return sync_storage_directory(parent.get()); } } // namespace @@ -222,8 +227,7 @@ FileDescriptor copy( NSString* name = [[NSString alloc] initWithBytes:entry->d_name length:strlen(entry->d_name) encoding:NSUTF8StringEncoding]; - if (name == nil || - strcmp(name.fileSystemRepresentation, entry->d_name) != 0) { + if (name == nil) { if (skip_invalid_names) { continue; } @@ -287,8 +291,16 @@ Error publish_storage_data(int root_fd, NSString* name, NSData* data) { name.fileSystemRepresentation); }) != 0) result = Error::AccessFailed; - if (result == Error::Ok) - return sync_storage_directory(root_fd); + if (result == Error::Ok) { + // Flush the renamed entry as well as the file's pre-rename contents. + ET_CHECK_OR_RETURN_ERROR( + storage_call( + StorageOperation::FullSync, + [&] { return fcntl(root_fd, F_FULLFSYNC); }) == 0, + AccessFailed, + "Cannot fully synchronize Core AI publication directory"); + return Error::Ok; + } if (unlinkat(root_fd, temporary.fileSystemRepresentation, 0) == 0) { (void)sync_storage_directory(root_fd); } diff --git a/backends/apple/coreai/runtime/test/coreai_storage_test.mm b/backends/apple/coreai/runtime/test/coreai_storage_test.mm index 9b1ae57c5a1..064aab1a9d5 100644 --- a/backends/apple/coreai/runtime/test/coreai_storage_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_storage_test.mm @@ -6,7 +6,10 @@ * LICENSE file in the root directory of this source tree. */ +#include #include +#include +#include #include "coreai_fault_scope.h" #include "coreai_file.h" #include "coreai_filesystem_fixture.h" @@ -68,6 +71,141 @@ FileDescriptor file(openat(root.get(), name.fileSystemRepresentation, } } +TEST(CoreAIStorageTest, ListsComposedAndDecomposedUnicodeNames) { + for (NSString* name in @[ @"caf\u00e9", @"cafe\u0301" ]) { + SCOPED_TRACE(name.UTF8String); + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + FileDescriptor file(openat( + root.get(), + name.UTF8String, + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, + 0600)); + ASSERT_GE(file.get(), 0); + for (bool skip_invalid : {false, true}) { + auto names = storage_children(root.get(), skip_invalid); + ASSERT_TRUE(names.ok()); + ASSERT_EQ(names.get().count, 1); + EXPECT_STREQ(names.get().firstObject.UTF8String, name.UTF8String); + } + } +} + +TEST(CoreAIStorageTest, SkipsInvalidUtf8NamesOnlyWhenRequested) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + FileDescriptor invalid(openat( + root.get(), + "invalid-\xff", + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, + 0600)); + if (invalid.get() < 0 && errno == EILSEQ) { + GTEST_SKIP() << "Filesystem rejects invalid UTF-8 filenames"; + } + ASSERT_GE(invalid.get(), 0); + FileDescriptor valid(openat( + root.get(), "valid", O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); + ASSERT_GE(valid.get(), 0); + EXPECT_EQ(storage_children(root.get()).error(), Error::InvalidExternalData); + auto names = storage_children(root.get(), true); + ASSERT_TRUE(names.ok()); + EXPECT_TRUE(([names.get() isEqual:@[ @"valid" ]])); + EXPECT_EQ(unlinkat(root.get(), "invalid-\xff", 0), 0); +} + +TEST(CoreAIStorageTest, CreatesNestedFilesWithoutOverwritingExistingContents) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSData* original = [@"original" dataUsingEncoding:NSUTF8StringEncoding]; + NSData* replacement = [@"replacement" dataUsingEncoding:NSUTF8StringEncoding]; + NSString* path = @"nested/parents/model.bin"; + NSURL* file = [storage.url URLByAppendingPathComponent:path]; + ASSERT_EQ( + write_storage_file(root.get(), path, original.bytes, original.length), + Error::Ok); + EXPECT_TRUE(([[NSData dataWithContentsOfURL:file] isEqual:original])); + EXPECT_EQ( + write_storage_file( + root.get(), path, replacement.bytes, replacement.length), + Error::AccessFailed); + EXPECT_TRUE(([[NSData dataWithContentsOfURL:file] isEqual:original])); + ASSERT_EQ( + write_storage_file(root.get(), @"nested/parents/empty", nullptr, 0), + Error::Ok); + NSData* empty = [NSData + dataWithContentsOfURL: + [storage.url URLByAppendingPathComponent:@"nested/parents/empty"]]; + ASSERT_NE(empty, nil); + EXPECT_EQ(empty.length, 0); +} + +TEST(CoreAIStorageTest, RejectsUnsafeWritePathsWithoutCreatingFiles) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + for (NSString* path in @[ + @"", + @"..", + @"../escape", + @"a/../b", + @"a/./b", + @"a//b", + @"/leading", + @"trailing/" + ]) { + SCOPED_TRACE(path.UTF8String); + EXPECT_EQ( + write_storage_file(root.get(), path, "x", 1), Error::InvalidArgument); + } + EXPECT_EQ( + write_storage_file(root.get(), @"no-data", nullptr, 1), + Error::InvalidArgument); + auto names = storage_children(root.get()); + ASSERT_TRUE(names.ok()); + EXPECT_EQ(names.get().count, 0); +} + +TEST(CoreAIStorageTest, CreatesAndReopensSharedFileWithoutTruncation) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + FileDescriptor created(open_storage_shared_file(root.get(), "shared")); + ASSERT_GE(created.get(), 0); + ASSERT_EQ(write(created.get(), "data", 4), 4); + FileDescriptor reopened(open_storage_shared_file(root.get(), "shared")); + ASSERT_GE(reopened.get(), 0); + EXPECT_NE(created.get(), reopened.get()); + EXPECT_NE(fcntl(created.get(), F_GETFD) & FD_CLOEXEC, 0); + EXPECT_NE(fcntl(reopened.get(), F_GETFD) & FD_CLOEXEC, 0); + struct stat first, second; + ASSERT_EQ(fstat(created.get(), &first), 0); + ASSERT_EQ(fstat(reopened.get(), &second), 0); + EXPECT_EQ(first.st_ino, second.st_ino); + EXPECT_EQ(second.st_size, 4); + char bytes[4]; + ASSERT_EQ(read(reopened.get(), bytes, sizeof(bytes)), sizeof(bytes)); + EXPECT_EQ(memcmp(bytes, "data", sizeof(bytes)), 0); + EXPECT_EQ(lseek(created.get(), 0, SEEK_CUR), 4); +} + TEST(CoreAIStorageTest, RetriesEintr) { int calls = 0; const ssize_t count = retry_eintr([&]() -> ssize_t { @@ -99,15 +237,92 @@ FileDescriptor root( NSData* next = [@"next" dataUsingEncoding:NSUTF8StringEncoding]; NSURL* published = [storage.url URLByAppendingPathComponent:@"published"]; ASSERT_EQ(publish_storage_data(root.get(), @"published", old), Error::Ok); - { - StorageFaultScope fault(StorageOperation::Rename); - EXPECT_EQ(publish_storage_data(root.get(), @"published", next), Error::AccessFailed); + for (StorageOperation operation : + {StorageOperation::Write, + StorageOperation::SyncFile, + StorageOperation::FullSync, + StorageOperation::Rename}) { + SCOPED_TRACE(static_cast(operation)); + StorageFaultScope fault(operation); + EXPECT_EQ( + publish_storage_data(root.get(), @"published", next), + Error::AccessFailed); EXPECT_EQ(fault.hits, 1); + EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:old])); + EXPECT_TRUE(no_temporary_files(root.get())); } - EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:old])); - EXPECT_TRUE(no_temporary_files(root.get())); ASSERT_EQ(publish_storage_data(root.get(), @"published", next), Error::Ok); EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:next])); EXPECT_TRUE(no_temporary_files(root.get())); } + +TEST(CoreAIStorageTest, FullyFlushesPublicationBeforeAndAfterRename) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + __block std::vector operations; + StorageFaultScope fault(StorageOperation::FullSync); + fault.armed = false; + fault.observe = ^(StorageOperation operation) { + operations.push_back(operation); + }; + NSData* data = [@"published" dataUsingEncoding:NSUTF8StringEncoding]; + EXPECT_EQ(publish_storage_data(root.get(), @"published", data), Error::Ok); + EXPECT_EQ( + operations, + (std::vector{ + StorageOperation::Write, + StorageOperation::SyncFile, + StorageOperation::FullSync, + StorageOperation::Rename, + StorageOperation::FullSync})); +} + +TEST(CoreAIStorageTest, ReportsPostRenameFlushFailureWithNewContentsVisible) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSData* old = [@"old" dataUsingEncoding:NSUTF8StringEncoding]; + NSData* next = [@"next" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(publish_storage_data(root.get(), @"published", old), Error::Ok); + StorageFaultScope fault(StorageOperation::FullSync); + fault.match_index = 2; + EXPECT_EQ( + publish_storage_data(root.get(), @"published", next), + Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + NSURL* published = [storage.url URLByAppendingPathComponent:@"published"]; + EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:next])); + EXPECT_TRUE(no_temporary_files(root.get())); +} + +TEST(CoreAIStorageTest, RetriesInterruptedPublicationOperations) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSData* data = [@"published" dataUsingEncoding:NSUTF8StringEncoding]; + for (StorageOperation operation : + {StorageOperation::Write, + StorageOperation::SyncFile, + StorageOperation::FullSync, + StorageOperation::Rename}) { + SCOPED_TRACE(static_cast(operation)); + StorageFaultScope fault(operation); + fault.error = EINTR; + EXPECT_EQ(publish_storage_data(root.get(), @"published", data), Error::Ok); + EXPECT_EQ(fault.hits, 1); + NSURL* published = [storage.url URLByAppendingPathComponent:@"published"]; + EXPECT_TRUE(([[NSData dataWithContentsOfURL:published] isEqual:data])); + EXPECT_TRUE(no_temporary_files(root.get())); + } +} From 20d9d37d2ddb6e6f366b812f048e4eaeff9d2828 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Tue, 27 Oct 2026 12:06:17 -0700 Subject: [PATCH 4/6] [CoreAI] Harden storage failure handling and directory enumeration Summary: Keep failed writes from blocking retries and prevent directory listings from disturbing other readers. Reject invalid roots consistently, preserve strict publication durability, and cover directory-sync failures. Remove a fixture that APFS cannot create. Test Plan: All 46 native tests passed with zero skips, including partial-write cleanup and retry, independent enumeration offsets, root validation, and directory-sync ordering/failure/interruption checks. All 113 CoreAI Python tests passed. Formatting and whitespace checks passed. --- backends/apple/coreai/README.md | 3 +- .../apple/coreai/runtime/coreai_storage.h | 3 + .../apple/coreai/runtime/coreai_storage.mm | 153 ++++++++++------- .../runtime/test/coreai_filesystem_fixture.mm | 28 ++- .../runtime/test/coreai_storage_test.mm | 159 +++++++++++++----- 5 files changed, 233 insertions(+), 113 deletions(-) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index b61bc86e6f1..09361117fd6 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -59,7 +59,8 @@ processes of the same user rebinding paths. Written files and changed directorie are synced with `fsync`. Atomic publication uses `F_FULLFSYNC` on the temporary file before rename and on the directory afterward. A failure before rename preserves the old contents; a failure of the final flush reports an error even -though the new complete contents are already visible. +though the new complete contents are already visible. Use a local APFS volume; +unsupported full-device flushes are reported as errors, not downgraded to `fsync`. Preparing the assets root sets `NSURLIsExcludedFromBackupKey` on it, which covers everything beneath it. Ancestors are not modified. This is backup exclusion, not diff --git a/backends/apple/coreai/runtime/coreai_storage.h b/backends/apple/coreai/runtime/coreai_storage.h index e3e1eb5b3a8..bc583e59997 100644 --- a/backends/apple/coreai/runtime/coreai_storage.h +++ b/backends/apple/coreai/runtime/coreai_storage.h @@ -25,6 +25,7 @@ runtime::Error sync_storage_directory(int fd); // Returns an independently owned descriptor for a stable coordination file. int open_storage_shared_file(int parent_fd, const char* name); // Creates a new file and missing parents, then syncs the file and its parent. +// On failure, attempts to remove the new file; created parents may remain. runtime::Error write_storage_file( int root_fd, NSString* relative_path, @@ -33,6 +34,7 @@ runtime::Error write_storage_file( // Atomically replaces name with data and fully flushes the file and renamed // entry. On error, readers see either the old or the new complete bytes. runtime::Error publish_storage_data(int root_fd, NSString* name, NSData* data); +// Lists sorted names without changing fd's enumeration offset. runtime::Result*> storage_children( int fd, bool skip_invalid_names = false); @@ -40,6 +42,7 @@ runtime::Result*> storage_children( enum class StorageOperation { Write, SyncFile, + SyncDirectory, FullSync, Rename, BeforeSDK, diff --git a/backends/apple/coreai/runtime/coreai_storage.mm b/backends/apple/coreai/runtime/coreai_storage.mm index 8eed9099f63..5c32d432cd4 100644 --- a/backends/apple/coreai/runtime/coreai_storage.mm +++ b/backends/apple/coreai/runtime/coreai_storage.mm @@ -9,9 +9,9 @@ #import "coreai_storage.h" #include "coreai_file.h" -#include -#include #include +#include +#include #include #include #include @@ -49,20 +49,32 @@ Error ensure_excluded_from_backup(NSURL* directory) { } Result default_coreai_assets_root() { - NSURL* caches = [NSFileManager.defaultManager - URLsForDirectory:NSCachesDirectory - inDomains:NSUserDomainMask].firstObject; - ET_CHECK_OR_RETURN_ERROR(caches.isFileURL, AccessFailed, - "Cannot resolve Core AI cache directory"); + NSURL* caches = + [NSFileManager.defaultManager URLsForDirectory:NSCachesDirectory + inDomains:NSUserDomainMask] + .firstObject; + ET_CHECK_OR_RETURN_ERROR( + caches.isFileURL, AccessFailed, "Cannot resolve Core AI cache directory"); return [caches.path stringByAppendingPathComponent:@"executorch_coreai"]; } Result prepare_storage_root(NSString* path, bool create) { ET_CHECK_OR_RETURN_ERROR( - [path isKindOfClass:NSString.class] && path.isAbsolutePath && + [path isKindOfClass:NSString.class] && [path hasPrefix:@"/"] && [path rangeOfString:@"\0"].location == NSNotFound, InvalidArgument, "Core AI storage requires an absolute path"); + struct stat info; + const int result = + retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); + ET_CHECK_OR_RETURN_ERROR( + result == 0 || errno == ENOENT || errno == ENOTDIR, + AccessFailed, + "Cannot inspect Core AI storage directory"); + ET_CHECK_OR_RETURN_ERROR( + result == 0 ? S_ISDIR(info.st_mode) : create && errno == ENOENT, + InvalidArgument, + "Core AI storage must be a directory"); if (create) { NSError* error = nil; ET_CHECK_OR_RETURN_ERROR( @@ -76,19 +88,6 @@ Error ensure_excluded_from_backup(NSURL* directory) { AccessFailed, "Cannot create Core AI storage directory: %s", error.localizedDescription.UTF8String); - } - struct stat info; - const int result = - retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); - ET_CHECK_OR_RETURN_ERROR( - result == 0 || errno == ENOENT || errno == ENOTDIR, - AccessFailed, - "Cannot inspect Core AI storage directory"); - ET_CHECK_OR_RETURN_ERROR( - result == 0 && S_ISDIR(info.st_mode), - InvalidArgument, - "Core AI storage must be an existing directory"); - if (create) { ET_CHECK_OK_OR_RETURN_ERROR(ensure_excluded_from_backup( [NSURL fileURLWithPath:path isDirectory:YES])); } @@ -142,14 +141,15 @@ Error write_file( ET_CHECK_OR_RETURN_ERROR( storage_component(component), InvalidArgument, "Invalid storage path"); } - FileDescriptor parent(retry_eintr([&] { return dup(root_fd); })); + FileDescriptor parent( + retry_eintr([&] { return fcntl(root_fd, F_DUPFD_CLOEXEC, 0); })); ET_CHECK_OR_RETURN_ERROR( parent.get() >= 0, AccessFailed, "Cannot open storage parent"); for (NSUInteger i = 0; i + 1 < components.count; ++i) { const char* component = components[i].fileSystemRepresentation; ET_CHECK_OR_RETURN_ERROR( - retry_eintr( - [&] { return mkdirat(parent.get(), component, 0700); }) == 0 || + retry_eintr([&] { return mkdirat(parent.get(), component, 0700); }) == + 0 || errno == EEXIST, AccessFailed, "Cannot create Core AI source directory"); @@ -173,52 +173,70 @@ FileDescriptor file(retry_eintr([&] { })); ET_CHECK_OR_RETURN_ERROR( file.get() >= 0, AccessFailed, "Cannot create Core AI storage file"); - const auto* bytes = static_cast(data); - while (size > 0) { - const ssize_t count = storage_call(StorageOperation::Write, [&] { - return write(file.get(), bytes, std::min(size, size_t(1024 * 1024))); - }); + const auto result = [&]() -> Error { + const auto* bytes = static_cast(data); + while (size > 0) { + const ssize_t count = storage_call(StorageOperation::Write, [&] { + return write(file.get(), bytes, std::min(size, size_t(1024 * 1024))); + }); + ET_CHECK_OR_RETURN_ERROR( + count > 0, AccessFailed, "Cannot write Core AI storage file"); + bytes += count; + size -= count; + } + ET_CHECK_OR_RETURN_ERROR( + storage_call( + StorageOperation::SyncFile, [&] { return fsync(file.get()); }) == + 0 && + (!full_sync || + storage_call( + StorageOperation::FullSync, + [&] { return fcntl(file.get(), F_FULLFSYNC); }) == 0), + AccessFailed, + "Cannot synchronize Core AI file"); ET_CHECK_OR_RETURN_ERROR( - count > 0, AccessFailed, "Cannot write Core AI storage file"); - bytes += count; - size -= count; + ::close(file.release()) == 0, + AccessFailed, + "Cannot close Core AI storage file"); + return sync_storage_directory(parent.get()); + }(); + if (result != Error::Ok) { + if (retry_eintr([&] { + return unlinkat( + parent.get(), components.lastObject.fileSystemRepresentation, 0); + }) == 0) { + (void)sync_storage_directory(parent.get()); + } else { + ET_LOG( + Error, "Cannot remove failed Core AI storage write: errno=%d", errno); + } } - ET_CHECK_OR_RETURN_ERROR( - storage_call( - StorageOperation::SyncFile, [&] { return fsync(file.get()); }) == 0 && - (!full_sync || - storage_call( - StorageOperation::FullSync, - [&] { return fcntl(file.get(), F_FULLFSYNC); }) == 0), - AccessFailed, - "Cannot synchronize Core AI file"); - ET_CHECK_OR_RETURN_ERROR( - ::close(file.release()) == 0, - AccessFailed, - "Cannot close Core AI storage file"); - return sync_storage_directory(parent.get()); + return result; } } // namespace Result*> storage_children(int fd, bool skip_invalid_names) { - FileDescriptor copy( - retry_eintr([&] { return fcntl(fd, F_DUPFD_CLOEXEC, 0); })); - ET_CHECK_OR_RETURN_ERROR(copy.get() >= 0, AccessFailed, - "Cannot duplicate storage directory"); + // Reopening avoids sharing the caller's directory offset. + FileDescriptor copy(retry_eintr( + [&] { return openat(fd, ".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); })); + ET_CHECK_OR_RETURN_ERROR( + copy.get() >= 0, AccessFailed, "Cannot open storage directory"); DIR* directory = fdopendir(copy.get()); - ET_CHECK_OR_RETURN_ERROR(directory != nullptr, AccessFailed, - "Cannot open storage directory stream"); + ET_CHECK_OR_RETURN_ERROR( + directory != nullptr, + AccessFailed, + "Cannot open storage directory stream"); copy.release(); - // dup shares the enumeration offset with the original descriptor. - rewinddir(directory); NSMutableArray* names = [NSMutableArray array]; Error result = Error::Ok; while (true) { errno = 0; dirent* entry = readdir(directory); if (entry == nullptr) { - if (errno == EINTR) continue; - if (errno != 0) result = Error::AccessFailed; + if (errno == EINTR) + continue; + if (errno != 0) + result = Error::AccessFailed; break; } if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { @@ -238,16 +256,19 @@ FileDescriptor copy( } const int closed = closedir(directory); ET_CHECK_OK_OR_RETURN_ERROR(result); - ET_CHECK_OR_RETURN_ERROR(closed == 0, AccessFailed, - "Cannot close storage directory stream"); + ET_CHECK_OR_RETURN_ERROR( + closed == 0, AccessFailed, "Cannot close storage directory stream"); return [names sortedArrayUsingSelector:@selector(compare:)]; } int open_storage_shared_file(int parent_fd, const char* name) { // APFS can return ENOENT to a concurrent O_CREAT loser without O_EXCL. int fd = retry_eintr([&] { - return openat(parent_fd, name, - O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, 0600); + return openat( + parent_fd, + name, + O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, + 0600); }); if (fd < 0 && errno == EEXIST) { fd = retry_eintr([&] { @@ -259,7 +280,8 @@ int open_storage_shared_file(int parent_fd, const char* name) { Error sync_storage_directory(int fd) { ET_CHECK_OR_RETURN_ERROR( - retry_eintr([&] { return fsync(fd); }) == 0, + storage_call( + StorageOperation::SyncDirectory, [&] { return fsync(fd); }) == 0, AccessFailed, "Cannot synchronize Core AI directory"); return Error::Ok; @@ -281,16 +303,17 @@ Error publish_storage_data(int root_fd, NSString* name, NSData* data) { NSString* temporary = [@".tmp-" stringByAppendingString:NSUUID.UUID.UUIDString]; // Bookmarks are small and costly to lose, so they get a full device flush. - auto result = - write_file(root_fd, temporary, data.bytes, data.length, true); + auto result = write_file(root_fd, temporary, data.bytes, data.length, true); if (result == Error::Ok && storage_call(StorageOperation::Rename, [&] { return renameat( root_fd, temporary.fileSystemRepresentation, root_fd, name.fileSystemRepresentation); - }) != 0) + }) != 0) { + ET_LOG(Error, "Cannot publish Core AI storage data: errno=%d", errno); result = Error::AccessFailed; + } if (result == Error::Ok) { // Flush the renamed entry as well as the file's pre-rename contents. ET_CHECK_OR_RETURN_ERROR( @@ -307,4 +330,4 @@ Error publish_storage_data(int root_fd, NSString* name, NSData* data) { return result; } -} // namespace executorch::backends::coreai +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm index f9810e02aad..6c60dddd6c5 100644 --- a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm @@ -17,14 +17,17 @@ ADD_FAILURE() << "mkdtemp: " << strerror(errno); return; } - url = [NSURL fileURLWithPath:[NSString stringWithUTF8String:pattern] isDirectory:YES] + url = [NSURL fileURLWithPath:[NSString stringWithUTF8String:pattern] + isDirectory:YES] .URLByResolvingSymlinksInPath; } TestDirectory::~TestDirectory() { - if (url == nil) return; + if (url == nil) + return; EXPECT_TRUE([url.lastPathComponent hasPrefix:@"coreai-host-test-"]); - if (![url.lastPathComponent hasPrefix:@"coreai-host-test-"]) return; + if (![url.lastPathComponent hasPrefix:@"coreai-host-test-"]) + return; EXPECT_TRUE([NSFileManager.defaultManager removeItemAtURL:url error:nil]); } @@ -33,13 +36,16 @@ [fresh removeAllCachedResourceValues]; NSNumber* value = nil; NSError* error = nil; - if (![fresh getResourceValue:&value forKey:NSURLIsExcludedFromBackupKey error:&error] || + if (![fresh getResourceValue:&value + forKey:NSURLIsExcludedFromBackupKey + error:&error] || error != nil || value == nil) { ADD_FAILURE() << "Cannot read backup flag: " << url.path.UTF8String; return ::testing::AssertionFailure() << "Backup flag unavailable"; } return value.boolValue ? ::testing::AssertionSuccess() - : ::testing::AssertionFailure() << "Not excluded: " << url.path.UTF8String; + : ::testing::AssertionFailure() + << "Not excluded: " << url.path.UTF8String; } ::testing::AssertionResult set_backup_excluded(NSURL* url, bool excluded) { @@ -47,15 +53,19 @@ for (int attempt = 0; attempt < 50; ++attempt) { NSURL* fresh = [NSURL fileURLWithPath:url.path]; [fresh removeAllCachedResourceValues]; - if (![fresh setResourceValue:@(excluded) forKey:NSURLIsExcludedFromBackupKey error:nil]) { - return ::testing::AssertionFailure() << "Cannot set backup flag: " << url.path.UTF8String; + if (![fresh setResourceValue:@(excluded) + forKey:NSURLIsExcludedFromBackupKey + error:nil]) { + return ::testing::AssertionFailure() + << "Cannot set backup flag: " << url.path.UTF8String; } usleep(20000); if (static_cast(backup_excluded(url)) == excluded) { return ::testing::AssertionSuccess(); } } - return ::testing::AssertionFailure() << "Backup flag did not settle: " << url.path.UTF8String; + return ::testing::AssertionFailure() + << "Backup flag did not settle: " << url.path.UTF8String; } -} // namespace executorch::backends::coreai::testing +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_storage_test.mm b/backends/apple/coreai/runtime/test/coreai_storage_test.mm index 064aab1a9d5..77b23eef53f 100644 --- a/backends/apple/coreai/runtime/test/coreai_storage_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_storage_test.mm @@ -6,9 +6,10 @@ * LICENSE file in the root directory of this source tree. */ -#include #include #include +#include +#include #include #include "coreai_fault_scope.h" #include "coreai_file.h" @@ -40,34 +41,72 @@ auto inspected = prepare_storage_root(root.path, false); ASSERT_TRUE((inspected.ok() && [inspected.get() isEqualToString:root.path])); ASSERT_TRUE((!backup_excluded(root))); - ASSERT_TRUE((prepare_storage_root(root.path, true).ok() && backup_excluded(root))); + ASSERT_TRUE( + (prepare_storage_root(root.path, true).ok() && backup_excluded(root))); NSURL* missing = [sandbox.url URLByAppendingPathComponent:@"missing"]; - ASSERT_EQ(prepare_storage_root(missing.path, false).error(), Error::InvalidArgument); + ASSERT_EQ( + prepare_storage_root(missing.path, false).error(), + Error::InvalidArgument); EXPECT_FALSE([manager fileExistsAtPath:missing.path]); - EXPECT_EQ(prepare_storage_root(@"relative", true).error(), Error::InvalidArgument); + EXPECT_EQ( + prepare_storage_root(@"relative", true).error(), Error::InvalidArgument); EXPECT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); } +TEST(CoreAIStorageTest, RejectsInvalidRootsConsistently) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* regular = [storage.url URLByAppendingPathComponent:@"regular"]; + NSData* bytes = [@"unchanged" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_TRUE([bytes writeToURL:regular atomically:NO]); + for (bool create : {false, true}) { + SCOPED_TRACE(create); + EXPECT_EQ( + prepare_storage_root(nil, create).error(), Error::InvalidArgument); + for (NSString* path in @[ + @"", + @"relative", + @"~", + @"~/cache", + @"~nobody/cache", + regular.path, + [regular.path stringByAppendingPathComponent:@"child"] + ]) { + SCOPED_TRACE(path.UTF8String); + EXPECT_EQ( + prepare_storage_root(path, create).error(), Error::InvalidArgument); + } + } + EXPECT_TRUE([[NSData dataWithContentsOfURL:regular] isEqual:bytes]); +} + TEST(CoreAIStorageTest, ListsChildrenRepeatedlyInSortedOrder) { TestDirectory storage; ASSERT_NE(storage.url, nil); - FileDescriptor root( - open(storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_TRUE((root.get() >= 0)); NSMutableArray* expected = [NSMutableArray array]; for (int i = 7; i >= 0; --i) { NSString* name = [NSString stringWithFormat:@"entry-%d", i]; - FileDescriptor file(openat(root.get(), name.fileSystemRepresentation, - O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); + FileDescriptor file(openat( + root.get(), + name.fileSystemRepresentation, + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, + 0600)); ASSERT_TRUE((file.get() >= 0)); [expected insertObject:name atIndex:0]; } + const off_t position = lseek(root.get(), 0, SEEK_CUR); + ASSERT_GE(position, 0); for (int pass = 0; pass < 3; ++pass) { errno = EIO; auto names = storage_children(root.get()); ASSERT_TRUE((names.ok() && [names.get() isEqual:expected])); ASSERT_TRUE((fcntl(root.get(), F_GETFD) >= 0)); + EXPECT_EQ(lseek(root.get(), 0, SEEK_CUR), position); } } @@ -95,32 +134,6 @@ FileDescriptor file(openat( } } -TEST(CoreAIStorageTest, SkipsInvalidUtf8NamesOnlyWhenRequested) { - TestDirectory storage; - ASSERT_NE(storage.url, nil); - FileDescriptor root(open( - storage.url.fileSystemRepresentation, - O_RDONLY | O_DIRECTORY | O_CLOEXEC)); - ASSERT_GE(root.get(), 0); - FileDescriptor invalid(openat( - root.get(), - "invalid-\xff", - O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, - 0600)); - if (invalid.get() < 0 && errno == EILSEQ) { - GTEST_SKIP() << "Filesystem rejects invalid UTF-8 filenames"; - } - ASSERT_GE(invalid.get(), 0); - FileDescriptor valid(openat( - root.get(), "valid", O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); - ASSERT_GE(valid.get(), 0); - EXPECT_EQ(storage_children(root.get()).error(), Error::InvalidExternalData); - auto names = storage_children(root.get(), true); - ASSERT_TRUE(names.ok()); - EXPECT_TRUE(([names.get() isEqual:@[ @"valid" ]])); - EXPECT_EQ(unlinkat(root.get(), "invalid-\xff", 0), 0); -} - TEST(CoreAIStorageTest, CreatesNestedFilesWithoutOverwritingExistingContents) { TestDirectory storage; ASSERT_NE(storage.url, nil); @@ -151,6 +164,70 @@ FileDescriptor root(open( EXPECT_EQ(empty.length, 0); } +TEST(CoreAIStorageTest, RemovesFailedWritesAndAllowsRetry) { + const struct { + StorageOperation operation; + int match_index; + } rows[] = { + {StorageOperation::Write, 2}, + {StorageOperation::SyncFile, 1}, + {StorageOperation::SyncDirectory, 1}, + {StorageOperation::SyncDirectory, 2}}; + const std::vector bytes(1024 * 1024 + 1, 0x5a); + for (const auto& row : rows) { + SCOPED_TRACE(static_cast(row.operation)); + SCOPED_TRACE(row.match_index); + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSString* path = @"nested/model.bin"; + NSURL* file = [storage.url URLByAppendingPathComponent:path]; + StorageFaultScope fault(row.operation); + fault.match_index = row.match_index; + EXPECT_EQ( + write_storage_file(root.get(), path, bytes.data(), bytes.size()), + Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:file.path]); + ASSERT_EQ( + write_storage_file(root.get(), path, bytes.data(), bytes.size()), + Error::Ok); + NSData* actual = [NSData dataWithContentsOfURL:file]; + ASSERT_NE(actual, nil); + ASSERT_EQ(actual.length, bytes.size()); + EXPECT_EQ(memcmp(actual.bytes, bytes.data(), bytes.size()), 0); + } +} + +TEST(CoreAIStorageTest, SyncsEachCreatedParentAndTheWrittenFile) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + __block std::vector operations; + StorageFaultScope fault(StorageOperation::SyncDirectory); + fault.armed = false; + fault.observe = ^(StorageOperation operation) { + operations.push_back(operation); + }; + EXPECT_EQ( + write_storage_file(root.get(), @"nested/parents/model.bin", "x", 1), + Error::Ok); + EXPECT_EQ( + operations, + (std::vector{ + StorageOperation::SyncDirectory, + StorageOperation::SyncDirectory, + StorageOperation::Write, + StorageOperation::SyncFile, + StorageOperation::SyncDirectory})); +} + TEST(CoreAIStorageTest, RejectsUnsafeWritePathsWithoutCreatingFiles) { TestDirectory storage; ASSERT_NE(storage.url, nil); @@ -220,9 +297,11 @@ FileDescriptor root(open( static ::testing::AssertionResult no_temporary_files(int root) { auto children = storage_children(root); - if (!children.ok()) return ::testing::AssertionFailure() << "Cannot list storage"; + if (!children.ok()) + return ::testing::AssertionFailure() << "Cannot list storage"; for (NSString* child in children.get()) { - if ([child hasPrefix:@".tmp-"]) return ::testing::AssertionFailure() << child.UTF8String; + if ([child hasPrefix:@".tmp-"]) + return ::testing::AssertionFailure() << child.UTF8String; } return ::testing::AssertionSuccess(); } @@ -230,8 +309,9 @@ FileDescriptor root(open( TEST(CoreAIStorageTest, AtomicPublicationPreservesOldBytesOnFailure) { TestDirectory storage; ASSERT_NE(storage.url, nil); - FileDescriptor root( - open(storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_TRUE((root.get() >= 0)); NSData* old = [@"old" dataUsingEncoding:NSUTF8StringEncoding]; NSData* next = [@"next" dataUsingEncoding:NSUTF8StringEncoding]; @@ -240,6 +320,7 @@ FileDescriptor root( for (StorageOperation operation : {StorageOperation::Write, StorageOperation::SyncFile, + StorageOperation::SyncDirectory, StorageOperation::FullSync, StorageOperation::Rename}) { SCOPED_TRACE(static_cast(operation)); @@ -278,6 +359,7 @@ FileDescriptor root(open( StorageOperation::Write, StorageOperation::SyncFile, StorageOperation::FullSync, + StorageOperation::SyncDirectory, StorageOperation::Rename, StorageOperation::FullSync})); } @@ -314,6 +396,7 @@ FileDescriptor root(open( for (StorageOperation operation : {StorageOperation::Write, StorageOperation::SyncFile, + StorageOperation::SyncDirectory, StorageOperation::FullSync, StorageOperation::Rename}) { SCOPED_TRACE(static_cast(operation)); From 94e3b32f52993f1374c2455ef399624e7d81da2c Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Wed, 28 Oct 2026 15:30:18 -0700 Subject: [PATCH 5/6] [CoreAI] Protect new caches and reuse existing directories Reduce redundant filesystem work during cache preparation, particularly on iOS, while giving newly created cache contents an explicit first-unlock protection policy. Preserve backup exclusion and durability behavior, use default directory permissions, and leave existing cache contents unmigrated. Test Plan: Native filesystem tests passed, including protection inheritance, default permissions, creation-free warm paths, concurrent creation, invalid paths, and failure/retry durability. The combined bookmark-layer suite previously passed all 95 native tests; the Python suite passed all 113 tests. --- backends/apple/coreai/README.md | 10 + .../apple/coreai/runtime/coreai_storage.h | 9 + .../apple/coreai/runtime/coreai_storage.mm | 77 ++++-- .../runtime/test/coreai_storage_test.mm | 237 ++++++++++++++++++ 4 files changed, 310 insertions(+), 23 deletions(-) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 09361117fd6..2ca9d0b8665 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -66,6 +66,16 @@ Preparing the assets root sets `NSURLIsExcludedFromBackupKey` on it, which cover everything beneath it. Ancestors are not modified. This is backup exclusion, not a control for iCloud Drive synchronization. +Newly created roots request +`NSFileProtectionCompleteUntilFirstUserAuthentication`. On iOS, this allows cache +access while the device is locked after its first unlock following a reboot. New +subdirectories and files inherit their parent's protection. Existing cache +contents are not migrated. Directory preparation skips creation when the root +already exists; child directories are opened first and created only when missing. +Directory creation uses default permissions filtered by the process umask, without +an explicit owner-only restriction. Existing permissions are not changed. +Backup exclusion and required durability flushes still apply on reused paths. + ## Host Tests On macOS, `EXECUTORCH_BUILD_COREAI=ON` with `EXECUTORCH_BUILD_TESTS=ON` diff --git a/backends/apple/coreai/runtime/coreai_storage.h b/backends/apple/coreai/runtime/coreai_storage.h index bc583e59997..d807eb523de 100644 --- a/backends/apple/coreai/runtime/coreai_storage.h +++ b/backends/apple/coreai/runtime/coreai_storage.h @@ -22,6 +22,14 @@ runtime::Result prepare_storage_root(NSString* path, bool create); runtime::Error ensure_excluded_from_backup(NSURL* directory); runtime::Error sync_storage_directory(int fd); +struct StorageDirectory { + int fd; + bool created; +}; +// Opens one child component without following symlinks. The caller owns fd; +// missing with create=false returns {-1, false}. Does not flush directories. +runtime::Result +open_storage_directory(int parent_fd, NSString* name, bool create); // Returns an independently owned descriptor for a stable coordination file. int open_storage_shared_file(int parent_fd, const char* name); // Creates a new file and missing parents, then syncs the file and its parent. @@ -45,6 +53,7 @@ enum class StorageOperation { SyncDirectory, FullSync, Rename, + BeforeCreateDirectory, BeforeSDK, AfterSDK, BeforeEvict, diff --git a/backends/apple/coreai/runtime/coreai_storage.mm b/backends/apple/coreai/runtime/coreai_storage.mm index 5c32d432cd4..550ffaaf1ea 100644 --- a/backends/apple/coreai/runtime/coreai_storage.mm +++ b/backends/apple/coreai/runtime/coreai_storage.mm @@ -75,19 +75,26 @@ Error ensure_excluded_from_backup(NSURL* directory) { result == 0 ? S_ISDIR(info.st_mode) : create && errno == ENOENT, InvalidArgument, "Core AI storage must be a directory"); - if (create) { + if (create && result != 0) { + NSDictionary* attributes = @{ + NSFileProtectionKey : + NSFileProtectionCompleteUntilFirstUserAuthentication, + }; + ET_CHECK_OR_RETURN_ERROR( + storage_fault(StorageOperation::BeforeCreateDirectory) == 0, + AccessFailed, + "Cannot begin Core AI directory creation"); NSError* error = nil; ET_CHECK_OR_RETURN_ERROR( - [NSFileManager.defaultManager - createDirectoryAtPath:path - withIntermediateDirectories:YES - attributes:@{ - NSFilePosixPermissions : @0700 - } - error:&error], + [NSFileManager.defaultManager createDirectoryAtPath:path + withIntermediateDirectories:YES + attributes:attributes + error:&error], AccessFailed, "Cannot create Core AI storage directory: %s", error.localizedDescription.UTF8String); + } + if (create) { ET_CHECK_OK_OR_RETURN_ERROR(ensure_excluded_from_backup( [NSURL fileURLWithPath:path isDirectory:YES])); } @@ -146,21 +153,10 @@ FileDescriptor parent( ET_CHECK_OR_RETURN_ERROR( parent.get() >= 0, AccessFailed, "Cannot open storage parent"); for (NSUInteger i = 0; i + 1 < components.count; ++i) { - const char* component = components[i].fileSystemRepresentation; - ET_CHECK_OR_RETURN_ERROR( - retry_eintr([&] { return mkdirat(parent.get(), component, 0700); }) == - 0 || - errno == EEXIST, - AccessFailed, - "Cannot create Core AI source directory"); - FileDescriptor next(retry_eintr([&] { - return openat( - parent.get(), - component, - O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - })); - ET_CHECK_OR_RETURN_ERROR( - next.get() >= 0, AccessFailed, "Cannot open source directory"); + auto opened = open_storage_directory(parent.get(), components[i], true); + if (!opened.ok()) + return opened.error(); + FileDescriptor next(opened.get().fd); ET_CHECK_OK_OR_RETURN_ERROR(sync_storage_directory(parent.get())); parent.reset(next.release()); } @@ -215,6 +211,41 @@ FileDescriptor file(retry_eintr([&] { } } // namespace +Result +open_storage_directory(int parent_fd, NSString* name, bool create) { + ET_CHECK_OR_RETURN_ERROR( + storage_component(name), InvalidArgument, "Invalid storage directory name"); + const auto open_directory = [&] { + return retry_eintr([&] { + return openat( + parent_fd, + name.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + }); + }; + int fd = open_directory(); + bool created = false; + if (fd < 0 && errno == ENOENT && create) { + ET_CHECK_OR_RETURN_ERROR( + storage_fault(StorageOperation::BeforeCreateDirectory) == 0, + AccessFailed, + "Cannot begin Core AI directory creation"); + const int made = retry_eintr( + [&] { return mkdirat(parent_fd, name.fileSystemRepresentation, 0777); }); + ET_CHECK_OR_RETURN_ERROR( + made == 0 || errno == EEXIST, + AccessFailed, + "Cannot create Core AI storage directory"); + created = made == 0; + fd = open_directory(); + } + if (fd < 0 && errno == ENOENT && !create) + return StorageDirectory{-1, false}; + ET_CHECK_OR_RETURN_ERROR( + fd >= 0, AccessFailed, "Cannot open Core AI storage directory"); + return StorageDirectory{fd, created}; +} + Result*> storage_children(int fd, bool skip_invalid_names) { // Reopening avoids sharing the caller's directory offset. FileDescriptor copy(retry_eintr( diff --git a/backends/apple/coreai/runtime/test/coreai_storage_test.mm b/backends/apple/coreai/runtime/test/coreai_storage_test.mm index 77b23eef53f..dd38d9386ae 100644 --- a/backends/apple/coreai/runtime/test/coreai_storage_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_storage_test.mm @@ -54,6 +54,172 @@ EXPECT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); } +TEST(CoreAIStorageTest, OpensDirectoriesAndReportsCreation) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + auto missing = open_storage_directory(root.get(), @"child", false); + ASSERT_TRUE(missing.ok()); + EXPECT_EQ(missing.get().fd, -1); + EXPECT_FALSE(missing.get().created); + + StorageFaultScope sync_fault(StorageOperation::SyncDirectory); + auto created = open_storage_directory(root.get(), @"child", true); + ASSERT_TRUE(created.ok()); + FileDescriptor first(created.get().fd); + ASSERT_GE(first.get(), 0); + EXPECT_TRUE(created.get().created); + EXPECT_NE(fcntl(first.get(), F_GETFD) & FD_CLOEXEC, 0); + EXPECT_EQ(sync_fault.matches, 0); + struct stat original; + ASSERT_EQ(fstat(first.get(), &original), 0); + EXPECT_TRUE(S_ISDIR(original.st_mode)); + + StorageFaultScope create_fault(StorageOperation::BeforeCreateDirectory); + for (bool create : {false, true}) { + auto reopened = open_storage_directory(root.get(), @"child", create); + ASSERT_TRUE(reopened.ok()); + FileDescriptor next(reopened.get().fd); + ASSERT_GE(next.get(), 0); + EXPECT_FALSE(reopened.get().created); + struct stat current; + ASSERT_EQ(fstat(next.get(), ¤t), 0); + EXPECT_EQ(current.st_ino, original.st_ino); + } + EXPECT_EQ(create_fault.matches, 0); +} + +TEST(CoreAIStorageTest, ConcurrentDirectoryCreationIsNotReportedAsOwnCreation) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSURL* child = [storage.url URLByAppendingPathComponent:@"child"]; + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + fault.armed = false; + fault.observe = ^(StorageOperation operation) { + if (operation == StorageOperation::BeforeCreateDirectory) + EXPECT_EQ(mkdir(child.fileSystemRepresentation, 0777), 0); + }; + auto opened = open_storage_directory(root.get(), @"child", true); + ASSERT_TRUE(opened.ok()); + FileDescriptor directory(opened.get().fd); + EXPECT_GE(directory.get(), 0); + EXPECT_FALSE(opened.get().created); +} + +TEST(CoreAIStorageTest, RejectsInvalidDirectoryComponents) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + EXPECT_EQ( + open_storage_directory(root.get(), nil, false).error(), + Error::InvalidArgument); + for (NSString* name in @[ @"", @".", @"..", @"nested/child" ]) { + EXPECT_EQ( + open_storage_directory(root.get(), name, false).error(), + Error::InvalidArgument); + } +} + +TEST(CoreAIStorageTest, SkipsCreationForExistingRoots) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + EXPECT_TRUE(prepare_storage_root(root.path, true).ok()); + EXPECT_TRUE(prepare_storage_root(root.path, false).ok()); + EXPECT_EQ(fault.matches, 0); + EXPECT_TRUE(backup_excluded(root)); + + NSURL* missing = [storage.url URLByAppendingPathComponent:@"missing"]; + EXPECT_EQ( + prepare_storage_root(missing.path, false).error(), + Error::InvalidArgument); + EXPECT_EQ(fault.matches, 0); + EXPECT_EQ( + prepare_storage_root(missing.path, true).error(), Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:missing.path]); + EXPECT_TRUE(prepare_storage_root(missing.path, true).ok()); +} + +TEST(CoreAIStorageTest, CreatesDirectoriesWithDefaultPermissions) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* reference = [storage.url URLByAppendingPathComponent:@"reference"]; + ASSERT_EQ(mkdir(reference.fileSystemRepresentation, 0777), 0); + struct stat expected; + ASSERT_EQ(stat(reference.fileSystemRepresentation, &expected), 0); + NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); + FileDescriptor directory(open( + root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(directory.get(), 0); + ASSERT_EQ( + write_storage_file(directory.get(), @"nested/file", "x", 1), Error::Ok); + for (NSURL* entry in @[ root, [root URLByAppendingPathComponent:@"nested"] ]) { + struct stat actual; + ASSERT_EQ(stat(entry.fileSystemRepresentation, &actual), 0); + EXPECT_EQ(actual.st_mode & 0777, expected.st_mode & 0777); + } +} + +TEST(CoreAIStorageTest, RecreatesRemovedRoot) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); + ASSERT_EQ(rmdir(root.fileSystemRepresentation), 0); + ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); + EXPECT_TRUE(backup_excluded(root)); +} + +TEST(CoreAIStorageTest, NewRootAndDescendantsUseFirstUnlockProtection) { + NSFileManager* manager = NSFileManager.defaultManager; + NSURL* root = [[NSURL fileURLWithPath:NSTemporaryDirectory() isDirectory:YES] + URLByAppendingPathComponent:NSUUID.UUID.UUIDString + isDirectory:YES]; + auto prepared = prepare_storage_root(root.path, true); + EXPECT_TRUE(prepared.ok()); + if (prepared.ok()) { + FileDescriptor directory(open( + root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + EXPECT_GE(directory.get(), 0); + if (directory.get() >= 0) { + EXPECT_EQ( + write_storage_file(directory.get(), @"nested/model.bin", "x", 1), + Error::Ok); + for (NSURL* entry in @[ + root, + [root URLByAppendingPathComponent:@"nested"], + [root URLByAppendingPathComponent:@"nested/model.bin"] + ]) { + NSDictionary* attributes = [manager attributesOfItemAtPath:entry.path + error:nil]; + EXPECT_TRUE([attributes[NSFileProtectionKey] + isEqual:NSFileProtectionCompleteUntilFirstUserAuthentication]); + } + } + EXPECT_TRUE([manager + setAttributes:@{NSFileProtectionKey : NSFileProtectionComplete} + ofItemAtPath:root.path + error:nil]); + EXPECT_TRUE(prepare_storage_root(root.path, true).ok()); + NSDictionary* existing = [manager attributesOfItemAtPath:root.path + error:nil]; + EXPECT_TRUE( + [existing[NSFileProtectionKey] isEqual:NSFileProtectionComplete]); + EXPECT_TRUE([manager removeItemAtURL:root error:nil]); + } +} + TEST(CoreAIStorageTest, RejectsInvalidRootsConsistently) { TestDirectory storage; ASSERT_NE(storage.url, nil); @@ -164,6 +330,75 @@ FileDescriptor root(open( EXPECT_EQ(empty.length, 0); } +TEST(CoreAIStorageTest, ReusesSourceParentsWithoutCreation) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + ASSERT_EQ( + write_storage_file(root.get(), @"nested/parents/first", "x", 1), + Error::Ok); + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + EXPECT_EQ( + write_storage_file(root.get(), @"nested/parents/second", "x", 1), + Error::Ok); + EXPECT_EQ(fault.matches, 0); + EXPECT_EQ( + write_storage_file(root.get(), @"missing/file", "x", 1), + Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + EXPECT_EQ(write_storage_file(root.get(), @"missing/file", "x", 1), Error::Ok); +} + +TEST(CoreAIStorageTest, AcceptsConcurrentSourceParentCreation) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSURL* nested = [storage.url URLByAppendingPathComponent:@"nested"]; + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + fault.armed = false; + __block int creations = 0; + fault.observe = ^(StorageOperation operation) { + if (operation == StorageOperation::BeforeCreateDirectory) { + ++creations; + EXPECT_EQ(mkdir(nested.fileSystemRepresentation, 0700), 0); + } + }; + EXPECT_EQ(write_storage_file(root.get(), @"nested/file", "x", 1), Error::Ok); + EXPECT_EQ(creations, 1); +} + +TEST(CoreAIStorageTest, RejectsUnsafeSourceParentsWithoutCreation) { + TestDirectory storage; + TestDirectory target; + ASSERT_NE(storage.url, nil); + ASSERT_NE(target.url, nil); + FileDescriptor root(open( + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(root.get(), 0); + NSURL* link = [storage.url URLByAppendingPathComponent:@"link"]; + ASSERT_EQ( + symlink( + target.url.fileSystemRepresentation, link.fileSystemRepresentation), + 0); + NSURL* regular = [storage.url URLByAppendingPathComponent:@"regular"]; + ASSERT_TRUE([[NSData data] writeToURL:regular atomically:NO]); + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + for (NSString* path in @[ @"link/file", @"regular/file" ]) { + EXPECT_EQ( + write_storage_file(root.get(), path, "x", 1), Error::AccessFailed); + } + EXPECT_EQ(fault.matches, 0); + EXPECT_FALSE([NSFileManager.defaultManager + fileExistsAtPath:[target.url URLByAppendingPathComponent:@"file"].path]); +} + TEST(CoreAIStorageTest, RemovesFailedWritesAndAllowsRetry) { const struct { StorageOperation operation; @@ -221,7 +456,9 @@ FileDescriptor root(open( EXPECT_EQ( operations, (std::vector{ + StorageOperation::BeforeCreateDirectory, StorageOperation::SyncDirectory, + StorageOperation::BeforeCreateDirectory, StorageOperation::SyncDirectory, StorageOperation::Write, StorageOperation::SyncFile, From d1eb228cd0c6e0ca7f0da469ce6374c7dfaa9507 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Thu, 29 Oct 2026 13:31:16 -0700 Subject: [PATCH 6/6] [CoreAI] Separate cache-root policy and filesystem primitives Extract root policy and fault injection, name creation/enumeration/sync policies, and share no-follow descriptor and component-validation helpers. Preserve storage publication, synchronization, and failure semantics. Split protection preservation from creation/inheritance testing. Create the preexisting protected fixture directly instead of changing the protection class of a populated cache, the setup operation rejected by hosted CI. Retain protection assertions and surface NSError diagnostics. Hosted rerun remains pending; no production protection behavior changed. Test Plan: Native baseline 60/60 passed; updated suite 62/62 passed including new no-follow helper coverage. CTest, clang-format 18.1.3 across all CoreAI Objective-C++ sources, scoped lint and diff whitespace checks passed. Generic Buck owner validation remains unavailable for this checkout. --- backends/apple/coreai/CMakeLists.txt | 8 +- backends/apple/coreai/README.md | 6 + .../apple/coreai/runtime/coreai_cache_root.h | 25 ++ .../apple/coreai/runtime/coreai_cache_root.mm | 103 ++++++ .../coreai/runtime/coreai_fault_injection.h | 34 ++ .../coreai/runtime/coreai_fault_injection.mm | 24 ++ backends/apple/coreai/runtime/coreai_file.h | 22 ++ .../apple/coreai/runtime/coreai_storage.h | 45 +-- .../apple/coreai/runtime/coreai_storage.mm | 162 +++------- .../apple/coreai/runtime/coreai_validation.h | 3 + .../apple/coreai/runtime/coreai_validation.mm | 5 + .../runtime/test/coreai_cache_root_test.mm | 207 ++++++++++++ .../coreai/runtime/test/coreai_fault_scope.h | 21 +- .../runtime/test/coreai_storage_test.mm | 306 ++++++------------ 14 files changed, 583 insertions(+), 388 deletions(-) create mode 100644 backends/apple/coreai/runtime/coreai_cache_root.h create mode 100644 backends/apple/coreai/runtime/coreai_cache_root.mm create mode 100644 backends/apple/coreai/runtime/coreai_fault_injection.h create mode 100644 backends/apple/coreai/runtime/coreai_fault_injection.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_cache_root_test.mm diff --git a/backends/apple/coreai/CMakeLists.txt b/backends/apple/coreai/CMakeLists.txt index 85d742adf3d..bb63606a1a3 100644 --- a/backends/apple/coreai/CMakeLists.txt +++ b/backends/apple/coreai/CMakeLists.txt @@ -10,8 +10,10 @@ endif() enable_language(OBJCXX) -set(_coreai_runtime_sources runtime/coreai_manifest.mm - runtime/coreai_validation.mm runtime/coreai_storage.mm +set(_coreai_runtime_sources + runtime/coreai_cache_root.mm runtime/coreai_fault_injection.mm + runtime/coreai_manifest.mm runtime/coreai_validation.mm + runtime/coreai_storage.mm ) function(coreai_configure_objc_target target) @@ -31,7 +33,7 @@ if(EXECUTORCH_BUILD_TESTS AND CMAKE_SYSTEM_NAME STREQUAL "Darwin") add_executable( coreai_host_test runtime/test/coreai_host_test.mm runtime/test/coreai_manifest_test.mm - runtime/test/coreai_storage_test.mm + runtime/test/coreai_cache_root_test.mm runtime/test/coreai_storage_test.mm runtime/test/coreai_filesystem_fixture.mm ${_coreai_runtime_sources} ) coreai_configure_objc_target(coreai_host_test) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 2ca9d0b8665..1a3d96adc7a 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -76,6 +76,12 @@ Directory creation uses default permissions filtered by the process umask, witho an explicit owner-only restriction. Existing permissions are not changed. Backup exclusion and required durability flushes still apply on reused paths. +Cache-root preparation owns location and backup/protection policy; descriptor +storage owns no-follow operations, publication, and synchronization. Creation +and enumeration use explicit policies, with a missing child distinguishable +from an error. Fault injection is a separate, thread-local test interface and +compiles away in production. + ## Host Tests On macOS, `EXECUTORCH_BUILD_COREAI=ON` with `EXECUTORCH_BUILD_TESTS=ON` diff --git a/backends/apple/coreai/runtime/coreai_cache_root.h b/backends/apple/coreai/runtime/coreai_cache_root.h new file mode 100644 index 00000000000..ee536e241fc --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_cache_root.h @@ -0,0 +1,25 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#import +#include +#include "coreai_file.h" + +namespace executorch::backends::coreai { + +// Resolves the default location without creating or modifying storage. +runtime::Result default_coreai_assets_root(); +// Requires an absolute directory path. With Create, makes the directory and +// excludes it from backup; otherwise a missing directory is InvalidArgument. +runtime::Result prepare_storage_root( + NSString* path, + StorageCreationMode mode); + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_cache_root.mm b/backends/apple/coreai/runtime/coreai_cache_root.mm new file mode 100644 index 00000000000..18ca1b6f198 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_cache_root.mm @@ -0,0 +1,103 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "coreai_cache_root.h" +#include "coreai_fault_injection.h" +#include "coreai_file.h" + +#include +#include + +namespace executorch::backends::coreai { +namespace { +using runtime::Error; +using runtime::Result; + +Error ensure_excluded_from_backup(NSURL* directory) { + NSURL* fresh = [NSURL fileURLWithPath:directory.path isDirectory:YES]; + NSNumber* excluded = nil; + NSError* error = nil; + ET_CHECK_OR_RETURN_ERROR( + [fresh getResourceValue:&excluded + forKey:NSURLIsExcludedFromBackupKey + error:&error], + AccessFailed, + "Cannot read Core AI backup exclusion: %s", + error.localizedDescription.UTF8String); + if (excluded.boolValue) { + return Error::Ok; + } + ET_CHECK_OR_RETURN_ERROR( + [fresh setResourceValue:@YES + forKey:NSURLIsExcludedFromBackupKey + error:&error], + AccessFailed, + "Cannot exclude Core AI storage from backup: %s", + error.localizedDescription.UTF8String); + return Error::Ok; +} + +} // namespace + +Result default_coreai_assets_root() { + NSURL* caches = + [NSFileManager.defaultManager URLsForDirectory:NSCachesDirectory + inDomains:NSUserDomainMask] + .firstObject; + ET_CHECK_OR_RETURN_ERROR( + caches.isFileURL, AccessFailed, "Cannot resolve Core AI cache directory"); + return [caches.path stringByAppendingPathComponent:@"executorch_coreai"]; +} + +Result prepare_storage_root( + NSString* path, + StorageCreationMode mode) { + const bool create = mode == StorageCreationMode::Create; + ET_CHECK_OR_RETURN_ERROR( + [path isKindOfClass:NSString.class] && [path hasPrefix:@"/"] && + [path rangeOfString:@"\0"].location == NSNotFound, + InvalidArgument, + "Core AI storage requires an absolute path"); + struct stat info; + const int result = + retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); + ET_CHECK_OR_RETURN_ERROR( + result == 0 || errno == ENOENT || errno == ENOTDIR, + AccessFailed, + "Cannot inspect Core AI storage directory"); + ET_CHECK_OR_RETURN_ERROR( + result == 0 ? S_ISDIR(info.st_mode) : create && errno == ENOENT, + InvalidArgument, + "Core AI storage must be a directory"); + if (create && result != 0) { + NSDictionary* attributes = @{ + NSFileProtectionKey : + NSFileProtectionCompleteUntilFirstUserAuthentication, + }; + ET_CHECK_OR_RETURN_ERROR( + fault_point(FaultPoint::BeforeCreateDirectory) == 0, + AccessFailed, + "Cannot begin Core AI directory creation"); + NSError* error = nil; + ET_CHECK_OR_RETURN_ERROR( + [NSFileManager.defaultManager createDirectoryAtPath:path + withIntermediateDirectories:YES + attributes:attributes + error:&error], + AccessFailed, + "Cannot create Core AI storage directory: %s", + error.localizedDescription.UTF8String); + } + if (create) { + ET_CHECK_OK_OR_RETURN_ERROR(ensure_excluded_from_backup( + [NSURL fileURLWithPath:path isDirectory:YES])); + } + return path; +} + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_fault_injection.h b/backends/apple/coreai/runtime/coreai_fault_injection.h new file mode 100644 index 00000000000..cba60c5dfb8 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_fault_injection.h @@ -0,0 +1,34 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +namespace executorch::backends::coreai { + +enum class FaultPoint { + Write, + SyncFile, + SyncDirectory, + FullSync, + Rename, + BeforeCreateDirectory, +}; + +#if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING +int fault_point(FaultPoint point); +namespace testing { +using FaultCallback = int (*)(FaultPoint); +extern thread_local FaultCallback fault_callback; +} // namespace testing +#else +constexpr int fault_point(FaultPoint) { + return 0; +} +#endif + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_fault_injection.mm b/backends/apple/coreai/runtime/coreai_fault_injection.mm new file mode 100644 index 00000000000..7bbcc0a316a --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_fault_injection.mm @@ -0,0 +1,24 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "coreai_fault_injection.h" + +namespace executorch::backends::coreai { + +#if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING +namespace testing { +thread_local FaultCallback fault_callback = nullptr; +} + +int fault_point(FaultPoint point) { + return testing::fault_callback == nullptr ? 0 + : testing::fault_callback(point); +} +#endif + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_file.h b/backends/apple/coreai/runtime/coreai_file.h index 478f03a7b91..cbc991a5fae 100644 --- a/backends/apple/coreai/runtime/coreai_file.h +++ b/backends/apple/coreai/runtime/coreai_file.h @@ -8,11 +8,18 @@ #pragma once +#include +#include #include #include namespace executorch::backends::coreai { +enum class StorageCreationMode { + OpenExisting, + Create, +}; + class FileDescriptor { public: explicit FileDescriptor(int fd) : fd_(fd) {} @@ -50,4 +57,19 @@ auto retry_eintr(F call) { return result; } +// Opens a child directory without following symlinks. Returns -1 with errno +// set, including ENOENT when the child is missing. +inline int open_directory_at(int parent_fd, const char* name) { + return retry_eintr([&] { + return openat( + parent_fd, name, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + }); +} + +// Reads type and permission metadata for a child without following symlinks. +inline int lstat_at(int parent_fd, const char* name, struct stat* info) { + return retry_eintr( + [&] { return fstatat(parent_fd, name, info, AT_SYMLINK_NOFOLLOW); }); +} + } // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_storage.h b/backends/apple/coreai/runtime/coreai_storage.h index d807eb523de..5bcfecfa4f5 100644 --- a/backends/apple/coreai/runtime/coreai_storage.h +++ b/backends/apple/coreai/runtime/coreai_storage.h @@ -11,25 +11,20 @@ #import #include #include +#include "coreai_file.h" namespace executorch::backends::coreai { -// Resolves the default location without creating or modifying storage. -runtime::Result default_coreai_assets_root(); -// Requires an absolute directory path. With create, makes the directory and -// excludes it from backup; otherwise a missing directory is InvalidArgument. -runtime::Result prepare_storage_root(NSString* path, bool create); -runtime::Error ensure_excluded_from_backup(NSURL* directory); - runtime::Error sync_storage_directory(int fd); struct StorageDirectory { int fd; bool created; }; // Opens one child component without following symlinks. The caller owns fd; -// missing with create=false returns {-1, false}. Does not flush directories. +// a missing child with OpenExisting returns {-1, false}. Does not flush +// directories. runtime::Result -open_storage_directory(int parent_fd, NSString* name, bool create); +open_storage_directory(int parent_fd, NSString* name, StorageCreationMode mode); // Returns an independently owned descriptor for a stable coordination file. int open_storage_shared_file(int parent_fd, const char* name); // Creates a new file and missing parents, then syncs the file and its parent. @@ -42,34 +37,14 @@ runtime::Error write_storage_file( // Atomically replaces name with data and fully flushes the file and renamed // entry. On error, readers see either the old or the new complete bytes. runtime::Error publish_storage_data(int root_fd, NSString* name, NSData* data); + +enum class StorageNamePolicy { + Strict, + SkipInvalid, +}; // Lists sorted names without changing fd's enumeration offset. runtime::Result*> storage_children( int fd, - bool skip_invalid_names = false); - -enum class StorageOperation { - Write, - SyncFile, - SyncDirectory, - FullSync, - Rename, - BeforeCreateDirectory, - BeforeSDK, - AfterSDK, - BeforeEvict, - AfterEvict, - Remove, -}; -#if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING -int storage_fault(StorageOperation operation); -namespace testing { -using StorageFaultCallback = int (*)(StorageOperation); -extern thread_local StorageFaultCallback storage_fault_callback; -} // namespace testing -#else -constexpr int storage_fault(StorageOperation) { - return 0; -} -#endif + StorageNamePolicy policy = StorageNamePolicy::Strict); } // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_storage.mm b/backends/apple/coreai/runtime/coreai_storage.mm index 550ffaaf1ea..066c60e17ba 100644 --- a/backends/apple/coreai/runtime/coreai_storage.mm +++ b/backends/apple/coreai/runtime/coreai_storage.mm @@ -7,7 +7,9 @@ */ #import "coreai_storage.h" +#include "coreai_fault_injection.h" #include "coreai_file.h" +#include "coreai_validation.h" #include #include @@ -22,102 +24,11 @@ namespace { using runtime::Error; using runtime::Result; -} // namespace - -Error ensure_excluded_from_backup(NSURL* directory) { - NSURL* fresh = [NSURL fileURLWithPath:directory.path isDirectory:YES]; - NSNumber* excluded = nil; - NSError* error = nil; - ET_CHECK_OR_RETURN_ERROR( - [fresh getResourceValue:&excluded - forKey:NSURLIsExcludedFromBackupKey - error:&error], - AccessFailed, - "Cannot read Core AI backup exclusion: %s", - error.localizedDescription.UTF8String); - if (excluded.boolValue) { - return Error::Ok; - } - ET_CHECK_OR_RETURN_ERROR( - [fresh setResourceValue:@YES - forKey:NSURLIsExcludedFromBackupKey - error:&error], - AccessFailed, - "Cannot exclude Core AI storage from backup: %s", - error.localizedDescription.UTF8String); - return Error::Ok; -} - -Result default_coreai_assets_root() { - NSURL* caches = - [NSFileManager.defaultManager URLsForDirectory:NSCachesDirectory - inDomains:NSUserDomainMask] - .firstObject; - ET_CHECK_OR_RETURN_ERROR( - caches.isFileURL, AccessFailed, "Cannot resolve Core AI cache directory"); - return [caches.path stringByAppendingPathComponent:@"executorch_coreai"]; -} - -Result prepare_storage_root(NSString* path, bool create) { - ET_CHECK_OR_RETURN_ERROR( - [path isKindOfClass:NSString.class] && [path hasPrefix:@"/"] && - [path rangeOfString:@"\0"].location == NSNotFound, - InvalidArgument, - "Core AI storage requires an absolute path"); - struct stat info; - const int result = - retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); - ET_CHECK_OR_RETURN_ERROR( - result == 0 || errno == ENOENT || errno == ENOTDIR, - AccessFailed, - "Cannot inspect Core AI storage directory"); - ET_CHECK_OR_RETURN_ERROR( - result == 0 ? S_ISDIR(info.st_mode) : create && errno == ENOENT, - InvalidArgument, - "Core AI storage must be a directory"); - if (create && result != 0) { - NSDictionary* attributes = @{ - NSFileProtectionKey : - NSFileProtectionCompleteUntilFirstUserAuthentication, - }; - ET_CHECK_OR_RETURN_ERROR( - storage_fault(StorageOperation::BeforeCreateDirectory) == 0, - AccessFailed, - "Cannot begin Core AI directory creation"); - NSError* error = nil; - ET_CHECK_OR_RETURN_ERROR( - [NSFileManager.defaultManager createDirectoryAtPath:path - withIntermediateDirectories:YES - attributes:attributes - error:&error], - AccessFailed, - "Cannot create Core AI storage directory: %s", - error.localizedDescription.UTF8String); - } - if (create) { - ET_CHECK_OK_OR_RETURN_ERROR(ensure_excluded_from_backup( - [NSURL fileURLWithPath:path isDirectory:YES])); - } - return path; -} -#if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING -namespace testing { -thread_local StorageFaultCallback storage_fault_callback = nullptr; -} - -int storage_fault(StorageOperation operation) { - return testing::storage_fault_callback == nullptr - ? 0 - : testing::storage_fault_callback(operation); -} -#endif - -namespace { template -auto storage_call(StorageOperation operation, F call) { +auto storage_call(FaultPoint point, F call) { return retry_eintr([&]() -> decltype(call()) { - const int fault = storage_fault(operation); + const int fault = fault_point(point); if (fault != 0) { errno = fault; return -1; @@ -126,18 +37,17 @@ auto storage_call(StorageOperation operation, F call) { }); } -bool storage_component(NSString* name) { - return [name isKindOfClass:NSString.class] && name.length > 0 && - ![name isEqualToString:@"."] && ![name isEqualToString:@".."] && - ![name containsString:@"/"] && ![name containsString:@"\0"]; -} +enum class StorageSyncMode { + Normal, + Full, +}; Error write_file( int root_fd, NSString* relative_path, const void* data, size_t size, - bool full_sync) { + StorageSyncMode sync_mode) { NSArray* components = [relative_path componentsSeparatedByString:@"/"]; ET_CHECK_OR_RETURN_ERROR( @@ -153,7 +63,8 @@ FileDescriptor parent( ET_CHECK_OR_RETURN_ERROR( parent.get() >= 0, AccessFailed, "Cannot open storage parent"); for (NSUInteger i = 0; i + 1 < components.count; ++i) { - auto opened = open_storage_directory(parent.get(), components[i], true); + auto opened = open_storage_directory( + parent.get(), components[i], StorageCreationMode::Create); if (!opened.ok()) return opened.error(); FileDescriptor next(opened.get().fd); @@ -172,7 +83,7 @@ FileDescriptor file(retry_eintr([&] { const auto result = [&]() -> Error { const auto* bytes = static_cast(data); while (size > 0) { - const ssize_t count = storage_call(StorageOperation::Write, [&] { + const ssize_t count = storage_call(FaultPoint::Write, [&] { return write(file.get(), bytes, std::min(size, size_t(1024 * 1024))); }); ET_CHECK_OR_RETURN_ERROR( @@ -181,12 +92,11 @@ FileDescriptor file(retry_eintr([&] { size -= count; } ET_CHECK_OR_RETURN_ERROR( - storage_call( - StorageOperation::SyncFile, [&] { return fsync(file.get()); }) == + storage_call(FaultPoint::SyncFile, [&] { return fsync(file.get()); }) == 0 && - (!full_sync || + (sync_mode != StorageSyncMode::Full || storage_call( - StorageOperation::FullSync, + FaultPoint::FullSync, [&] { return fcntl(file.get(), F_FULLFSYNC); }) == 0), AccessFailed, "Cannot synchronize Core AI file"); @@ -211,27 +121,28 @@ FileDescriptor file(retry_eintr([&] { } } // namespace -Result -open_storage_directory(int parent_fd, NSString* name, bool create) { +Result open_storage_directory( + int parent_fd, + NSString* name, + StorageCreationMode mode) { ET_CHECK_OR_RETURN_ERROR( - storage_component(name), InvalidArgument, "Invalid storage directory name"); + storage_component(name), + InvalidArgument, + "Invalid storage directory name"); const auto open_directory = [&] { - return retry_eintr([&] { - return openat( - parent_fd, - name.fileSystemRepresentation, - O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - }); + return open_directory_at(parent_fd, name.fileSystemRepresentation); }; + const bool create = mode == StorageCreationMode::Create; int fd = open_directory(); bool created = false; if (fd < 0 && errno == ENOENT && create) { ET_CHECK_OR_RETURN_ERROR( - storage_fault(StorageOperation::BeforeCreateDirectory) == 0, + fault_point(FaultPoint::BeforeCreateDirectory) == 0, AccessFailed, "Cannot begin Core AI directory creation"); - const int made = retry_eintr( - [&] { return mkdirat(parent_fd, name.fileSystemRepresentation, 0777); }); + const int made = retry_eintr([&] { + return mkdirat(parent_fd, name.fileSystemRepresentation, 0777); + }); ET_CHECK_OR_RETURN_ERROR( made == 0 || errno == EEXIST, AccessFailed, @@ -246,7 +157,7 @@ FileDescriptor file(retry_eintr([&] { return StorageDirectory{fd, created}; } -Result*> storage_children(int fd, bool skip_invalid_names) { +Result*> storage_children(int fd, StorageNamePolicy policy) { // Reopening avoids sharing the caller's directory offset. FileDescriptor copy(retry_eintr( [&] { return openat(fd, ".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); })); @@ -277,7 +188,7 @@ FileDescriptor copy(retry_eintr( length:strlen(entry->d_name) encoding:NSUTF8StringEncoding]; if (name == nil) { - if (skip_invalid_names) { + if (policy == StorageNamePolicy::SkipInvalid) { continue; } result = Error::InvalidExternalData; @@ -311,8 +222,7 @@ int open_storage_shared_file(int parent_fd, const char* name) { Error sync_storage_directory(int fd) { ET_CHECK_OR_RETURN_ERROR( - storage_call( - StorageOperation::SyncDirectory, [&] { return fsync(fd); }) == 0, + storage_call(FaultPoint::SyncDirectory, [&] { return fsync(fd); }) == 0, AccessFailed, "Cannot synchronize Core AI directory"); return Error::Ok; @@ -323,7 +233,8 @@ Error write_storage_file( NSString* relative_path, const void* data, size_t size) { - return write_file(root_fd, relative_path, data, size, false); + return write_file( + root_fd, relative_path, data, size, StorageSyncMode::Normal); } Error publish_storage_data(int root_fd, NSString* name, NSData* data) { @@ -334,8 +245,9 @@ Error publish_storage_data(int root_fd, NSString* name, NSData* data) { NSString* temporary = [@".tmp-" stringByAppendingString:NSUUID.UUID.UUIDString]; // Bookmarks are small and costly to lose, so they get a full device flush. - auto result = write_file(root_fd, temporary, data.bytes, data.length, true); - if (result == Error::Ok && storage_call(StorageOperation::Rename, [&] { + auto result = write_file( + root_fd, temporary, data.bytes, data.length, StorageSyncMode::Full); + if (result == Error::Ok && storage_call(FaultPoint::Rename, [&] { return renameat( root_fd, temporary.fileSystemRepresentation, @@ -349,7 +261,7 @@ Error publish_storage_data(int root_fd, NSString* name, NSData* data) { // Flush the renamed entry as well as the file's pre-rename contents. ET_CHECK_OR_RETURN_ERROR( storage_call( - StorageOperation::FullSync, + FaultPoint::FullSync, [&] { return fcntl(root_fd, F_FULLFSYNC); }) == 0, AccessFailed, "Cannot fully synchronize Core AI publication directory"); diff --git a/backends/apple/coreai/runtime/coreai_validation.h b/backends/apple/coreai/runtime/coreai_validation.h index a3d060857dc..ea21885b19d 100644 --- a/backends/apple/coreai/runtime/coreai_validation.h +++ b/backends/apple/coreai/runtime/coreai_validation.h @@ -16,6 +16,9 @@ namespace executorch::backends::coreai { bool nonempty_string(id value); // Relative bundle path without escapes, backslashes, or empty components. bool relative_path(NSString* path); +// Single path component for an fd-relative open: no slashes, ".", or "..". +// Unlike relative_path, backslashes are ordinary bytes on this filesystem. +bool storage_component(NSString* name); // Lowercase hex of any positive length, as in the exporter model hash. bool lowercase_hex(NSString* value); // Lowercase hex of exactly length characters, as in bundle digests. diff --git a/backends/apple/coreai/runtime/coreai_validation.mm b/backends/apple/coreai/runtime/coreai_validation.mm index ea8b018f38f..54c611b81bc 100644 --- a/backends/apple/coreai/runtime/coreai_validation.mm +++ b/backends/apple/coreai/runtime/coreai_validation.mm @@ -38,6 +38,11 @@ bool relative_path(NSString* path) { return true; } +bool storage_component(NSString* name) { + return nonempty_string(name) && ![name isEqualToString:@"."] && + ![name isEqualToString:@".."] && ![name containsString:@"/"]; +} + bool lowercase_hex(NSString* value) { return nonempty_string(value) && [value rangeOfCharacterFromSet:invalid_hex_characters()].location == diff --git a/backends/apple/coreai/runtime/test/coreai_cache_root_test.mm b/backends/apple/coreai/runtime/test/coreai_cache_root_test.mm new file mode 100644 index 00000000000..b6cf8141065 --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_cache_root_test.mm @@ -0,0 +1,207 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include +#include +#include "coreai_cache_root.h" +#include "coreai_fault_scope.h" +#include "coreai_file.h" +#include "coreai_filesystem_fixture.h" +#include "coreai_storage.h" + +using namespace executorch::runtime; +using namespace executorch::backends::coreai; +using namespace executorch::backends::coreai::testing; + +TEST(CoreAICacheRootTest, PreparesRootWithRootOnlyBackupPolicy) { + NSFileManager* manager = NSFileManager.defaultManager; + TestDirectory sandbox; + ASSERT_NE(sandbox.url, nil); + NSURL* parent = [sandbox.url URLByAppendingPathComponent:@"parent"]; + ASSERT_TRUE(([manager createDirectoryAtURL:parent + withIntermediateDirectories:NO + attributes:nil + error:nil])); + ASSERT_TRUE(set_backup_excluded(sandbox.url, false)); + ASSERT_TRUE(set_backup_excluded(parent, false)); + NSURL* root = [parent URLByAppendingPathComponent:@"nested/models"]; + auto prepared = prepare_storage_root(root.path, StorageCreationMode::Create); + ASSERT_TRUE((prepared.ok() && [prepared.get() isEqualToString:root.path])); + ASSERT_TRUE((backup_excluded(root))); + ASSERT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); + ASSERT_TRUE((!backup_excluded(root.URLByDeletingLastPathComponent))); + + ASSERT_TRUE(set_backup_excluded(root, false)); + auto inspected = + prepare_storage_root(root.path, StorageCreationMode::OpenExisting); + ASSERT_TRUE((inspected.ok() && [inspected.get() isEqualToString:root.path])); + ASSERT_TRUE((!backup_excluded(root))); + ASSERT_TRUE( + (prepare_storage_root(root.path, StorageCreationMode::Create).ok() && + backup_excluded(root))); + + NSURL* missing = [sandbox.url URLByAppendingPathComponent:@"missing"]; + ASSERT_EQ( + prepare_storage_root(missing.path, StorageCreationMode::OpenExisting) + .error(), + Error::InvalidArgument); + EXPECT_FALSE([manager fileExistsAtPath:missing.path]); + EXPECT_EQ( + prepare_storage_root(@"relative", StorageCreationMode::Create).error(), + Error::InvalidArgument); + EXPECT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); +} + +TEST(CoreAICacheRootTest, SkipsCreationForExistingRoots) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::Create).ok()); + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); + EXPECT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::Create).ok()); + EXPECT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::OpenExisting).ok()); + EXPECT_EQ(fault.matches, 0); + EXPECT_TRUE(backup_excluded(root)); + + NSURL* missing = [storage.url URLByAppendingPathComponent:@"missing"]; + EXPECT_EQ( + prepare_storage_root(missing.path, StorageCreationMode::OpenExisting) + .error(), + Error::InvalidArgument); + EXPECT_EQ(fault.matches, 0); + EXPECT_EQ( + prepare_storage_root(missing.path, StorageCreationMode::Create).error(), + Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:missing.path]); + EXPECT_TRUE( + prepare_storage_root(missing.path, StorageCreationMode::Create).ok()); +} + +TEST(CoreAICacheRootTest, CreatesDirectoriesWithDefaultPermissions) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* reference = [storage.url URLByAppendingPathComponent:@"reference"]; + ASSERT_EQ(mkdir(reference.fileSystemRepresentation, 0777), 0); + struct stat expected; + ASSERT_EQ(stat(reference.fileSystemRepresentation, &expected), 0); + NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::Create).ok()); + FileDescriptor directory( + open(root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + ASSERT_GE(directory.get(), 0); + ASSERT_EQ( + write_storage_file(directory.get(), @"nested/file", "x", 1), Error::Ok); + for (NSURL* entry in + @[ root, [root URLByAppendingPathComponent:@"nested"] ]) { + struct stat actual; + ASSERT_EQ(stat(entry.fileSystemRepresentation, &actual), 0); + EXPECT_EQ(actual.st_mode & 0777, expected.st_mode & 0777); + } +} + +TEST(CoreAICacheRootTest, RecreatesRemovedRoot) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::Create).ok()); + ASSERT_EQ(rmdir(root.fileSystemRepresentation), 0); + ASSERT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::Create).ok()); + EXPECT_TRUE(backup_excluded(root)); +} + +TEST(CoreAICacheRootTest, NewRootAndDescendantsUseFirstUnlockProtection) { + TestDirectory sandbox; + ASSERT_NE(sandbox.url, nil); + NSURL* root = [sandbox.url URLByAppendingPathComponent:@"cache"]; + ASSERT_TRUE( + prepare_storage_root(root.path, StorageCreationMode::Create).ok()); + FileDescriptor directory( + open_directory_at(AT_FDCWD, root.fileSystemRepresentation)); + ASSERT_GE(directory.get(), 0); + ASSERT_EQ( + write_storage_file(directory.get(), @"nested/model.bin", "x", 1), + Error::Ok); + for (NSURL* entry in @[ + root, + [root URLByAppendingPathComponent:@"nested"], + [root URLByAppendingPathComponent:@"nested/model.bin"] + ]) { + SCOPED_TRACE(entry.path.UTF8String); + NSError* error = nil; + NSDictionary* attributes = + [NSFileManager.defaultManager attributesOfItemAtPath:entry.path + error:&error]; + ASSERT_NE(attributes, nil) << error.description.UTF8String; + EXPECT_TRUE([attributes[NSFileProtectionKey] + isEqual:NSFileProtectionCompleteUntilFirstUserAuthentication]); + } +} + +TEST(CoreAICacheRootTest, PreservesExistingRootProtection) { + TestDirectory sandbox; + ASSERT_NE(sandbox.url, nil); + NSURL* root = [sandbox.url URLByAppendingPathComponent:@"existing"]; + NSFileManager* manager = NSFileManager.defaultManager; + NSError* error = nil; + // Set the fixture policy at creation, without reclassifying a populated + // cache. + ASSERT_TRUE([manager + createDirectoryAtURL:root + withIntermediateDirectories:NO + attributes:@{ + NSFileProtectionKey : NSFileProtectionComplete + } + error:&error]) + << error.description.UTF8String; + NSDictionary* before = [manager attributesOfItemAtPath:root.path + error:&error]; + ASSERT_NE(before, nil) << error.description.UTF8String; + ASSERT_TRUE([before[NSFileProtectionKey] isEqual:NSFileProtectionComplete]); + for (StorageCreationMode mode : + {StorageCreationMode::OpenExisting, StorageCreationMode::Create}) { + ASSERT_TRUE(prepare_storage_root(root.path, mode).ok()); + NSDictionary* after = [manager attributesOfItemAtPath:root.path + error:&error]; + ASSERT_NE(after, nil) << error.description.UTF8String; + EXPECT_TRUE([after[NSFileProtectionKey] isEqual:NSFileProtectionComplete]); + } +} + +TEST(CoreAICacheRootTest, RejectsInvalidRootsConsistently) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + NSURL* regular = [storage.url URLByAppendingPathComponent:@"regular"]; + NSData* bytes = [@"unchanged" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_TRUE([bytes writeToURL:regular atomically:NO]); + for (StorageCreationMode mode : + {StorageCreationMode::OpenExisting, StorageCreationMode::Create}) { + SCOPED_TRACE(static_cast(mode)); + EXPECT_EQ(prepare_storage_root(nil, mode).error(), Error::InvalidArgument); + for (NSString* path in @[ + @"", + @"relative", + @"~", + @"~/cache", + @"~nobody/cache", + regular.path, + [regular.path stringByAppendingPathComponent:@"child"] + ]) { + SCOPED_TRACE(path.UTF8String); + EXPECT_EQ( + prepare_storage_root(path, mode).error(), Error::InvalidArgument); + } + } + EXPECT_TRUE([[NSData dataWithContentsOfURL:regular] isEqual:bytes]); +} diff --git a/backends/apple/coreai/runtime/test/coreai_fault_scope.h b/backends/apple/coreai/runtime/test/coreai_fault_scope.h index e55de88579a..f60092cd91a 100644 --- a/backends/apple/coreai/runtime/test/coreai_fault_scope.h +++ b/backends/apple/coreai/runtime/test/coreai_fault_scope.h @@ -8,29 +8,28 @@ #pragma once +#import #include -#include "coreai_storage.h" +#include "coreai_fault_injection.h" namespace executorch::backends::coreai::testing { struct StorageFaultScope { - StorageOperation operation; + FaultPoint operation; int error = EIO; int match_index = 1; int matches = 0; int hits = 0; bool armed = true; - void (^observe)(StorageOperation) = nil; + void (^observe)(FaultPoint) = nil; - explicit StorageFaultScope(StorageOperation op) - : operation(op), - previous_(current_), - previous_callback_(storage_fault_callback) { + explicit StorageFaultScope(FaultPoint op) + : operation(op), previous_(current_), previous_callback_(fault_callback) { current_ = this; - storage_fault_callback = inject; + fault_callback = inject; } ~StorageFaultScope() { - storage_fault_callback = previous_callback_; + fault_callback = previous_callback_; current_ = previous_; } StorageFaultScope(const StorageFaultScope&) = delete; @@ -39,9 +38,9 @@ struct StorageFaultScope { private: inline static thread_local StorageFaultScope* current_ = nullptr; StorageFaultScope* previous_; - StorageFaultCallback previous_callback_; + FaultCallback previous_callback_; - static int inject(StorageOperation operation) { + static int inject(FaultPoint operation) { auto& scope = *current_; if (scope.observe != nil) scope.observe(operation); diff --git a/backends/apple/coreai/runtime/test/coreai_storage_test.mm b/backends/apple/coreai/runtime/test/coreai_storage_test.mm index dd38d9386ae..266dfbd34bf 100644 --- a/backends/apple/coreai/runtime/test/coreai_storage_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_storage_test.mm @@ -14,59 +14,49 @@ #include "coreai_fault_scope.h" #include "coreai_file.h" #include "coreai_filesystem_fixture.h" +#include "coreai_storage.h" using namespace executorch::runtime; using namespace executorch::backends::coreai; using namespace executorch::backends::coreai::testing; -TEST(CoreAIStorageTest, PreparesRootWithRootOnlyBackupPolicy) { - NSFileManager* manager = NSFileManager.defaultManager; - TestDirectory sandbox; - ASSERT_NE(sandbox.url, nil); - NSURL* parent = [sandbox.url URLByAppendingPathComponent:@"parent"]; - ASSERT_TRUE(([manager createDirectoryAtURL:parent - withIntermediateDirectories:NO - attributes:nil - error:nil])); - ASSERT_TRUE(set_backup_excluded(sandbox.url, false)); - ASSERT_TRUE(set_backup_excluded(parent, false)); - NSURL* root = [parent URLByAppendingPathComponent:@"nested/models"]; - auto prepared = prepare_storage_root(root.path, true); - ASSERT_TRUE((prepared.ok() && [prepared.get() isEqualToString:root.path])); - ASSERT_TRUE((backup_excluded(root))); - ASSERT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); - ASSERT_TRUE((!backup_excluded(root.URLByDeletingLastPathComponent))); - - ASSERT_TRUE(set_backup_excluded(root, false)); - auto inspected = prepare_storage_root(root.path, false); - ASSERT_TRUE((inspected.ok() && [inspected.get() isEqualToString:root.path])); - ASSERT_TRUE((!backup_excluded(root))); - ASSERT_TRUE( - (prepare_storage_root(root.path, true).ok() && backup_excluded(root))); - - NSURL* missing = [sandbox.url URLByAppendingPathComponent:@"missing"]; - ASSERT_EQ( - prepare_storage_root(missing.path, false).error(), - Error::InvalidArgument); - EXPECT_FALSE([manager fileExistsAtPath:missing.path]); - EXPECT_EQ( - prepare_storage_root(@"relative", true).error(), Error::InvalidArgument); - EXPECT_TRUE((!backup_excluded(parent) && !backup_excluded(sandbox.url))); +TEST(CoreAIStorageTest, DirectoryHelpersDoNotFollowSymlinks) { + TestDirectory storage; + ASSERT_NE(storage.url, nil); + FileDescriptor root( + open_directory_at(AT_FDCWD, storage.url.fileSystemRepresentation)); + ASSERT_GE(root.get(), 0); + EXPECT_NE(fcntl(root.get(), F_GETFD) & FD_CLOEXEC, 0); + ASSERT_EQ(mkdirat(root.get(), "directory", 0700), 0); + ASSERT_EQ(symlinkat("directory", root.get(), "link"), 0); + struct stat info; + ASSERT_EQ(lstat_at(root.get(), "link", &info), 0); + EXPECT_TRUE(S_ISLNK(info.st_mode)); + ASSERT_EQ(lstat_at(root.get(), "directory", &info), 0); + EXPECT_TRUE(S_ISDIR(info.st_mode)); + EXPECT_EQ(open_directory_at(root.get(), "link"), -1); + EXPECT_EQ(open_directory_at(root.get(), "missing"), -1); + EXPECT_EQ(errno, ENOENT); + EXPECT_EQ(lstat_at(root.get(), "missing", &info), -1); + EXPECT_EQ(errno, ENOENT); } TEST(CoreAIStorageTest, OpensDirectoriesAndReportsCreation) { TestDirectory storage; ASSERT_NE(storage.url, nil); FileDescriptor root(open( - storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); - auto missing = open_storage_directory(root.get(), @"child", false); + auto missing = open_storage_directory( + root.get(), @"child", StorageCreationMode::OpenExisting); ASSERT_TRUE(missing.ok()); EXPECT_EQ(missing.get().fd, -1); EXPECT_FALSE(missing.get().created); - StorageFaultScope sync_fault(StorageOperation::SyncDirectory); - auto created = open_storage_directory(root.get(), @"child", true); + StorageFaultScope sync_fault(FaultPoint::SyncDirectory); + auto created = + open_storage_directory(root.get(), @"child", StorageCreationMode::Create); ASSERT_TRUE(created.ok()); FileDescriptor first(created.get().fd); ASSERT_GE(first.get(), 0); @@ -77,9 +67,10 @@ FileDescriptor root(open( ASSERT_EQ(fstat(first.get(), &original), 0); EXPECT_TRUE(S_ISDIR(original.st_mode)); - StorageFaultScope create_fault(StorageOperation::BeforeCreateDirectory); - for (bool create : {false, true}) { - auto reopened = open_storage_directory(root.get(), @"child", create); + StorageFaultScope create_fault(FaultPoint::BeforeCreateDirectory); + for (StorageCreationMode mode : + {StorageCreationMode::OpenExisting, StorageCreationMode::Create}) { + auto reopened = open_storage_directory(root.get(), @"child", mode); ASSERT_TRUE(reopened.ok()); FileDescriptor next(reopened.get().fd); ASSERT_GE(next.get(), 0); @@ -95,16 +86,18 @@ FileDescriptor root(open( TestDirectory storage; ASSERT_NE(storage.url, nil); FileDescriptor root(open( - storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); NSURL* child = [storage.url URLByAppendingPathComponent:@"child"]; - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); fault.armed = false; - fault.observe = ^(StorageOperation operation) { - if (operation == StorageOperation::BeforeCreateDirectory) + fault.observe = ^(FaultPoint operation) { + if (operation == FaultPoint::BeforeCreateDirectory) EXPECT_EQ(mkdir(child.fileSystemRepresentation, 0777), 0); }; - auto opened = open_storage_directory(root.get(), @"child", true); + auto opened = + open_storage_directory(root.get(), @"child", StorageCreationMode::Create); ASSERT_TRUE(opened.ok()); FileDescriptor directory(opened.get().fd); EXPECT_GE(directory.get(), 0); @@ -115,138 +108,22 @@ FileDescriptor root(open( TestDirectory storage; ASSERT_NE(storage.url, nil); FileDescriptor root(open( - storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + storage.url.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); EXPECT_EQ( - open_storage_directory(root.get(), nil, false).error(), + open_storage_directory(root.get(), nil, StorageCreationMode::OpenExisting) + .error(), Error::InvalidArgument); for (NSString* name in @[ @"", @".", @"..", @"nested/child" ]) { EXPECT_EQ( - open_storage_directory(root.get(), name, false).error(), + open_storage_directory( + root.get(), name, StorageCreationMode::OpenExisting) + .error(), Error::InvalidArgument); } } -TEST(CoreAIStorageTest, SkipsCreationForExistingRoots) { - TestDirectory storage; - ASSERT_NE(storage.url, nil); - NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; - ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); - EXPECT_TRUE(prepare_storage_root(root.path, true).ok()); - EXPECT_TRUE(prepare_storage_root(root.path, false).ok()); - EXPECT_EQ(fault.matches, 0); - EXPECT_TRUE(backup_excluded(root)); - - NSURL* missing = [storage.url URLByAppendingPathComponent:@"missing"]; - EXPECT_EQ( - prepare_storage_root(missing.path, false).error(), - Error::InvalidArgument); - EXPECT_EQ(fault.matches, 0); - EXPECT_EQ( - prepare_storage_root(missing.path, true).error(), Error::AccessFailed); - EXPECT_EQ(fault.hits, 1); - EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:missing.path]); - EXPECT_TRUE(prepare_storage_root(missing.path, true).ok()); -} - -TEST(CoreAIStorageTest, CreatesDirectoriesWithDefaultPermissions) { - TestDirectory storage; - ASSERT_NE(storage.url, nil); - NSURL* reference = [storage.url URLByAppendingPathComponent:@"reference"]; - ASSERT_EQ(mkdir(reference.fileSystemRepresentation, 0777), 0); - struct stat expected; - ASSERT_EQ(stat(reference.fileSystemRepresentation, &expected), 0); - NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; - ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); - FileDescriptor directory(open( - root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); - ASSERT_GE(directory.get(), 0); - ASSERT_EQ( - write_storage_file(directory.get(), @"nested/file", "x", 1), Error::Ok); - for (NSURL* entry in @[ root, [root URLByAppendingPathComponent:@"nested"] ]) { - struct stat actual; - ASSERT_EQ(stat(entry.fileSystemRepresentation, &actual), 0); - EXPECT_EQ(actual.st_mode & 0777, expected.st_mode & 0777); - } -} - -TEST(CoreAIStorageTest, RecreatesRemovedRoot) { - TestDirectory storage; - ASSERT_NE(storage.url, nil); - NSURL* root = [storage.url URLByAppendingPathComponent:@"cache"]; - ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); - ASSERT_EQ(rmdir(root.fileSystemRepresentation), 0); - ASSERT_TRUE(prepare_storage_root(root.path, true).ok()); - EXPECT_TRUE(backup_excluded(root)); -} - -TEST(CoreAIStorageTest, NewRootAndDescendantsUseFirstUnlockProtection) { - NSFileManager* manager = NSFileManager.defaultManager; - NSURL* root = [[NSURL fileURLWithPath:NSTemporaryDirectory() isDirectory:YES] - URLByAppendingPathComponent:NSUUID.UUID.UUIDString - isDirectory:YES]; - auto prepared = prepare_storage_root(root.path, true); - EXPECT_TRUE(prepared.ok()); - if (prepared.ok()) { - FileDescriptor directory(open( - root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); - EXPECT_GE(directory.get(), 0); - if (directory.get() >= 0) { - EXPECT_EQ( - write_storage_file(directory.get(), @"nested/model.bin", "x", 1), - Error::Ok); - for (NSURL* entry in @[ - root, - [root URLByAppendingPathComponent:@"nested"], - [root URLByAppendingPathComponent:@"nested/model.bin"] - ]) { - NSDictionary* attributes = [manager attributesOfItemAtPath:entry.path - error:nil]; - EXPECT_TRUE([attributes[NSFileProtectionKey] - isEqual:NSFileProtectionCompleteUntilFirstUserAuthentication]); - } - } - EXPECT_TRUE([manager - setAttributes:@{NSFileProtectionKey : NSFileProtectionComplete} - ofItemAtPath:root.path - error:nil]); - EXPECT_TRUE(prepare_storage_root(root.path, true).ok()); - NSDictionary* existing = [manager attributesOfItemAtPath:root.path - error:nil]; - EXPECT_TRUE( - [existing[NSFileProtectionKey] isEqual:NSFileProtectionComplete]); - EXPECT_TRUE([manager removeItemAtURL:root error:nil]); - } -} - -TEST(CoreAIStorageTest, RejectsInvalidRootsConsistently) { - TestDirectory storage; - ASSERT_NE(storage.url, nil); - NSURL* regular = [storage.url URLByAppendingPathComponent:@"regular"]; - NSData* bytes = [@"unchanged" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_TRUE([bytes writeToURL:regular atomically:NO]); - for (bool create : {false, true}) { - SCOPED_TRACE(create); - EXPECT_EQ( - prepare_storage_root(nil, create).error(), Error::InvalidArgument); - for (NSString* path in @[ - @"", - @"relative", - @"~", - @"~/cache", - @"~nobody/cache", - regular.path, - [regular.path stringByAppendingPathComponent:@"child"] - ]) { - SCOPED_TRACE(path.UTF8String); - EXPECT_EQ( - prepare_storage_root(path, create).error(), Error::InvalidArgument); - } - } - EXPECT_TRUE([[NSData dataWithContentsOfURL:regular] isEqual:bytes]); -} - TEST(CoreAIStorageTest, ListsChildrenRepeatedlyInSortedOrder) { TestDirectory storage; ASSERT_NE(storage.url, nil); @@ -291,8 +168,9 @@ FileDescriptor file(openat( O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); ASSERT_GE(file.get(), 0); - for (bool skip_invalid : {false, true}) { - auto names = storage_children(root.get(), skip_invalid); + for (StorageNamePolicy policy : + {StorageNamePolicy::Strict, StorageNamePolicy::SkipInvalid}) { + auto names = storage_children(root.get(), policy); ASSERT_TRUE(names.ok()); ASSERT_EQ(names.get().count, 1); EXPECT_STREQ(names.get().firstObject.UTF8String, name.UTF8String); @@ -340,7 +218,7 @@ FileDescriptor root(open( ASSERT_EQ( write_storage_file(root.get(), @"nested/parents/first", "x", 1), Error::Ok); - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); EXPECT_EQ( write_storage_file(root.get(), @"nested/parents/second", "x", 1), Error::Ok); @@ -360,11 +238,11 @@ FileDescriptor root(open( O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); NSURL* nested = [storage.url URLByAppendingPathComponent:@"nested"]; - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); fault.armed = false; __block int creations = 0; - fault.observe = ^(StorageOperation operation) { - if (operation == StorageOperation::BeforeCreateDirectory) { + fault.observe = ^(FaultPoint operation) { + if (operation == FaultPoint::BeforeCreateDirectory) { ++creations; EXPECT_EQ(mkdir(nested.fileSystemRepresentation, 0700), 0); } @@ -389,7 +267,7 @@ FileDescriptor root(open( 0); NSURL* regular = [storage.url URLByAppendingPathComponent:@"regular"]; ASSERT_TRUE([[NSData data] writeToURL:regular atomically:NO]); - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); for (NSString* path in @[ @"link/file", @"regular/file" ]) { EXPECT_EQ( write_storage_file(root.get(), path, "x", 1), Error::AccessFailed); @@ -401,13 +279,13 @@ FileDescriptor root(open( TEST(CoreAIStorageTest, RemovesFailedWritesAndAllowsRetry) { const struct { - StorageOperation operation; + FaultPoint operation; int match_index; } rows[] = { - {StorageOperation::Write, 2}, - {StorageOperation::SyncFile, 1}, - {StorageOperation::SyncDirectory, 1}, - {StorageOperation::SyncDirectory, 2}}; + {FaultPoint::Write, 2}, + {FaultPoint::SyncFile, 1}, + {FaultPoint::SyncDirectory, 1}, + {FaultPoint::SyncDirectory, 2}}; const std::vector bytes(1024 * 1024 + 1, 0x5a); for (const auto& row : rows) { SCOPED_TRACE(static_cast(row.operation)); @@ -444,10 +322,10 @@ FileDescriptor root(open( storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); - __block std::vector operations; - StorageFaultScope fault(StorageOperation::SyncDirectory); + __block std::vector operations; + StorageFaultScope fault(FaultPoint::SyncDirectory); fault.armed = false; - fault.observe = ^(StorageOperation operation) { + fault.observe = ^(FaultPoint operation) { operations.push_back(operation); }; EXPECT_EQ( @@ -455,14 +333,14 @@ FileDescriptor root(open( Error::Ok); EXPECT_EQ( operations, - (std::vector{ - StorageOperation::BeforeCreateDirectory, - StorageOperation::SyncDirectory, - StorageOperation::BeforeCreateDirectory, - StorageOperation::SyncDirectory, - StorageOperation::Write, - StorageOperation::SyncFile, - StorageOperation::SyncDirectory})); + (std::vector{ + FaultPoint::BeforeCreateDirectory, + FaultPoint::SyncDirectory, + FaultPoint::BeforeCreateDirectory, + FaultPoint::SyncDirectory, + FaultPoint::Write, + FaultPoint::SyncFile, + FaultPoint::SyncDirectory})); } TEST(CoreAIStorageTest, RejectsUnsafeWritePathsWithoutCreatingFiles) { @@ -554,12 +432,12 @@ FileDescriptor root(open( NSData* next = [@"next" dataUsingEncoding:NSUTF8StringEncoding]; NSURL* published = [storage.url URLByAppendingPathComponent:@"published"]; ASSERT_EQ(publish_storage_data(root.get(), @"published", old), Error::Ok); - for (StorageOperation operation : - {StorageOperation::Write, - StorageOperation::SyncFile, - StorageOperation::SyncDirectory, - StorageOperation::FullSync, - StorageOperation::Rename}) { + for (FaultPoint operation : + {FaultPoint::Write, + FaultPoint::SyncFile, + FaultPoint::SyncDirectory, + FaultPoint::FullSync, + FaultPoint::Rename}) { SCOPED_TRACE(static_cast(operation)); StorageFaultScope fault(operation); EXPECT_EQ( @@ -582,23 +460,23 @@ FileDescriptor root(open( storage.url.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); - __block std::vector operations; - StorageFaultScope fault(StorageOperation::FullSync); + __block std::vector operations; + StorageFaultScope fault(FaultPoint::FullSync); fault.armed = false; - fault.observe = ^(StorageOperation operation) { + fault.observe = ^(FaultPoint operation) { operations.push_back(operation); }; NSData* data = [@"published" dataUsingEncoding:NSUTF8StringEncoding]; EXPECT_EQ(publish_storage_data(root.get(), @"published", data), Error::Ok); EXPECT_EQ( operations, - (std::vector{ - StorageOperation::Write, - StorageOperation::SyncFile, - StorageOperation::FullSync, - StorageOperation::SyncDirectory, - StorageOperation::Rename, - StorageOperation::FullSync})); + (std::vector{ + FaultPoint::Write, + FaultPoint::SyncFile, + FaultPoint::FullSync, + FaultPoint::SyncDirectory, + FaultPoint::Rename, + FaultPoint::FullSync})); } TEST(CoreAIStorageTest, ReportsPostRenameFlushFailureWithNewContentsVisible) { @@ -611,7 +489,7 @@ FileDescriptor root(open( NSData* old = [@"old" dataUsingEncoding:NSUTF8StringEncoding]; NSData* next = [@"next" dataUsingEncoding:NSUTF8StringEncoding]; ASSERT_EQ(publish_storage_data(root.get(), @"published", old), Error::Ok); - StorageFaultScope fault(StorageOperation::FullSync); + StorageFaultScope fault(FaultPoint::FullSync); fault.match_index = 2; EXPECT_EQ( publish_storage_data(root.get(), @"published", next), @@ -630,12 +508,12 @@ FileDescriptor root(open( O_RDONLY | O_DIRECTORY | O_CLOEXEC)); ASSERT_GE(root.get(), 0); NSData* data = [@"published" dataUsingEncoding:NSUTF8StringEncoding]; - for (StorageOperation operation : - {StorageOperation::Write, - StorageOperation::SyncFile, - StorageOperation::SyncDirectory, - StorageOperation::FullSync, - StorageOperation::Rename}) { + for (FaultPoint operation : + {FaultPoint::Write, + FaultPoint::SyncFile, + FaultPoint::SyncDirectory, + FaultPoint::FullSync, + FaultPoint::Rename}) { SCOPED_TRACE(static_cast(operation)); StorageFaultScope fault(operation); fault.error = EINTR;