From 7e686f057981db2f31c70358f23fe2288d63408d Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Sun, 4 Oct 2026 19:49:18 -0700 Subject: [PATCH 1/6] [CoreAI] Persist raw bookmarks Summary: Persist the raw SDK bookmark for a selected Core AI bundle, so later loads can restore a specialized model without re-reading the payload. - Bookmarks (`runtime/coreai_bookmarks.{h,mm}`): store raw opaque bookmarks (at most 8 MiB) with atomic publication. Keys include the bundle digest, platform, architecture and persistence policy, but not function bindings or delivery location. Per-key OS locks coordinate threads and processes. - SDK acquisition and cache removal are not included here. - README: the "Asset storage" key derivation, bookmark bound and locking text. Trust model: the application owns the assets root and is trusted not to modify it while the backend uses 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 bookmark suite, including a cross-process lock test that spawns child processes. --- backends/apple/coreai/CMakeLists.txt | 17 +- backends/apple/coreai/README.md | 16 ++ .../apple/coreai/runtime/coreai_bookmarks.h | 65 +++++ .../apple/coreai/runtime/coreai_bookmarks.mm | 246 +++++++++++++++++ .../runtime/test/coreai_bookmark_fixture.h | 49 ++++ .../runtime/test/coreai_bookmark_fixture.mm | 254 ++++++++++++++++++ .../runtime/test/coreai_bookmark_test.mm | 210 +++++++++++++++ .../runtime/test/coreai_filesystem_fixture.h | 8 + .../runtime/test/coreai_filesystem_fixture.mm | 22 ++ .../coreai/runtime/test/coreai_host_test.mm | 4 + 10 files changed, 885 insertions(+), 6 deletions(-) create mode 100644 backends/apple/coreai/runtime/coreai_bookmarks.h create mode 100644 backends/apple/coreai/runtime/coreai_bookmarks.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h create mode 100644 backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_bookmark_test.mm diff --git a/backends/apple/coreai/CMakeLists.txt b/backends/apple/coreai/CMakeLists.txt index bb63606a1a3..b7a6cce7226 100644 --- a/backends/apple/coreai/CMakeLists.txt +++ b/backends/apple/coreai/CMakeLists.txt @@ -11,9 +11,9 @@ endif() enable_language(OBJCXX) 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 + runtime/coreai_bookmarks.mm 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) @@ -32,9 +32,14 @@ endfunction() 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_cache_root_test.mm runtime/test/coreai_storage_test.mm - runtime/test/coreai_filesystem_fixture.mm ${_coreai_runtime_sources} + runtime/test/coreai_host_test.mm + runtime/test/coreai_manifest_test.mm + runtime/test/coreai_cache_root_test.mm + runtime/test/coreai_storage_test.mm + runtime/test/coreai_filesystem_fixture.mm + runtime/test/coreai_bookmark_test.mm + runtime/test/coreai_bookmark_fixture.mm + ${_coreai_runtime_sources} ) coreai_configure_objc_target(coreai_host_test) target_compile_definitions(coreai_host_test PRIVATE COREAI_ASSETS_TESTING=1) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 1a3d96adc7a..323ad97bac3 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -82,6 +82,22 @@ 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. +Each partition derives a key from its selected export digest/bundle, platform, +Core AI device architecture, default SDK cache namespace, versioned default +options and persistent policy. Function bindings and delivery location are +excluded. No model bytes are hashed at runtime. Each key determines a raw +bookmark in the `bookmarks` subdirectory, a bundle directory in `staging`, and an +empty coordination file in `locks`. There is no PTE-wide bookmark list or +serialized lifecycle record. + +Raw bookmarks have an 8 MiB backend allocation limit, not an asserted SDK format +maximum, and are published atomically because losing one orphans an SDK entry. +Per-key disk locks coordinate loading and eviction across threads and processes; +different keys remain independent. Lock ownership is maintained by the OS, and +acquiring a lock does not flush its file or directory. Lock files are not +removed by the backend, including after process exit. Removing the assets root +externally requires all loads, sessions and maintenance to stop. + ## Host Tests On macOS, `EXECUTORCH_BUILD_COREAI=ON` with `EXECUTORCH_BUILD_TESTS=ON` diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.h b/backends/apple/coreai/runtime/coreai_bookmarks.h new file mode 100644 index 00000000000..fa838985fd7 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_bookmarks.h @@ -0,0 +1,65 @@ +/* + * 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 +#import "coreai_assets.h" +#include "coreai_file.h" + +namespace executorch::backends::coreai { + +runtime::Result bookmark_key( + const Manifest& selected, + NSString* platform, + NSString* architecture); +// With create=false, a missing root is returned unchanged for no-op +// maintenance. +runtime::Result resolve_bookmark_root( + NSString* path, + bool create = true); + +class BookmarkLock final { + public: + BookmarkLock(const BookmarkLock&) = delete; + BookmarkLock& operator=(const BookmarkLock&) = delete; + NSString* root_path() const { + return root_path_; + } + + private: + friend runtime::Result> + lock_bookmark(NSString* root, NSString* key, bool create); + friend runtime::Result read_bookmark(const BookmarkLock& lock); + friend runtime::Error write_bookmark(const BookmarkLock& lock, NSData* data); + friend runtime::Result prepare_bookmark_staging( + const BookmarkLock& lock); + BookmarkLock( + int root, + int bookmarks, + int file, + NSString* root_path, + NSString* key); + FileDescriptor root_; + FileDescriptor bookmarks_; + FileDescriptor file_; + NSString* root_path_; + NSString* key_; +}; + +// Each acquisition opens an independent descriptor; the backend never removes +// lock files. With create=false, missing roots or entries return a null lock. +runtime::Result> +lock_bookmark(NSString* root, NSString* key, bool create = true); +// Missing is nil. Empty, oversized, inaccessible or nonregular files fail +// closed. +runtime::Result read_bookmark(const BookmarkLock& lock); +runtime::Error write_bookmark(const BookmarkLock& lock, NSData* data); +// Prepared only for cold loads; source preparation appends key/bundle. +runtime::Result prepare_bookmark_staging(const BookmarkLock& lock); + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.mm b/backends/apple/coreai/runtime/coreai_bookmarks.mm new file mode 100644 index 00000000000..0ef1fd972f2 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_bookmarks.mm @@ -0,0 +1,246 @@ +/* + * 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_bookmarks.h" +#import "coreai_storage.h" + +#include +#include +#include +#include +#include +#include +#include + +namespace executorch::backends::coreai { +namespace { +using runtime::Error; +using runtime::Result; +// An allocation bound, not an SDK bookmark format limit. +constexpr size_t kBookmarkByteLimit = 8 * 1024 * 1024; + +bool text(id value) { + return [value isKindOfClass:NSString.class] && [value length] > 0 && + [value length] <= 4096 && + [value rangeOfString:@"\0"].location == NSNotFound; +} +bool component(id value) { + return text(value) && ![value containsString:@"/"] && + ![value containsString:@"\\"] && ![value isEqual:@"."] && + ![value isEqual:@".."]; +} +bool digest(id value) { + return text(value) && [value length] == 64 && + [value rangeOfCharacterFromSet: + [[NSCharacterSet + characterSetWithCharactersInString:@"0123456789abcdef"] + invertedSet]] + .location == NSNotFound; +} +NSString* bookmark_name(NSString* key) { + return [key stringByAppendingString:@".bookmark"]; +} +} // namespace + +Result bookmark_key(const Manifest& selected, NSString* platform, + NSString* architecture) { + NSString* bundle = selected.path.lastPathComponent; + NSString* exported_digest = selected.bundle_digests[bundle]; + ET_CHECK_OR_RETURN_ERROR( + digest(exported_digest) && component(bundle) && text(platform) && + text(architecture), + DelegateInvalidCompatibility, + "Core AI bookmark key requires a selected bundle and SDK architecture"); + NSArray* fields = @[ + @"1", exported_digest, selected.aot_compiled ? @"aot" : @"source", bundle, + platform, architecture, @"AIModelCache.default", @"default-options-v1", + @"persistent" + ]; + NSData* encoded = [NSJSONSerialization dataWithJSONObject:fields + options:0 + error:nil]; + ET_CHECK_OR_RETURN_ERROR(encoded != nil && encoded.length <= UINT32_MAX, + InvalidExternalData, "Cannot encode bookmark key"); + unsigned char bytes[CC_SHA256_DIGEST_LENGTH]; + CC_SHA256(encoded.bytes, static_cast(encoded.length), bytes); + NSMutableString* key = [NSMutableString stringWithCapacity:64]; + for (unsigned char byte : bytes) [key appendFormat:@"%02x", byte]; + return [key copy]; +} + +namespace { +Result resolve_root(NSString* path, bool create) { + ET_CHECK_OR_RETURN_ERROR(text(path) && path.isAbsolutePath, InvalidArgument, + "Core AI assets require an absolute root path"); + if (!create) { + struct stat info; + const int result = + retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); + if (result < 0 && errno == ENOENT) return path; + ET_CHECK_OR_RETURN_ERROR(result == 0 && S_ISDIR(info.st_mode), + AccessFailed, "Invalid Core AI assets root"); + } + return prepare_storage_root(path, create); +} + +// Returns -1 when the root does not exist. +Result open_root(NSString* root) { + const int fd = retry_eintr([&] { + return open(root.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_CLOEXEC); + }); + if (fd < 0 && errno == ENOENT) return -1; + ET_CHECK_OR_RETURN_ERROR(fd >= 0, AccessFailed, + "Cannot open Core AI assets root"); + return fd; +} + +// Returns -1 when the child does not exist and create is false. +Result open_child(int root_fd, NSString* name, bool create) { + bool created = false; + if (create) { + const int made = retry_eintr([&] { + return mkdirat(root_fd, name.fileSystemRepresentation, 0700); + }); + ET_CHECK_OR_RETURN_ERROR(made == 0 || errno == EEXIST, AccessFailed, + "Cannot create Core AI assets directory"); + created = made == 0; + } + const int fd = retry_eintr([&] { + return openat(root_fd, name.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + }); + if (fd < 0 && errno == ENOENT && !create) return -1; + ET_CHECK_OR_RETURN_ERROR(fd >= 0, AccessFailed, + "Invalid Core AI assets directory"); + if (created) { + FileDescriptor child(fd); + ET_CHECK_OK_OR_RETURN_ERROR(sync_storage_directory(root_fd)); + return child.release(); + } + return fd; +} + +Result entry_exists(int root_fd, NSString* key) { + for (NSString* name in @[ @"bookmarks", @"staging" ]) { + auto opened = open_child(root_fd, name, false); + if (!opened.ok()) return opened.error(); + FileDescriptor child(opened.get()); + if (child.get() < 0) continue; + NSString* entry = [name isEqual:@"bookmarks"] ? bookmark_name(key) : key; + struct stat info; + const int result = retry_eintr([&] { + return fstatat(child.get(), entry.fileSystemRepresentation, &info, + AT_SYMLINK_NOFOLLOW); + }); + if (result == 0) return true; + ET_CHECK_OR_RETURN_ERROR(errno == ENOENT, AccessFailed, + "Cannot inspect Core AI cache entry"); + } + return false; +} +} // namespace + +Result resolve_bookmark_root(NSString* path, bool create) { + return resolve_root(path, create); +} + +BookmarkLock::BookmarkLock(int root, int bookmarks, int file, + NSString* root_path, NSString* key) + : root_(root), + bookmarks_(bookmarks), + file_(file), + root_path_(root_path), + key_(key) {} + +Result> lock_bookmark(NSString* root, + NSString* key, + bool create) { + ET_CHECK_OR_RETURN_ERROR(digest(key), InvalidArgument, + "Invalid Core AI bookmark key"); + auto resolved = resolve_root(root, create); + if (!resolved.ok()) return resolved.error(); + root = resolved.get(); + auto opened = open_root(root); + if (!opened.ok()) return opened.error(); + FileDescriptor directory(opened.get()); + if (directory.get() < 0) { + ET_CHECK_OR_RETURN_ERROR(!create, AccessFailed, + "Core AI assets root disappeared"); + return std::unique_ptr(); + } + if (!create) { + auto exists = entry_exists(directory.get(), key); + if (!exists.ok()) return exists.error(); + if (!exists.get()) return std::unique_ptr(); + } + auto bookmark_result = open_child(directory.get(), @"bookmarks", create); + if (!bookmark_result.ok()) return bookmark_result.error(); + FileDescriptor bookmarks(bookmark_result.get()); + auto lock_result = open_child(directory.get(), @"locks", true); + if (!lock_result.ok()) return lock_result.error(); + FileDescriptor locks(lock_result.get()); + NSString* name = [key stringByAppendingString:@".lock"]; + FileDescriptor file( + open_storage_shared_file(locks.get(), name.fileSystemRepresentation)); + struct stat info; + ET_CHECK_OR_RETURN_ERROR(file.get() >= 0 && fstat(file.get(), &info) == 0 && + S_ISREG(info.st_mode), + AccessFailed, "Invalid Core AI bookmark lock"); + ET_CHECK_OR_RETURN_ERROR( + retry_eintr([&] { return flock(file.get(), LOCK_EX); }) == 0, + AccessFailed, "Cannot lock Core AI bookmark"); + return std::unique_ptr( + new BookmarkLock(directory.release(), bookmarks.release(), file.release(), + root, [key copy])); +} + +Result read_bookmark(const BookmarkLock& lock) { + if (lock.bookmarks_.get() < 0) return static_cast(nil); + FileDescriptor file(openat(lock.bookmarks_.get(), + bookmark_name(lock.key_).fileSystemRepresentation, + O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC)); + if (file.get() < 0 && errno == ENOENT) return static_cast(nil); + ET_CHECK_OR_RETURN_ERROR(file.get() >= 0, AccessFailed, + "Cannot open Core AI bookmark"); + struct stat info; + ET_CHECK_OR_RETURN_ERROR( + fstat(file.get(), &info) == 0 && S_ISREG(info.st_mode) && + info.st_size > 0 && + static_cast(info.st_size) <= kBookmarkByteLimit, + InvalidExternalData, "Invalid Core AI bookmark file"); + NSMutableData* bytes = [NSMutableData dataWithLength:info.st_size]; + size_t offset = 0; + while (offset < bytes.length) { + ssize_t count = + ::read(file.get(), static_cast(bytes.mutableBytes) + offset, + bytes.length - offset); + if (count < 0 && errno == EINTR) continue; + ET_CHECK_OR_RETURN_ERROR(count > 0, AccessFailed, + "Cannot read Core AI bookmark"); + offset += count; + } + return bytes; +} + +Error write_bookmark(const BookmarkLock& lock, NSData* data) { + ET_CHECK_OR_RETURN_ERROR( + [data isKindOfClass:NSData.class] && data.length > 0 && + data.length <= kBookmarkByteLimit, + InvalidExternalData, "Invalid Core AI bookmark data"); + return publish_storage_data(lock.bookmarks_.get(), bookmark_name(lock.key_), + data); +} + +Result prepare_bookmark_staging(const BookmarkLock& lock) { + auto staging = open_child(lock.root_.get(), @"staging", true); + if (!staging.ok()) return staging.error(); + FileDescriptor directory(staging.get()); + return [lock.root_path_ stringByAppendingPathComponent:@"staging"]; +} + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h new file mode 100644 index 00000000000..2208af49ec2 --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h @@ -0,0 +1,49 @@ +/* + * 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 +#include +#include +#include "coreai_bookmarks.h" + +namespace executorch::backends::coreai::testing { + +runtime::Result bookmark_manifest(bool aot = false); +runtime::Result test_bookmark_key(const Manifest& manifest); +NSURL* test_bookmark_url(NSURL* root, NSString* key); +runtime::Result saved_bookmark(NSURL* root, NSString* key); +runtime::Result sized_bookmark(size_t size, uint32_t tag = 1); + +void set_bookmark_test_executable(const char* path); +// Returns -1 for normal test invocations, otherwise a child-mode exit code. +int bookmark_child_mode(int argc, char** argv); + +class BookmarkChild { + public: + BookmarkChild() = default; + ~BookmarkChild(); + BookmarkChild(const BookmarkChild&) = delete; + BookmarkChild& operator=(const BookmarkChild&) = delete; + + ::testing::AssertionResult + spawn(NSString* mode, NSString* root, NSString* argument); + ::testing::AssertionResult receive(char expected); + ::testing::AssertionResult expect_blocked(); + ::testing::AssertionResult expect_exit(int expected); + ::testing::AssertionResult kill_and_reap(); + + private: + pid_t pid_ = -1; + int signal_fd_ = -1; + ::testing::AssertionResult reap(int& status); +}; + +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm new file mode 100644 index 00000000000..51713168626 --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm @@ -0,0 +1,254 @@ +/* + * 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_bookmark_fixture.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "coreai_manifest_fixture.h" + +extern char** environ; + +namespace executorch::backends::coreai::testing { +namespace { +using runtime::Error; +using runtime::Result; +using ::testing::AssertionFailure; +using ::testing::AssertionSuccess; +using Clock = std::chrono::steady_clock; +constexpr auto kChildTimeout = std::chrono::seconds(10); +constexpr auto kContentionObservation = std::chrono::milliseconds(100); +constexpr int kChildSignalFD = 3; +std::string executable; + +int remaining_ms(Clock::time_point deadline) { + const auto remaining = + std::chrono::duration_cast(deadline - Clock::now()); + return remaining.count() <= 0 ? 0 : static_cast(remaining.count()); +} + +bool send_signal_byte(int fd, char byte) { + const auto deadline = Clock::now() + kChildTimeout; + while (const int remaining = remaining_ms(deadline)) { + pollfd entry{fd, POLLOUT, 0}; + const int ready = poll(&entry, 1, remaining); + if (ready < 0 && errno == EINTR) continue; + if (ready <= 0 || !(entry.revents & POLLOUT)) return false; + const ssize_t written = write(fd, &byte, 1); + if (written == 1) return true; + if (written < 0 && (errno == EINTR || errno == EAGAIN)) continue; + return false; + } + return false; +} +} // namespace + +Result bookmark_manifest(bool aot) { + NSData* encoded = encode(aot ? aot_manifest_dict() : manifest_dict()); + if (encoded == nil) return Error::InvalidExternalData; + auto parsed = parse_manifest(encoded); + if (!parsed.ok()) return parsed.error(); + return select_assets(parsed.get(), @"arch_b", @"macOS"); +} + +Result test_bookmark_key(const Manifest& manifest) { + return bookmark_key(manifest, @"macOS", @"arch_b"); +} + +NSURL* test_bookmark_url(NSURL* root, NSString* key) { + return [[root URLByAppendingPathComponent:@"bookmarks"] + URLByAppendingPathComponent:[key stringByAppendingString:@".bookmark"]]; +} + +Result saved_bookmark(NSURL* root, NSString* key) { + auto locked = lock_bookmark(root.path, key); + if (!locked.ok()) return locked.error(); + return read_bookmark(*locked.get()); +} + +Result sized_bookmark(size_t size, uint32_t tag) { + if (size < sizeof(tag)) return Error::InvalidArgument; + NSMutableData* bytes = [NSMutableData dataWithLength:size]; + if (bytes == nil) return Error::MemoryAllocationFailed; + std::memcpy(bytes.mutableBytes, &tag, sizeof(tag)); + return [bytes copy]; +} + +void set_bookmark_test_executable(const char* path) { executable = path == nullptr ? "" : path; } + +int bookmark_child_mode(int argc, char** argv) { + if (argc < 2 || std::strncmp(argv[1], "--bookmark-", 11) != 0) return -1; + if (argc != 5) return 1; + const bool hold = std::strcmp(argv[1], "--bookmark-hold") == 0; + const bool lock = std::strcmp(argv[1], "--bookmark-lock") == 0; + const bool other = std::strcmp(argv[3], "other") == 0; + const bool same = std::strcmp(argv[3], "same") == 0; + if ((!hold && !lock) || (!other && !same)) return 2; + char* end = nullptr; + errno = 0; + const long value = std::strtol(argv[4], &end, 10); + if (errno != 0 || end == argv[4] || *end != '\0' || value < 0 || value > INT_MAX) return 3; + FileDescriptor signal_fd(static_cast(value)); + if (fcntl(signal_fd.get(), F_GETFD) < 0) return 4; + signal(SIGPIPE, SIG_IGN); + NSString* root = [NSString stringWithUTF8String:argv[2]]; + if (root == nil) return 5; + // Child modes run before InitGoogleTest and must not call assertion helpers. + Manifest manifest; + manifest.path = @"ab/model.aimodel"; + manifest.bundle_digests = @{ + @"model.aimodel" : [NSString stringWithUTF8String:std::string(64, other ? '2' : '1').c_str()] + }; + auto key = test_bookmark_key(manifest); + if (!key.ok()) return 6; + if (lock && same && !send_signal_byte(signal_fd.get(), 'B')) return 7; + auto locked = lock_bookmark(root, key.get()); + if (!locked.ok()) return 7; + if (!send_signal_byte(signal_fd.get(), 'L')) return 8; + if (close(signal_fd.release()) != 0) return 9; + if (hold) { + // The parent owns this PID and always kills and reaps it, including on failure. + for (;;) pause(); + } + return 0; +} + +BookmarkChild::~BookmarkChild() { + if (pid_ > 0) { + const auto cleaned = kill_and_reap(); + if (!cleaned) ADD_FAILURE() << cleaned.message(); + } + if (signal_fd_ >= 0 && close(signal_fd_) != 0) { + ADD_FAILURE() << "Cannot close child signal descriptor: " << errno; + } +} + +::testing::AssertionResult BookmarkChild::spawn(NSString* mode, NSString* root, + NSString* argument) { + if (pid_ > 0 || signal_fd_ >= 0 || executable.empty() || root == nil) { + return AssertionFailure() << "Invalid bookmark child setup"; + } + int descriptors[2]; + if (pipe(descriptors) != 0) return AssertionFailure() << "pipe: " << errno; + FileDescriptor reader(descriptors[0]); + FileDescriptor writer(descriptors[1]); + for (int fd : descriptors) { + if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0 || fcntl(fd, F_SETFL, O_NONBLOCK) != 0) { + return AssertionFailure() << "fcntl pipe: " << errno; + } + } + posix_spawn_file_actions_t actions; + int error = posix_spawn_file_actions_init(&actions); + if (error != 0) return AssertionFailure() << "spawn actions: " << error; + error = posix_spawn_file_actions_adddup2(&actions, writer.get(), kChildSignalFD); + std::string descriptor = std::to_string(kChildSignalFD); + char* arguments[] = {const_cast(executable.c_str()), + const_cast(mode.UTF8String), + const_cast(root.fileSystemRepresentation), + const_cast(argument.UTF8String), + descriptor.data(), + nullptr}; + if (error == 0) { + error = posix_spawn(&pid_, arguments[0], &actions, nullptr, arguments, environ); + } + const int destroyed = posix_spawn_file_actions_destroy(&actions); + if (error != 0) { + pid_ = -1; + return AssertionFailure() << "posix_spawn: " << error; + } + signal_fd_ = reader.release(); + if (destroyed != 0) return AssertionFailure() << "destroy spawn actions: " << destroyed; + return AssertionSuccess(); +} + +::testing::AssertionResult BookmarkChild::receive(char expected) { + const auto deadline = Clock::now() + kChildTimeout; + while (const int remaining = remaining_ms(deadline)) { + pollfd entry{signal_fd_, POLLIN, 0}; + const int ready = poll(&entry, 1, remaining); + if (ready < 0 && errno == EINTR) continue; + if (ready < 0) return AssertionFailure() << "poll: " << errno; + if (ready == 0) break; + if (!(entry.revents & POLLIN)) + return AssertionFailure() << "Child closed signal pipe before " << expected; + char byte = 0; + const ssize_t count = read(signal_fd_, &byte, 1); + if (count < 0 && (errno == EINTR || errno == EAGAIN)) continue; + if (count != 1 || byte != expected) { + return AssertionFailure() << "Expected child signal " << expected << ", got " << int(byte); + } + return AssertionSuccess(); + } + return AssertionFailure() << "Timed out waiting for child signal " << expected; +} + +::testing::AssertionResult BookmarkChild::expect_blocked() { + const auto deadline = Clock::now() + kContentionObservation; + while (const int remaining = remaining_ms(deadline)) { + pollfd entry{signal_fd_, POLLIN, 0}; + const int ready = poll(&entry, 1, remaining); + if (ready < 0 && errno == EINTR) continue; + if (ready < 0) return AssertionFailure() << "poll: " << errno; + if (ready > 0) return AssertionFailure() << "Child completed while lock was held"; + return AssertionSuccess(); + } + return AssertionSuccess(); +} + +::testing::AssertionResult BookmarkChild::reap(int& status) { + if (pid_ <= 0) return AssertionFailure() << "No owned child to reap"; + const auto deadline = Clock::now() + kChildTimeout; + while (const int remaining = remaining_ms(deadline)) { + const pid_t result = waitpid(pid_, &status, WNOHANG); + if (result == pid_) { + pid_ = -1; + return AssertionSuccess(); + } + if (result < 0 && errno == EINTR) continue; + if (result < 0) return AssertionFailure() << "waitpid: " << errno; + poll(nullptr, 0, std::min(remaining, 10)); + } + return AssertionFailure() << "Timed out reaping child " << pid_; +} + +::testing::AssertionResult BookmarkChild::expect_exit(int expected) { + int status = 0; + auto result = reap(status); + if (!result) return result; + if (!WIFEXITED(status) || WEXITSTATUS(status) != expected) { + return AssertionFailure() << "Expected child exit " << expected << ", wait status " << status; + } + return AssertionSuccess(); +} + +::testing::AssertionResult BookmarkChild::kill_and_reap() { + if (pid_ <= 0) return AssertionFailure() << "No owned child to kill"; + const int killed = kill(pid_, SIGKILL); + const int error = errno; + int status = 0; + auto result = reap(status); + if (!result) return result; + if (killed != 0 || !WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) { + return AssertionFailure() << "Expected SIGKILL, kill errno " << (killed == 0 ? 0 : error) + << ", wait status " << status; + } + return AssertionSuccess(); +} + +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm new file mode 100644 index 00000000000..1cce13d973a --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm @@ -0,0 +1,210 @@ +/* + * 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 +#include "coreai_bookmark_fixture.h" +#include "coreai_filesystem_fixture.h" +#include "coreai_manifest_fixture.h" +#include "coreai_storage.h" + +using namespace executorch::runtime; +using namespace executorch::backends::coreai; +using namespace executorch::backends::coreai::testing; + +namespace { +constexpr size_t kMiB = 1024 * 1024; + +class CoreAIBookmarkIdentityTest : public ::testing::Test { + protected: + Manifest manifest; + NSString* key = nil; + + void SetUp() override { + auto selected = bookmark_manifest(); + ASSERT_TRUE(selected.ok()); + manifest = selected.get(); + auto result = test_bookmark_key(manifest); + ASSERT_TRUE(result.ok()); + key = result.get(); + } +}; + +class CoreAIBookmarkPathTest : public CoreAIBookmarkIdentityTest { + protected: + BookmarkDirectory root; + void SetUp() override { + CoreAIBookmarkIdentityTest::SetUp(); + if (HasFatalFailure()) return; + ASSERT_NE(root.url, nil); + } + NSURL* lock_url() { + return [[root.url URLByAppendingPathComponent:@"locks"] + URLByAppendingPathComponent:[key stringByAppendingString:@".lock"]]; + } +}; + +class CoreAIBookmarkRawTest : public CoreAIBookmarkPathTest { + protected: + std::unique_ptr locked; + NSURL* url = nil; + void SetUp() override { + CoreAIBookmarkPathTest::SetUp(); + if (HasFatalFailure()) return; + auto result = lock_bookmark(root.url.path, key); + ASSERT_TRUE(result.ok()); + locked = std::move(result.get()); + ASSERT_NE(locked, nullptr); + url = test_bookmark_url(root.url, key); + } +}; + +::testing::AssertionResult bookmark_bytes_equal(const BookmarkLock& lock, NSData* expected) { + auto result = read_bookmark(lock); + if (!result.ok()) + return ::testing::AssertionFailure() << "Bookmark read error " << int(result.error()); + if (expected == nil ? result.get() != nil : ![result.get() isEqual:expected]) { + return ::testing::AssertionFailure() << "Bookmark bytes differ"; + } + return ::testing::AssertionSuccess(); +} + +::testing::AssertionResult no_temporary_bookmarks(NSURL* root) { + NSURL* directory = [root URLByAppendingPathComponent:@"bookmarks"]; + FileDescriptor fd(open(directory.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + if (fd.get() < 0) return ::testing::AssertionFailure() << "open bookmark directory: " << errno; + auto children = storage_children(fd.get()); + if (!children.ok()) return ::testing::AssertionFailure() << "Cannot enumerate bookmark directory"; + for (NSString* name in children.get()) { + if ([name hasPrefix:@".tmp-"]) + return ::testing::AssertionFailure() << "Leaked " << name.UTF8String; + } + return ::testing::AssertionSuccess(); +} + +TEST_F(CoreAIBookmarkIdentityTest, IgnoresFunctionBindingsAndDeliveryLocation) { + EXPECT_EQ(key.length, 64u); + manifest.function = @"another_function"; + manifest.inputs = @[ @"different_input" ]; + manifest.outputs = @[ @"different_output" ]; + manifest.hash = @"another_delivery_location"; + auto changed = test_bookmark_key(manifest); + ASSERT_TRUE(changed.ok()); + EXPECT_TRUE([changed.get() isEqual:key]); +} + +TEST_F(CoreAIBookmarkIdentityTest, KeySeparatesPlatformArchitectureWeightsAndAotContext) { + auto platform = bookmark_key(manifest, @"iOS", @"arch_b"); + auto architecture = bookmark_key(manifest, @"macOS", @"arch_a"); + Manifest reweighted = manifest; + reweighted.bundle_digests = @{@"model.aimodel" : fixture_identity('2')}; + auto weights = test_bookmark_key(reweighted); + auto aot_manifest = bookmark_manifest(true); + ASSERT_TRUE(aot_manifest.ok()); + auto aot = test_bookmark_key(aot_manifest.get()); + for (auto* changed : {&platform, &architecture, &weights, &aot}) { + ASSERT_TRUE(changed->ok()); + EXPECT_FALSE([changed->get() isEqual:key]); + } +} + +TEST_F(CoreAIBookmarkIdentityTest, RejectsMissingArchitecture) { + EXPECT_FALSE(bookmark_key(manifest, @"macOS", nil).ok()); + EXPECT_FALSE(bookmark_key(manifest, @"macOS", @"").ok()); +} + +TEST_F(CoreAIBookmarkIdentityTest, RejectsUnselectedBundle) { + manifest.path = @"../escape"; + EXPECT_FALSE(test_bookmark_key(manifest).ok()); +} + +TEST_F(CoreAIBookmarkRawTest, MissingBookmarkIsAnEmptySuccessfulRead) { + EXPECT_TRUE(bookmark_bytes_equal(*locked, nil)); + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:url.path]); +} + +TEST_F(CoreAIBookmarkRawTest, RoundTripsMaximumBookmarkAndReopens) { + auto bytes = sized_bookmark(8 * kMiB); + ASSERT_TRUE(bytes.ok()); + ASSERT_EQ(write_bookmark(*locked, bytes.get()), Error::Ok); + EXPECT_TRUE(bookmark_bytes_equal(*locked, bytes.get())); + EXPECT_TRUE([[NSData dataWithContentsOfURL:url] isEqual:bytes.get()]); + locked.reset(); + auto saved = saved_bookmark(root.url, key); + ASSERT_TRUE(saved.ok()); + EXPECT_TRUE([saved.get() isEqual:bytes.get()]); +} + +TEST_F(CoreAIBookmarkRawTest, InvalidWritesPreserveMaximumBookmark) { + auto maximum = sized_bookmark(8 * kMiB); + ASSERT_TRUE(maximum.ok()); + ASSERT_EQ(write_bookmark(*locked, maximum.get()), Error::Ok); + auto oversized = sized_bookmark(8 * kMiB + 1); + ASSERT_TRUE(oversized.ok()); + for (NSData* invalid : {[NSData data], oversized.get()}) { + SCOPED_TRACE(invalid.length); + EXPECT_EQ(write_bookmark(*locked, invalid), Error::InvalidExternalData); + EXPECT_TRUE([[NSData dataWithContentsOfURL:url] isEqual:maximum.get()]); + EXPECT_TRUE(bookmark_bytes_equal(*locked, maximum.get())); + EXPECT_TRUE(no_temporary_bookmarks(root.url)); + } +} + +TEST_F(CoreAIBookmarkRawTest, RejectsOversizedStoredBookmark) { + FileDescriptor file( + open(url.fileSystemRepresentation, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); + ASSERT_GE(file.get(), 0); + ASSERT_EQ(ftruncate(file.get(), 8 * kMiB + 1), 0); + ASSERT_EQ(close(file.release()), 0); + EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); + struct stat info {}; + ASSERT_EQ(lstat(url.fileSystemRepresentation, &info), 0); + EXPECT_EQ(info.st_size, static_cast(8 * kMiB + 1)); +} + +TEST_F(CoreAIBookmarkPathTest, AllowsTemporaryRootsAndRejectsInvalidKeys) { + auto allowed = lock_bookmark(root.url.path, key); + ASSERT_TRUE(allowed.ok()); + ASSERT_NE(allowed.get(), nullptr); + EXPECT_FALSE(lock_bookmark(root.url.path, @"../escape").ok()); +} + +TEST_F(CoreAIBookmarkPathTest, ExcludesRootWithoutCreatingStaging) { + auto locked = lock_bookmark(root.url.path, key); + ASSERT_TRUE(locked.ok()); + EXPECT_TRUE(backup_excluded(root.url)); + EXPECT_FALSE([NSFileManager.defaultManager + fileExistsAtPath:[root.url URLByAppendingPathComponent:@"staging"].path]); +} + +TEST_F(CoreAIBookmarkPathTest, ProcessContentionAllowsIndependentKeyAndRecoversAfterHolderDeath) { + BookmarkChild holder; + ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); + ASSERT_TRUE(holder.receive('L')); + NSURL* file = lock_url(); + struct stat before {}; + ASSERT_EQ(stat(file.fileSystemRepresentation, &before), 0); + BookmarkChild waiter; + ASSERT_TRUE(waiter.spawn(@"--bookmark-lock", root.url.path, @"same")); + ASSERT_TRUE(waiter.receive('B')); + ASSERT_TRUE(waiter.expect_blocked()); + BookmarkChild independent; + ASSERT_TRUE(independent.spawn(@"--bookmark-lock", root.url.path, @"other")); + ASSERT_TRUE(independent.receive('L')); + ASSERT_TRUE(independent.expect_exit(0)); + ASSERT_TRUE(holder.kill_and_reap()); + ASSERT_TRUE(waiter.receive('L')); + ASSERT_TRUE(waiter.expect_exit(0)); + struct stat after {}; + ASSERT_EQ(stat(file.fileSystemRepresentation, &after), 0); + EXPECT_EQ(before.st_dev, after.st_dev); + EXPECT_EQ(before.st_ino, after.st_ino); +} + +} // namespace diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h index 625f1f48a8a..8a09e5068ac 100644 --- a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h @@ -21,6 +21,14 @@ struct TestDirectory { TestDirectory& operator=(const TestDirectory&) = delete; }; +struct BookmarkDirectory { + NSURL* url = nil; + explicit BookmarkDirectory(NSString* parent = @"/private/tmp"); + ~BookmarkDirectory(); + BookmarkDirectory(const BookmarkDirectory&) = delete; + BookmarkDirectory& operator=(const BookmarkDirectory&) = delete; +}; + ::testing::AssertionResult backup_excluded(NSURL* url); ::testing::AssertionResult set_backup_excluded(NSURL* url, bool excluded); diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm index 6c60dddd6c5..7068b96e612 100644 --- a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm @@ -8,6 +8,7 @@ #include "coreai_filesystem_fixture.h" #include +#include namespace executorch::backends::coreai::testing { @@ -68,4 +69,25 @@ << "Backup flag did not settle: " << url.path.UTF8String; } +BookmarkDirectory::BookmarkDirectory(NSString* parent) { + if (parent == nil) { + ADD_FAILURE() << "Bookmark directory requires a parent"; + return; + } + std::string pattern = + std::string(parent.fileSystemRepresentation) + "/.coreai-bookmark-test-XXXXXX"; + if (mkdtemp(pattern.data()) == nullptr) { + ADD_FAILURE() << "mkdtemp: " << strerror(errno); + return; + } + url = [NSURL fileURLWithPath:[NSString stringWithUTF8String:pattern.c_str()] isDirectory:YES]; +} + +BookmarkDirectory::~BookmarkDirectory() { + if (url == nil) return; + EXPECT_TRUE([url.lastPathComponent hasPrefix:@".coreai-bookmark-test-"]); + if (![url.lastPathComponent hasPrefix:@".coreai-bookmark-test-"]) return; + EXPECT_TRUE([NSFileManager.defaultManager removeItemAtURL:url error:nil]); +} + } // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_host_test.mm b/backends/apple/coreai/runtime/test/coreai_host_test.mm index 5c857dfddf4..ebf392c060a 100644 --- a/backends/apple/coreai/runtime/test/coreai_host_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_host_test.mm @@ -9,10 +9,14 @@ #import #include #include +#include "coreai_bookmark_fixture.h" int main(int argc, char** argv) { executorch::runtime::runtime_init(); @autoreleasepool { + executorch::backends::coreai::testing::set_bookmark_test_executable(argv[0]); + const int child_result = executorch::backends::coreai::testing::bookmark_child_mode(argc, argv); + if (child_result >= 0) return child_result; ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } From 29cb5f05c3d15eac95e74913dc62dd0899d9865e Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Mon, 26 Oct 2026 14:56:54 -0700 Subject: [PATCH 2/6] [CoreAI] Address bookmark persistence review feedback Exercise the maintenance and failure cases that later cache operations depend on, while preserving the existing staging contract. Handle interrupted reads consistently and remove redundant setup and validation work. Test Plan: Ran the native host suite on macOS 27: 61 tests passed; one existing test skipped because APFS rejects invalid UTF-8 filenames. The new bookmark tests covered noncreating lookups, staging, invalid stored files, and thread locking. The thread-contention test also passed 20 consecutive runs. Authored with assistance from Devmate (OpenAI GPT-6-Astra). --- .../apple/coreai/runtime/coreai_bookmarks.h | 3 - .../apple/coreai/runtime/coreai_bookmarks.mm | 44 ++-- .../runtime/test/coreai_bookmark_test.mm | 216 ++++++++++++++++++ .../runtime/test/coreai_filesystem_fixture.h | 8 +- .../runtime/test/coreai_filesystem_fixture.mm | 22 -- 5 files changed, 241 insertions(+), 52 deletions(-) diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.h b/backends/apple/coreai/runtime/coreai_bookmarks.h index fa838985fd7..5e91e61d396 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.h +++ b/backends/apple/coreai/runtime/coreai_bookmarks.h @@ -27,9 +27,6 @@ class BookmarkLock final { public: BookmarkLock(const BookmarkLock&) = delete; BookmarkLock& operator=(const BookmarkLock&) = delete; - NSString* root_path() const { - return root_path_; - } private: friend runtime::Result> diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.mm b/backends/apple/coreai/runtime/coreai_bookmarks.mm index 0ef1fd972f2..614e9153c50 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.mm +++ b/backends/apple/coreai/runtime/coreai_bookmarks.mm @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -73,20 +72,6 @@ bool digest(id value) { } namespace { -Result resolve_root(NSString* path, bool create) { - ET_CHECK_OR_RETURN_ERROR(text(path) && path.isAbsolutePath, InvalidArgument, - "Core AI assets require an absolute root path"); - if (!create) { - struct stat info; - const int result = - retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); - if (result < 0 && errno == ENOENT) return path; - ET_CHECK_OR_RETURN_ERROR(result == 0 && S_ISDIR(info.st_mode), - AccessFailed, "Invalid Core AI assets root"); - } - return prepare_storage_root(path, create); -} - // Returns -1 when the root does not exist. Result open_root(NSString* root) { const int fd = retry_eintr([&] { @@ -146,7 +131,23 @@ bool digest(id value) { } // namespace Result resolve_bookmark_root(NSString* path, bool create) { - return resolve_root(path, create); + ET_CHECK_OR_RETURN_ERROR( + text(path) && path.isAbsolutePath, + InvalidArgument, + "Core AI assets require an absolute root path"); + if (create) { + return prepare_storage_root(path, true); + } + struct stat info; + const int result = + retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); + if (result < 0 && errno == ENOENT) + return path; + ET_CHECK_OR_RETURN_ERROR( + result == 0 && S_ISDIR(info.st_mode), + AccessFailed, + "Invalid Core AI assets root"); + return path; } BookmarkLock::BookmarkLock(int root, int bookmarks, int file, @@ -162,7 +163,7 @@ bool digest(id value) { bool create) { ET_CHECK_OR_RETURN_ERROR(digest(key), InvalidArgument, "Invalid Core AI bookmark key"); - auto resolved = resolve_root(root, create); + auto resolved = resolve_bookmark_root(root, create); if (!resolved.ok()) return resolved.error(); root = resolved.get(); auto opened = open_root(root); @@ -201,9 +202,12 @@ new BookmarkLock(directory.release(), bookmarks.release(), file.release(), Result read_bookmark(const BookmarkLock& lock) { if (lock.bookmarks_.get() < 0) return static_cast(nil); - FileDescriptor file(openat(lock.bookmarks_.get(), - bookmark_name(lock.key_).fileSystemRepresentation, - O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC)); + FileDescriptor file(retry_eintr([&] { + return openat( + lock.bookmarks_.get(), + bookmark_name(lock.key_).fileSystemRepresentation, + O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC); + })); if (file.get() < 0 && errno == ENOENT) return static_cast(nil); ET_CHECK_OR_RETURN_ERROR(file.get() >= 0, AccessFailed, "Cannot open Core AI bookmark"); diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm index 1cce13d973a..ff8c4e25996 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm @@ -6,7 +6,9 @@ * LICENSE file in the root directory of this source tree. */ +#include #include +#include #include #include #include "coreai_bookmark_fixture.h" @@ -156,6 +158,101 @@ void SetUp() override { } } +TEST_F(CoreAIBookmarkRawTest, RejectsEmptyStoredBookmark) { + FileDescriptor file(open( + url.fileSystemRepresentation, + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, + 0600)); + ASSERT_GE(file.get(), 0); + EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); +} + +TEST_F(CoreAIBookmarkRawTest, RejectsSymlinkedBookmarkWithoutReadingTarget) { + NSURL* target = [root.url URLByAppendingPathComponent:@"target"]; + NSData* bytes = [@"target bytes" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_TRUE([bytes writeToURL:target atomically:NO]); + ASSERT_EQ( + symlink(target.fileSystemRepresentation, url.fileSystemRepresentation), + 0); + EXPECT_EQ(read_bookmark(*locked).error(), Error::AccessFailed); + EXPECT_TRUE(([[NSData dataWithContentsOfURL:target] isEqual:bytes])); +} + +TEST_F(CoreAIBookmarkRawTest, RejectsDirectoryAtBookmarkPath) { + ASSERT_EQ(mkdir(url.fileSystemRepresentation, 0700), 0); + EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); +} + +TEST_F(CoreAIBookmarkRawTest, RejectsFifoWithoutWaitingForWriter) { + ASSERT_EQ(mkfifo(url.fileSystemRepresentation, 0600), 0); + EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); +} + +TEST_F(CoreAIBookmarkRawTest, RejectsInaccessibleBookmark) { + if (geteuid() == 0) { + GTEST_SKIP() << "Root can bypass file read permissions"; + } + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(write_bookmark(*locked, bytes), Error::Ok); + ASSERT_EQ(chmod(url.fileSystemRepresentation, 0000), 0); + EXPECT_EQ(read_bookmark(*locked).error(), Error::AccessFailed); + ASSERT_EQ(chmod(url.fileSystemRepresentation, 0600), 0); + EXPECT_TRUE(bookmark_bytes_equal(*locked, bytes)); +} + +TEST_F(CoreAIBookmarkRawTest, NoncreatingLookupReadsExistingBookmark) { + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(write_bookmark(*locked, bytes), Error::Ok); + locked.reset(); + auto existing = lock_bookmark(root.url.path, key, false); + ASSERT_TRUE(existing.ok()); + ASSERT_NE(existing.get(), nullptr); + EXPECT_TRUE(bookmark_bytes_equal(*existing.get(), bytes)); + EXPECT_FALSE([NSFileManager.defaultManager + fileExistsAtPath:[root.url URLByAppendingPathComponent:@"staging"].path]); +} + +TEST_F(CoreAIBookmarkRawTest, PreparesSharedStagingParentOnDemand) { + NSString* expected = [root.url URLByAppendingPathComponent:@"staging"].path; + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:expected]); + auto staging = prepare_bookmark_staging(*locked); + ASSERT_TRUE(staging.ok()); + EXPECT_TRUE([staging.get() isEqual:expected]); + struct stat info{}; + ASSERT_EQ(lstat(staging.get().fileSystemRepresentation, &info), 0); + EXPECT_TRUE(S_ISDIR(info.st_mode)); + auto repeated = prepare_bookmark_staging(*locked); + ASSERT_TRUE(repeated.ok()); + EXPECT_TRUE([repeated.get() isEqual:expected]); + EXPECT_FALSE([NSFileManager.defaultManager + fileExistsAtPath:[expected stringByAppendingPathComponent:key]]); +} + +TEST_F(CoreAIBookmarkRawTest, RejectsSymlinkedStagingParent) { + TestDirectory target; + ASSERT_NE(target.url, nil); + NSURL* staging = [root.url URLByAppendingPathComponent:@"staging"]; + ASSERT_EQ( + symlink( + target.url.fileSystemRepresentation, + staging.fileSystemRepresentation), + 0); + EXPECT_EQ(prepare_bookmark_staging(*locked).error(), Error::AccessFailed); + NSArray* contents = + [NSFileManager.defaultManager contentsOfDirectoryAtPath:target.url.path + error:nil]; + ASSERT_NE(contents, nil); + EXPECT_EQ(contents.count, 0u); +} + +TEST_F(CoreAIBookmarkRawTest, RejectsFileAtStagingParent) { + NSURL* staging = [root.url URLByAppendingPathComponent:@"staging"]; + NSData* bytes = [@"not a directory" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_TRUE([bytes writeToURL:staging atomically:NO]); + EXPECT_EQ(prepare_bookmark_staging(*locked).error(), Error::AccessFailed); + EXPECT_TRUE(([[NSData dataWithContentsOfURL:staging] isEqual:bytes])); +} + TEST_F(CoreAIBookmarkRawTest, RejectsOversizedStoredBookmark) { FileDescriptor file( open(url.fileSystemRepresentation, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); @@ -183,6 +280,125 @@ FileDescriptor file( fileExistsAtPath:[root.url URLByAppendingPathComponent:@"staging"].path]); } +TEST_F( + CoreAIBookmarkPathTest, + NoncreatingLookupLeavesMissingRootAndEntryUntouched) { + ASSERT_TRUE(set_backup_excluded(root.url, false)); + NSString* missing = [root.url URLByAppendingPathComponent:@"missing"].path; + for (NSString* path in @[ root.url.path, missing ]) { + SCOPED_TRACE(path.UTF8String); + auto resolved = resolve_bookmark_root(path, false); + ASSERT_TRUE(resolved.ok()); + EXPECT_TRUE([resolved.get() isEqual:path]); + auto absent = lock_bookmark(path, key, false); + ASSERT_TRUE(absent.ok()); + EXPECT_EQ(absent.get(), nullptr); + } + NSArray* contents = + [NSFileManager.defaultManager contentsOfDirectoryAtPath:root.url.path + error:nil]; + ASSERT_NE(contents, nil); + EXPECT_EQ(contents.count, 0u); + EXPECT_FALSE(backup_excluded(root.url)); +} + +TEST_F(CoreAIBookmarkPathTest, NoncreatingLookupIgnoresOtherEntries) { + NSString* other_key = fixture_identity('f'); + auto other = lock_bookmark(root.url.path, other_key); + ASSERT_TRUE(other.ok()); + ASSERT_NE(other.get(), nullptr); + NSData* bytes = [@"other" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(write_bookmark(*other.get(), bytes), Error::Ok); + auto absent = lock_bookmark(root.url.path, key, false); + ASSERT_TRUE(absent.ok()); + EXPECT_EQ(absent.get(), nullptr); + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); + EXPECT_TRUE(bookmark_bytes_equal(*other.get(), bytes)); +} + +TEST_F(CoreAIBookmarkPathTest, NoncreatingLookupLocksStagingOnlyEntry) { + NSURL* staged = [[root.url URLByAppendingPathComponent:@"staging"] + URLByAppendingPathComponent:key]; + ASSERT_TRUE([NSFileManager.defaultManager createDirectoryAtURL:staged + withIntermediateDirectories:YES + attributes:nil + error:nil]); + auto existing = lock_bookmark(root.url.path, key, false); + ASSERT_TRUE(existing.ok()); + ASSERT_NE(existing.get(), nullptr); + EXPECT_TRUE(bookmark_bytes_equal(*existing.get(), nil)); + EXPECT_TRUE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); + EXPECT_FALSE([NSFileManager.defaultManager + fileExistsAtPath:[root.url URLByAppendingPathComponent:@"bookmarks"] + .path]); +} + +TEST_F(CoreAIBookmarkPathTest, ResolvesAndCreatesRootWithBackupExclusion) { + ASSERT_TRUE(set_backup_excluded(root.url, false)); + NSString* path = [root.url URLByAppendingPathComponent:@"nested/assets"].path; + auto resolved = resolve_bookmark_root(path); + ASSERT_TRUE(resolved.ok()); + EXPECT_TRUE([resolved.get() isEqual:path]); + EXPECT_TRUE(backup_excluded([NSURL fileURLWithPath:resolved.get()])); + EXPECT_FALSE(backup_excluded(root.url)); + auto existing = resolve_bookmark_root(path, false); + ASSERT_TRUE(existing.ok()); + EXPECT_TRUE([existing.get() isEqual:path]); +} + +TEST_F(CoreAIBookmarkPathTest, RejectsInvalidResolvedRoots) { + for (bool create : {false, true}) { + EXPECT_EQ( + resolve_bookmark_root(@"relative", create).error(), + Error::InvalidArgument); + EXPECT_EQ( + resolve_bookmark_root(nil, create).error(), Error::InvalidArgument); + } + NSURL* file = [root.url URLByAppendingPathComponent:@"file"]; + ASSERT_TRUE([[NSData data] writeToURL:file atomically:NO]); + EXPECT_EQ( + resolve_bookmark_root(file.path, false).error(), Error::AccessFailed); + EXPECT_EQ( + resolve_bookmark_root( + [file.path stringByAppendingPathComponent:@"child"], false) + .error(), + Error::AccessFailed); +} + +TEST_F( + CoreAIBookmarkPathTest, + ThreadContentionSerializesSameKeyButNotDifferentKeys) { + auto holder = lock_bookmark(root.url.path, key); + ASSERT_TRUE(holder.ok()); + ASSERT_NE(holder.get(), nullptr); + auto acquire = [path = root.url.path](NSString* next_key) { + @autoreleasepool { + auto result = lock_bookmark(path, next_key); + if (!result.ok()) + return result.error(); + return result.get() != nullptr ? Error::Ok : Error::Internal; + } + }; + std::promise started; + auto starting = started.get_future(); + auto waiter = std::async(std::launch::async, [&] { + started.set_value(); + return acquire(key); + }); + EXPECT_EQ( + starting.wait_for(std::chrono::seconds(5)), std::future_status::ready); + EXPECT_EQ( + waiter.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + auto independent = + std::async(std::launch::async, acquire, fixture_identity('f')); + EXPECT_EQ( + independent.wait_for(std::chrono::seconds(5)), std::future_status::ready); + holder.get().reset(); + EXPECT_EQ(waiter.get(), Error::Ok); + EXPECT_EQ(independent.get(), Error::Ok); +} + TEST_F(CoreAIBookmarkPathTest, ProcessContentionAllowsIndependentKeyAndRecoversAfterHolderDeath) { BookmarkChild holder; ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h index 8a09e5068ac..f01a9ad6d3b 100644 --- a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h @@ -21,13 +21,7 @@ struct TestDirectory { TestDirectory& operator=(const TestDirectory&) = delete; }; -struct BookmarkDirectory { - NSURL* url = nil; - explicit BookmarkDirectory(NSString* parent = @"/private/tmp"); - ~BookmarkDirectory(); - BookmarkDirectory(const BookmarkDirectory&) = delete; - BookmarkDirectory& operator=(const BookmarkDirectory&) = delete; -}; +using BookmarkDirectory = TestDirectory; ::testing::AssertionResult backup_excluded(NSURL* url); ::testing::AssertionResult set_backup_excluded(NSURL* url, bool excluded); diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm index 7068b96e612..6c60dddd6c5 100644 --- a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.mm @@ -8,7 +8,6 @@ #include "coreai_filesystem_fixture.h" #include -#include namespace executorch::backends::coreai::testing { @@ -69,25 +68,4 @@ << "Backup flag did not settle: " << url.path.UTF8String; } -BookmarkDirectory::BookmarkDirectory(NSString* parent) { - if (parent == nil) { - ADD_FAILURE() << "Bookmark directory requires a parent"; - return; - } - std::string pattern = - std::string(parent.fileSystemRepresentation) + "/.coreai-bookmark-test-XXXXXX"; - if (mkdtemp(pattern.data()) == nullptr) { - ADD_FAILURE() << "mkdtemp: " << strerror(errno); - return; - } - url = [NSURL fileURLWithPath:[NSString stringWithUTF8String:pattern.c_str()] isDirectory:YES]; -} - -BookmarkDirectory::~BookmarkDirectory() { - if (url == nil) return; - EXPECT_TRUE([url.lastPathComponent hasPrefix:@".coreai-bookmark-test-"]); - if (![url.lastPathComponent hasPrefix:@".coreai-bookmark-test-"]) return; - EXPECT_TRUE([NSFileManager.defaultManager removeItemAtURL:url error:nil]); -} - } // namespace executorch::backends::coreai::testing From d5cdc8da351a01c084fc226459c751cad2a20695 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Tue, 27 Oct 2026 12:21:59 -0700 Subject: [PATCH 3/6] [CoreAI] Recheck bookmark state under lock and strengthen contention tests Summary: Prevent waiting callers from using stale bookmark state after eviction. Make staging-only write failures explicit and ensure contention tests reach lock acquisition without hanging teardown. Clarify maintenance side effects and key identity. Test Plan: All 74 native tests passed with zero skips, including eviction-waiter and staging-only write regressions. Four contention/waiter tests passed 25 repetitions. All 113 CoreAI Python tests passed. Formatting and whitespace checks passed. --- backends/apple/coreai/README.md | 14 +- .../apple/coreai/runtime/coreai_bookmarks.h | 3 + .../apple/coreai/runtime/coreai_bookmarks.mm | 217 +++++++++++------- .../runtime/test/coreai_bookmark_fixture.mm | 194 ++++++++++------ .../runtime/test/coreai_bookmark_test.mm | 169 ++++++++++---- .../runtime/test/coreai_filesystem_fixture.h | 2 - .../coreai/runtime/test/coreai_host_test.mm | 9 +- 7 files changed, 406 insertions(+), 202 deletions(-) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 323ad97bac3..6f2bf8be1e0 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -82,13 +82,13 @@ 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. -Each partition derives a key from its selected export digest/bundle, platform, -Core AI device architecture, default SDK cache namespace, versioned default -options and persistent policy. Function bindings and delivery location are -excluded. No model bytes are hashed at runtime. Each key determines a raw -bookmark in the `bookmarks` subdirectory, a bundle directory in `staging`, and an -empty coordination file in `locks`. There is no PTE-wide bookmark list or -serialized lifecycle record. +Each partition derives a key from its selected export digest/bundle, source/AOT +mode, platform, Core AI device architecture, default SDK cache namespace, +versioned default options and persistent policy. Function bindings and delivery +location are excluded. No model bytes are hashed at runtime. Each key determines +a raw bookmark in `bookmarks`, a bundle directory in `staging`, and an empty +coordination file in `locks`. There is no PTE-wide bookmark list or serialized +lifecycle record. Raw bookmarks have an 8 MiB backend allocation limit, not an asserted SDK format maximum, and are published atomically because losing one orphans an SDK entry. diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.h b/backends/apple/coreai/runtime/coreai_bookmarks.h index 5e91e61d396..e0fb78d2b7a 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.h +++ b/backends/apple/coreai/runtime/coreai_bookmarks.h @@ -50,11 +50,14 @@ class BookmarkLock final { // Each acquisition opens an independent descriptor; the backend never removes // lock files. With create=false, missing roots or entries return a null lock. +// Existing entries may require creating coordination files, so the root must +// be writable even with create=false. runtime::Result> lock_bookmark(NSString* root, NSString* key, bool create = true); // Missing is nil. Empty, oversized, inaccessible or nonregular files fail // closed. runtime::Result read_bookmark(const BookmarkLock& lock); +// A staging-only lock has no bookmark directory; writes return InvalidArgument. runtime::Error write_bookmark(const BookmarkLock& lock, NSData* data); // Prepared only for cold loads; source preparation appends key/bundle. runtime::Result prepare_bookmark_staging(const BookmarkLock& lock); diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.mm b/backends/apple/coreai/runtime/coreai_bookmarks.mm index 614e9153c50..7c63b1a5a1b 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.mm +++ b/backends/apple/coreai/runtime/coreai_bookmarks.mm @@ -24,29 +24,30 @@ bool text(id value) { return [value isKindOfClass:NSString.class] && [value length] > 0 && - [value length] <= 4096 && - [value rangeOfString:@"\0"].location == NSNotFound; + [value length] <= 4096 && + [value rangeOfString:@"\0"].location == NSNotFound; } bool component(id value) { return text(value) && ![value containsString:@"/"] && - ![value containsString:@"\\"] && ![value isEqual:@"."] && - ![value isEqual:@".."]; + ![value containsString:@"\\"] && ![value isEqual:@"."] && + ![value isEqual:@".."]; } bool digest(id value) { return text(value) && [value length] == 64 && - [value rangeOfCharacterFromSet: - [[NSCharacterSet - characterSetWithCharactersInString:@"0123456789abcdef"] - invertedSet]] - .location == NSNotFound; + [value rangeOfCharacterFromSet:[[NSCharacterSet + characterSetWithCharactersInString: + @"0123456789abcdef"] invertedSet]] + .location == NSNotFound; } NSString* bookmark_name(NSString* key) { return [key stringByAppendingString:@".bookmark"]; } -} // namespace +} // namespace -Result bookmark_key(const Manifest& selected, NSString* platform, - NSString* architecture) { +Result bookmark_key( + const Manifest& selected, + NSString* platform, + NSString* architecture) { NSString* bundle = selected.path.lastPathComponent; NSString* exported_digest = selected.bundle_digests[bundle]; ET_CHECK_OR_RETURN_ERROR( @@ -55,19 +56,26 @@ bool digest(id value) { DelegateInvalidCompatibility, "Core AI bookmark key requires a selected bundle and SDK architecture"); NSArray* fields = @[ - @"1", exported_digest, selected.aot_compiled ? @"aot" : @"source", bundle, - platform, architecture, @"AIModelCache.default", @"default-options-v1", + @"1", + exported_digest, + selected.aot_compiled ? @"aot" : @"source", + bundle, + platform, + architecture, + @"AIModelCache.default", + @"default-options-v1", @"persistent" ]; NSData* encoded = [NSJSONSerialization dataWithJSONObject:fields options:0 error:nil]; - ET_CHECK_OR_RETURN_ERROR(encoded != nil && encoded.length <= UINT32_MAX, - InvalidExternalData, "Cannot encode bookmark key"); + ET_CHECK_OR_RETURN_ERROR( + encoded != nil, InvalidExternalData, "Cannot encode bookmark key"); unsigned char bytes[CC_SHA256_DIGEST_LENGTH]; CC_SHA256(encoded.bytes, static_cast(encoded.length), bytes); NSMutableString* key = [NSMutableString stringWithCapacity:64]; - for (unsigned char byte : bytes) [key appendFormat:@"%02x", byte]; + for (unsigned char byte : bytes) + [key appendFormat:@"%02x", byte]; return [key copy]; } @@ -75,12 +83,13 @@ bool digest(id value) { // Returns -1 when the root does not exist. Result open_root(NSString* root) { const int fd = retry_eintr([&] { - return open(root.fileSystemRepresentation, - O_RDONLY | O_DIRECTORY | O_CLOEXEC); + return open( + root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC); }); - if (fd < 0 && errno == ENOENT) return -1; - ET_CHECK_OR_RETURN_ERROR(fd >= 0, AccessFailed, - "Cannot open Core AI assets root"); + if (fd < 0 && errno == ENOENT) + return -1; + ET_CHECK_OR_RETURN_ERROR( + fd >= 0, AccessFailed, "Cannot open Core AI assets root"); return fd; } @@ -88,20 +97,24 @@ bool digest(id value) { Result open_child(int root_fd, NSString* name, bool create) { bool created = false; if (create) { - const int made = retry_eintr([&] { - return mkdirat(root_fd, name.fileSystemRepresentation, 0700); - }); - ET_CHECK_OR_RETURN_ERROR(made == 0 || errno == EEXIST, AccessFailed, - "Cannot create Core AI assets directory"); + const int made = retry_eintr( + [&] { return mkdirat(root_fd, name.fileSystemRepresentation, 0700); }); + ET_CHECK_OR_RETURN_ERROR( + made == 0 || errno == EEXIST, + AccessFailed, + "Cannot create Core AI assets directory"); created = made == 0; } const int fd = retry_eintr([&] { - return openat(root_fd, name.fileSystemRepresentation, - O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + return openat( + root_fd, + name.fileSystemRepresentation, + O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); }); - if (fd < 0 && errno == ENOENT && !create) return -1; - ET_CHECK_OR_RETURN_ERROR(fd >= 0, AccessFailed, - "Invalid Core AI assets directory"); + if (fd < 0 && errno == ENOENT && !create) + return -1; + ET_CHECK_OR_RETURN_ERROR( + fd >= 0, AccessFailed, "Invalid Core AI assets directory"); if (created) { FileDescriptor child(fd); ET_CHECK_OK_OR_RETURN_ERROR(sync_storage_directory(root_fd)); @@ -113,26 +126,32 @@ bool digest(id value) { Result entry_exists(int root_fd, NSString* key) { for (NSString* name in @[ @"bookmarks", @"staging" ]) { auto opened = open_child(root_fd, name, false); - if (!opened.ok()) return opened.error(); + if (!opened.ok()) + return opened.error(); FileDescriptor child(opened.get()); - if (child.get() < 0) continue; + if (child.get() < 0) + continue; NSString* entry = [name isEqual:@"bookmarks"] ? bookmark_name(key) : key; struct stat info; const int result = retry_eintr([&] { - return fstatat(child.get(), entry.fileSystemRepresentation, &info, - AT_SYMLINK_NOFOLLOW); + return fstatat( + child.get(), + entry.fileSystemRepresentation, + &info, + AT_SYMLINK_NOFOLLOW); }); - if (result == 0) return true; - ET_CHECK_OR_RETURN_ERROR(errno == ENOENT, AccessFailed, - "Cannot inspect Core AI cache entry"); + if (result == 0) + return true; + ET_CHECK_OR_RETURN_ERROR( + errno == ENOENT, AccessFailed, "Cannot inspect Core AI cache entry"); } return false; } -} // namespace +} // namespace Result resolve_bookmark_root(NSString* path, bool create) { ET_CHECK_OR_RETURN_ERROR( - text(path) && path.isAbsolutePath, + text(path) && [path hasPrefix:@"/"], InvalidArgument, "Core AI assets require an absolute root path"); if (create) { @@ -150,82 +169,112 @@ bool digest(id value) { return path; } -BookmarkLock::BookmarkLock(int root, int bookmarks, int file, - NSString* root_path, NSString* key) +BookmarkLock::BookmarkLock( + int root, + int bookmarks, + int file, + NSString* root_path, + NSString* key) : root_(root), bookmarks_(bookmarks), file_(file), root_path_(root_path), key_(key) {} -Result> lock_bookmark(NSString* root, - NSString* key, - bool create) { - ET_CHECK_OR_RETURN_ERROR(digest(key), InvalidArgument, - "Invalid Core AI bookmark key"); +Result> +lock_bookmark(NSString* root, NSString* key, bool create) { + ET_CHECK_OR_RETURN_ERROR( + digest(key), InvalidArgument, "Invalid Core AI bookmark key"); auto resolved = resolve_bookmark_root(root, create); - if (!resolved.ok()) return resolved.error(); + if (!resolved.ok()) + return resolved.error(); root = resolved.get(); auto opened = open_root(root); - if (!opened.ok()) return opened.error(); + if (!opened.ok()) + return opened.error(); FileDescriptor directory(opened.get()); if (directory.get() < 0) { - ET_CHECK_OR_RETURN_ERROR(!create, AccessFailed, - "Core AI assets root disappeared"); + ET_CHECK_OR_RETURN_ERROR( + !create, AccessFailed, "Core AI assets root disappeared"); return std::unique_ptr(); } if (!create) { auto exists = entry_exists(directory.get(), key); - if (!exists.ok()) return exists.error(); - if (!exists.get()) return std::unique_ptr(); + if (!exists.ok()) + return exists.error(); + if (!exists.get()) + return std::unique_ptr(); } - auto bookmark_result = open_child(directory.get(), @"bookmarks", create); - if (!bookmark_result.ok()) return bookmark_result.error(); - FileDescriptor bookmarks(bookmark_result.get()); auto lock_result = open_child(directory.get(), @"locks", true); - if (!lock_result.ok()) return lock_result.error(); + if (!lock_result.ok()) + return lock_result.error(); FileDescriptor locks(lock_result.get()); NSString* name = [key stringByAppendingString:@".lock"]; FileDescriptor file( open_storage_shared_file(locks.get(), name.fileSystemRepresentation)); struct stat info; - ET_CHECK_OR_RETURN_ERROR(file.get() >= 0 && fstat(file.get(), &info) == 0 && - S_ISREG(info.st_mode), - AccessFailed, "Invalid Core AI bookmark lock"); + ET_CHECK_OR_RETURN_ERROR( + file.get() >= 0 && fstat(file.get(), &info) == 0 && S_ISREG(info.st_mode), + AccessFailed, + "Invalid Core AI bookmark lock"); + ET_CHECK_OR_RETURN_ERROR( + storage_fault(StorageOperation::BeforeBookmarkLock) == 0, + AccessFailed, + "Cannot begin Core AI bookmark lock acquisition"); ET_CHECK_OR_RETURN_ERROR( retry_eintr([&] { return flock(file.get(), LOCK_EX); }) == 0, - AccessFailed, "Cannot lock Core AI bookmark"); - return std::unique_ptr( - new BookmarkLock(directory.release(), bookmarks.release(), file.release(), - root, [key copy])); + AccessFailed, + "Cannot lock Core AI bookmark"); + if (!create) { + auto exists = entry_exists(directory.get(), key); + if (!exists.ok()) + return exists.error(); + if (!exists.get()) + return std::unique_ptr(); + } + auto bookmark_result = open_child(directory.get(), @"bookmarks", create); + if (!bookmark_result.ok()) + return bookmark_result.error(); + FileDescriptor bookmarks(bookmark_result.get()); + return std::unique_ptr(new BookmarkLock( + directory.release(), + bookmarks.release(), + file.release(), + root, + [key copy])); } Result read_bookmark(const BookmarkLock& lock) { - if (lock.bookmarks_.get() < 0) return static_cast(nil); + if (lock.bookmarks_.get() < 0) + return static_cast(nil); FileDescriptor file(retry_eintr([&] { return openat( lock.bookmarks_.get(), bookmark_name(lock.key_).fileSystemRepresentation, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC); })); - if (file.get() < 0 && errno == ENOENT) return static_cast(nil); - ET_CHECK_OR_RETURN_ERROR(file.get() >= 0, AccessFailed, - "Cannot open Core AI bookmark"); + if (file.get() < 0 && errno == ENOENT) + return static_cast(nil); + ET_CHECK_OR_RETURN_ERROR( + file.get() >= 0, AccessFailed, "Cannot open Core AI bookmark"); struct stat info; ET_CHECK_OR_RETURN_ERROR( fstat(file.get(), &info) == 0 && S_ISREG(info.st_mode) && info.st_size > 0 && static_cast(info.st_size) <= kBookmarkByteLimit, - InvalidExternalData, "Invalid Core AI bookmark file"); + InvalidExternalData, + "Invalid Core AI bookmark file"); NSMutableData* bytes = [NSMutableData dataWithLength:info.st_size]; size_t offset = 0; while (offset < bytes.length) { - ssize_t count = - ::read(file.get(), static_cast(bytes.mutableBytes) + offset, - bytes.length - offset); - if (count < 0 && errno == EINTR) continue; - ET_CHECK_OR_RETURN_ERROR(count > 0, AccessFailed, - "Cannot read Core AI bookmark"); + ssize_t count = ::read( + file.get(), + static_cast(bytes.mutableBytes) + offset, + bytes.length - offset); + if (count < 0 && errno == EINTR) + continue; + ET_CHECK_OR_RETURN_ERROR( + count > 0, AccessFailed, "Cannot read Core AI bookmark"); offset += count; } return bytes; @@ -235,16 +284,22 @@ Error write_bookmark(const BookmarkLock& lock, NSData* data) { ET_CHECK_OR_RETURN_ERROR( [data isKindOfClass:NSData.class] && data.length > 0 && data.length <= kBookmarkByteLimit, - InvalidExternalData, "Invalid Core AI bookmark data"); - return publish_storage_data(lock.bookmarks_.get(), bookmark_name(lock.key_), - data); + InvalidExternalData, + "Invalid Core AI bookmark data"); + ET_CHECK_OR_RETURN_ERROR( + lock.bookmarks_.get() >= 0, + InvalidArgument, + "Cannot write through a staging-only Core AI bookmark lock"); + return publish_storage_data( + lock.bookmarks_.get(), bookmark_name(lock.key_), data); } Result prepare_bookmark_staging(const BookmarkLock& lock) { auto staging = open_child(lock.root_.get(), @"staging", true); - if (!staging.ok()) return staging.error(); + if (!staging.ok()) + return staging.error(); FileDescriptor directory(staging.get()); return [lock.root_path_ stringByAppendingPathComponent:@"staging"]; } -} // namespace executorch::backends::coreai +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm index 51713168626..ee98ea14c05 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm @@ -21,6 +21,7 @@ #include #include #include +#include "coreai_fault_scope.h" #include "coreai_manifest_fixture.h" extern char** environ; @@ -38,8 +39,8 @@ std::string executable; int remaining_ms(Clock::time_point deadline) { - const auto remaining = - std::chrono::duration_cast(deadline - Clock::now()); + const auto remaining = std::chrono::duration_cast( + deadline - Clock::now()); return remaining.count() <= 0 ? 0 : static_cast(remaining.count()); } @@ -48,22 +49,28 @@ bool send_signal_byte(int fd, char byte) { while (const int remaining = remaining_ms(deadline)) { pollfd entry{fd, POLLOUT, 0}; const int ready = poll(&entry, 1, remaining); - if (ready < 0 && errno == EINTR) continue; - if (ready <= 0 || !(entry.revents & POLLOUT)) return false; + if (ready < 0 && errno == EINTR) + continue; + if (ready <= 0 || !(entry.revents & POLLOUT)) + return false; const ssize_t written = write(fd, &byte, 1); - if (written == 1) return true; - if (written < 0 && (errno == EINTR || errno == EAGAIN)) continue; + if (written == 1) + return true; + if (written < 0 && (errno == EINTR || errno == EAGAIN)) + continue; return false; } return false; } -} // namespace +} // namespace Result bookmark_manifest(bool aot) { NSData* encoded = encode(aot ? aot_manifest_dict() : manifest_dict()); - if (encoded == nil) return Error::InvalidExternalData; + if (encoded == nil) + return Error::InvalidExternalData; auto parsed = parse_manifest(encoded); - if (!parsed.ok()) return parsed.error(); + if (!parsed.ok()) + return parsed.error(); return select_assets(parsed.get(), @"arch_b", @"macOS"); } @@ -78,53 +85,80 @@ bool send_signal_byte(int fd, char byte) { Result saved_bookmark(NSURL* root, NSString* key) { auto locked = lock_bookmark(root.path, key); - if (!locked.ok()) return locked.error(); + if (!locked.ok()) + return locked.error(); return read_bookmark(*locked.get()); } Result sized_bookmark(size_t size, uint32_t tag) { - if (size < sizeof(tag)) return Error::InvalidArgument; + if (size < sizeof(tag)) + return Error::InvalidArgument; NSMutableData* bytes = [NSMutableData dataWithLength:size]; - if (bytes == nil) return Error::MemoryAllocationFailed; + if (bytes == nil) + return Error::MemoryAllocationFailed; std::memcpy(bytes.mutableBytes, &tag, sizeof(tag)); return [bytes copy]; } -void set_bookmark_test_executable(const char* path) { executable = path == nullptr ? "" : path; } +void set_bookmark_test_executable(const char* path) { + executable = path == nullptr ? "" : path; +} int bookmark_child_mode(int argc, char** argv) { - if (argc < 2 || std::strncmp(argv[1], "--bookmark-", 11) != 0) return -1; - if (argc != 5) return 1; + if (argc < 2 || std::strncmp(argv[1], "--bookmark-", 11) != 0) + return -1; + if (argc != 5) + return 1; const bool hold = std::strcmp(argv[1], "--bookmark-hold") == 0; const bool lock = std::strcmp(argv[1], "--bookmark-lock") == 0; const bool other = std::strcmp(argv[3], "other") == 0; const bool same = std::strcmp(argv[3], "same") == 0; - if ((!hold && !lock) || (!other && !same)) return 2; + if ((!hold && !lock) || (!other && !same)) + return 2; char* end = nullptr; errno = 0; const long value = std::strtol(argv[4], &end, 10); - if (errno != 0 || end == argv[4] || *end != '\0' || value < 0 || value > INT_MAX) return 3; + if (errno != 0 || end == argv[4] || *end != '\0' || value < 0 || + value > INT_MAX) + return 3; FileDescriptor signal_fd(static_cast(value)); - if (fcntl(signal_fd.get(), F_GETFD) < 0) return 4; + if (fcntl(signal_fd.get(), F_GETFD) < 0) + return 4; signal(SIGPIPE, SIG_IGN); NSString* root = [NSString stringWithUTF8String:argv[2]]; - if (root == nil) return 5; - // Child modes run before InitGoogleTest and must not call assertion helpers. - Manifest manifest; - manifest.path = @"ab/model.aimodel"; - manifest.bundle_digests = @{ - @"model.aimodel" : [NSString stringWithUTF8String:std::string(64, other ? '2' : '1').c_str()] + if (root == nil) + return 5; + auto manifest = bookmark_manifest(); + if (!manifest.ok()) + return 6; + if (other) { + manifest->bundle_digests = + @{manifest->path.lastPathComponent : fixture_identity('2')}; + } + auto key = test_bookmark_key(manifest.get()); + if (!key.ok()) + return 7; + StorageFaultScope scope(StorageOperation::BeforeBookmarkLock); + scope.armed = false; + const int notify_fd = signal_fd.get(); + scope.observe = ^(StorageOperation operation) { + if (lock && same && operation == StorageOperation::BeforeBookmarkLock && + !send_signal_byte(notify_fd, 'B')) { + _exit(11); + } }; - auto key = test_bookmark_key(manifest); - if (!key.ok()) return 6; - if (lock && same && !send_signal_byte(signal_fd.get(), 'B')) return 7; auto locked = lock_bookmark(root, key.get()); - if (!locked.ok()) return 7; - if (!send_signal_byte(signal_fd.get(), 'L')) return 8; - if (close(signal_fd.release()) != 0) return 9; + if (!locked.ok()) + return 8; + if (!send_signal_byte(signal_fd.get(), 'L')) + return 9; + if (close(signal_fd.release()) != 0) + return 10; if (hold) { - // The parent owns this PID and always kills and reaps it, including on failure. - for (;;) pause(); + // The parent owns this PID and always kills and reaps it, including on + // failure. + for (;;) + pause(); } return 0; } @@ -132,40 +166,47 @@ int bookmark_child_mode(int argc, char** argv) { BookmarkChild::~BookmarkChild() { if (pid_ > 0) { const auto cleaned = kill_and_reap(); - if (!cleaned) ADD_FAILURE() << cleaned.message(); + if (!cleaned) + ADD_FAILURE() << cleaned.message(); } if (signal_fd_ >= 0 && close(signal_fd_) != 0) { ADD_FAILURE() << "Cannot close child signal descriptor: " << errno; } } -::testing::AssertionResult BookmarkChild::spawn(NSString* mode, NSString* root, - NSString* argument) { +::testing::AssertionResult +BookmarkChild::spawn(NSString* mode, NSString* root, NSString* argument) { if (pid_ > 0 || signal_fd_ >= 0 || executable.empty() || root == nil) { return AssertionFailure() << "Invalid bookmark child setup"; } int descriptors[2]; - if (pipe(descriptors) != 0) return AssertionFailure() << "pipe: " << errno; + if (pipe(descriptors) != 0) + return AssertionFailure() << "pipe: " << errno; FileDescriptor reader(descriptors[0]); FileDescriptor writer(descriptors[1]); for (int fd : descriptors) { - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0 || fcntl(fd, F_SETFL, O_NONBLOCK) != 0) { + if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0 || + fcntl(fd, F_SETFL, O_NONBLOCK) != 0) { return AssertionFailure() << "fcntl pipe: " << errno; } } posix_spawn_file_actions_t actions; int error = posix_spawn_file_actions_init(&actions); - if (error != 0) return AssertionFailure() << "spawn actions: " << error; - error = posix_spawn_file_actions_adddup2(&actions, writer.get(), kChildSignalFD); + if (error != 0) + return AssertionFailure() << "spawn actions: " << error; + error = + posix_spawn_file_actions_adddup2(&actions, writer.get(), kChildSignalFD); std::string descriptor = std::to_string(kChildSignalFD); - char* arguments[] = {const_cast(executable.c_str()), - const_cast(mode.UTF8String), - const_cast(root.fileSystemRepresentation), - const_cast(argument.UTF8String), - descriptor.data(), - nullptr}; + char* arguments[] = { + const_cast(executable.c_str()), + const_cast(mode.UTF8String), + const_cast(root.fileSystemRepresentation), + const_cast(argument.UTF8String), + descriptor.data(), + nullptr}; if (error == 0) { - error = posix_spawn(&pid_, arguments[0], &actions, nullptr, arguments, environ); + error = + posix_spawn(&pid_, arguments[0], &actions, nullptr, arguments, environ); } const int destroyed = posix_spawn_file_actions_destroy(&actions); if (error != 0) { @@ -173,7 +214,8 @@ int bookmark_child_mode(int argc, char** argv) { return AssertionFailure() << "posix_spawn: " << error; } signal_fd_ = reader.release(); - if (destroyed != 0) return AssertionFailure() << "destroy spawn actions: " << destroyed; + if (destroyed != 0) + return AssertionFailure() << "destroy spawn actions: " << destroyed; return AssertionSuccess(); } @@ -182,20 +224,27 @@ int bookmark_child_mode(int argc, char** argv) { while (const int remaining = remaining_ms(deadline)) { pollfd entry{signal_fd_, POLLIN, 0}; const int ready = poll(&entry, 1, remaining); - if (ready < 0 && errno == EINTR) continue; - if (ready < 0) return AssertionFailure() << "poll: " << errno; - if (ready == 0) break; + if (ready < 0 && errno == EINTR) + continue; + if (ready < 0) + return AssertionFailure() << "poll: " << errno; + if (ready == 0) + break; if (!(entry.revents & POLLIN)) - return AssertionFailure() << "Child closed signal pipe before " << expected; + return AssertionFailure() + << "Child closed signal pipe before " << expected; char byte = 0; const ssize_t count = read(signal_fd_, &byte, 1); - if (count < 0 && (errno == EINTR || errno == EAGAIN)) continue; + if (count < 0 && (errno == EINTR || errno == EAGAIN)) + continue; if (count != 1 || byte != expected) { - return AssertionFailure() << "Expected child signal " << expected << ", got " << int(byte); + return AssertionFailure() + << "Expected child signal " << expected << ", got " << int(byte); } return AssertionSuccess(); } - return AssertionFailure() << "Timed out waiting for child signal " << expected; + return AssertionFailure() + << "Timed out waiting for child signal " << expected; } ::testing::AssertionResult BookmarkChild::expect_blocked() { @@ -203,16 +252,20 @@ int bookmark_child_mode(int argc, char** argv) { while (const int remaining = remaining_ms(deadline)) { pollfd entry{signal_fd_, POLLIN, 0}; const int ready = poll(&entry, 1, remaining); - if (ready < 0 && errno == EINTR) continue; - if (ready < 0) return AssertionFailure() << "poll: " << errno; - if (ready > 0) return AssertionFailure() << "Child completed while lock was held"; + if (ready < 0 && errno == EINTR) + continue; + if (ready < 0) + return AssertionFailure() << "poll: " << errno; + if (ready > 0) + return AssertionFailure() << "Child completed while lock was held"; return AssertionSuccess(); } return AssertionSuccess(); } ::testing::AssertionResult BookmarkChild::reap(int& status) { - if (pid_ <= 0) return AssertionFailure() << "No owned child to reap"; + if (pid_ <= 0) + return AssertionFailure() << "No owned child to reap"; const auto deadline = Clock::now() + kChildTimeout; while (const int remaining = remaining_ms(deadline)) { const pid_t result = waitpid(pid_, &status, WNOHANG); @@ -220,8 +273,10 @@ int bookmark_child_mode(int argc, char** argv) { pid_ = -1; return AssertionSuccess(); } - if (result < 0 && errno == EINTR) continue; - if (result < 0) return AssertionFailure() << "waitpid: " << errno; + if (result < 0 && errno == EINTR) + continue; + if (result < 0) + return AssertionFailure() << "waitpid: " << errno; poll(nullptr, 0, std::min(remaining, 10)); } return AssertionFailure() << "Timed out reaping child " << pid_; @@ -230,25 +285,30 @@ int bookmark_child_mode(int argc, char** argv) { ::testing::AssertionResult BookmarkChild::expect_exit(int expected) { int status = 0; auto result = reap(status); - if (!result) return result; + if (!result) + return result; if (!WIFEXITED(status) || WEXITSTATUS(status) != expected) { - return AssertionFailure() << "Expected child exit " << expected << ", wait status " << status; + return AssertionFailure() + << "Expected child exit " << expected << ", wait status " << status; } return AssertionSuccess(); } ::testing::AssertionResult BookmarkChild::kill_and_reap() { - if (pid_ <= 0) return AssertionFailure() << "No owned child to kill"; + if (pid_ <= 0) + return AssertionFailure() << "No owned child to kill"; const int killed = kill(pid_, SIGKILL); const int error = errno; int status = 0; auto result = reap(status); - if (!result) return result; + if (!result) + return result; if (killed != 0 || !WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) { - return AssertionFailure() << "Expected SIGKILL, kill errno " << (killed == 0 ? 0 : error) - << ", wait status " << status; + return AssertionFailure() + << "Expected SIGKILL, kill errno " << (killed == 0 ? 0 : error) + << ", wait status " << status; } return AssertionSuccess(); } -} // namespace executorch::backends::coreai::testing +} // namespace executorch::backends::coreai::testing diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm index ff8c4e25996..f93dc92d230 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm @@ -6,12 +6,14 @@ * LICENSE file in the root directory of this source tree. */ -#include #include -#include #include #include +#include +#include +#include #include "coreai_bookmark_fixture.h" +#include "coreai_fault_scope.h" #include "coreai_filesystem_fixture.h" #include "coreai_manifest_fixture.h" #include "coreai_storage.h" @@ -22,6 +24,38 @@ namespace { constexpr size_t kMiB = 1024 * 1024; +constexpr auto kWaitTimeout = std::chrono::seconds(5); + +struct BookmarkWaiter { + std::future attempting; + std::future>> result; +}; + +BookmarkWaiter +start_bookmark_waiter(NSString* path, NSString* key, bool create = true) { + std::promise attempting; + std::promise>> result; + BookmarkWaiter waiter{attempting.get_future(), result.get_future()}; + // Promise-backed futures cannot block teardown if acquisition stalls. + std::thread([path, + key, + create, + attempting = std::move(attempting), + result = std::move(result)]() mutable { + @autoreleasepool { + StorageFaultScope scope(StorageOperation::BeforeBookmarkLock); + scope.armed = false; + auto* signal = &attempting; + scope.observe = ^(StorageOperation operation) { + if (operation == StorageOperation::BeforeBookmarkLock) { + signal->set_value(); + } + }; + result.set_value(lock_bookmark(path, key, create)); + } + }).detach(); + return waiter; +} class CoreAIBookmarkIdentityTest : public ::testing::Test { protected: @@ -40,10 +74,11 @@ void SetUp() override { class CoreAIBookmarkPathTest : public CoreAIBookmarkIdentityTest { protected: - BookmarkDirectory root; + TestDirectory root; void SetUp() override { CoreAIBookmarkIdentityTest::SetUp(); - if (HasFatalFailure()) return; + if (HasFatalFailure()) + return; ASSERT_NE(root.url, nil); } NSURL* lock_url() { @@ -58,7 +93,8 @@ void SetUp() override { NSURL* url = nil; void SetUp() override { CoreAIBookmarkPathTest::SetUp(); - if (HasFatalFailure()) return; + if (HasFatalFailure()) + return; auto result = lock_bookmark(root.url.path, key); ASSERT_TRUE(result.ok()); locked = std::move(result.get()); @@ -67,11 +103,15 @@ void SetUp() override { } }; -::testing::AssertionResult bookmark_bytes_equal(const BookmarkLock& lock, NSData* expected) { +::testing::AssertionResult bookmark_bytes_equal( + const BookmarkLock& lock, + NSData* expected) { auto result = read_bookmark(lock); if (!result.ok()) - return ::testing::AssertionFailure() << "Bookmark read error " << int(result.error()); - if (expected == nil ? result.get() != nil : ![result.get() isEqual:expected]) { + return ::testing::AssertionFailure() + << "Bookmark read error " << int(result.error()); + if (expected == nil ? result.get() != nil + : ![result.get() isEqual:expected]) { return ::testing::AssertionFailure() << "Bookmark bytes differ"; } return ::testing::AssertionSuccess(); @@ -79,10 +119,15 @@ void SetUp() override { ::testing::AssertionResult no_temporary_bookmarks(NSURL* root) { NSURL* directory = [root URLByAppendingPathComponent:@"bookmarks"]; - FileDescriptor fd(open(directory.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); - if (fd.get() < 0) return ::testing::AssertionFailure() << "open bookmark directory: " << errno; + FileDescriptor fd(open( + directory.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); + if (fd.get() < 0) + return ::testing::AssertionFailure() + << "open bookmark directory: " << errno; auto children = storage_children(fd.get()); - if (!children.ok()) return ::testing::AssertionFailure() << "Cannot enumerate bookmark directory"; + if (!children.ok()) + return ::testing::AssertionFailure() + << "Cannot enumerate bookmark directory"; for (NSString* name in children.get()) { if ([name hasPrefix:@".tmp-"]) return ::testing::AssertionFailure() << "Leaked " << name.UTF8String; @@ -101,7 +146,9 @@ void SetUp() override { EXPECT_TRUE([changed.get() isEqual:key]); } -TEST_F(CoreAIBookmarkIdentityTest, KeySeparatesPlatformArchitectureWeightsAndAotContext) { +TEST_F( + CoreAIBookmarkIdentityTest, + KeySeparatesPlatformArchitectureWeightsAndAotContext) { auto platform = bookmark_key(manifest, @"iOS", @"arch_b"); auto architecture = bookmark_key(manifest, @"macOS", @"arch_a"); Manifest reweighted = manifest; @@ -218,7 +265,7 @@ FileDescriptor file(open( auto staging = prepare_bookmark_staging(*locked); ASSERT_TRUE(staging.ok()); EXPECT_TRUE([staging.get() isEqual:expected]); - struct stat info{}; + struct stat info {}; ASSERT_EQ(lstat(staging.get().fileSystemRepresentation, &info), 0); EXPECT_TRUE(S_ISDIR(info.st_mode)); auto repeated = prepare_bookmark_staging(*locked); @@ -254,8 +301,10 @@ FileDescriptor file(open( } TEST_F(CoreAIBookmarkRawTest, RejectsOversizedStoredBookmark) { - FileDescriptor file( - open(url.fileSystemRepresentation, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); + FileDescriptor file(open( + url.fileSystemRepresentation, + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, + 0600)); ASSERT_GE(file.get(), 0); ASSERT_EQ(ftruncate(file.get(), 8 * kMiB + 1), 0); ASSERT_EQ(close(file.release()), 0); @@ -327,6 +376,8 @@ FileDescriptor file( ASSERT_TRUE(existing.ok()); ASSERT_NE(existing.get(), nullptr); EXPECT_TRUE(bookmark_bytes_equal(*existing.get(), nil)); + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + EXPECT_EQ(write_bookmark(*existing.get(), bytes), Error::InvalidArgument); EXPECT_TRUE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:[root.url URLByAppendingPathComponent:@"bookmarks"] @@ -348,9 +399,12 @@ FileDescriptor file( TEST_F(CoreAIBookmarkPathTest, RejectsInvalidResolvedRoots) { for (bool create : {false, true}) { - EXPECT_EQ( - resolve_bookmark_root(@"relative", create).error(), - Error::InvalidArgument); + for (NSString* path in + @[ @"relative", @"~", @"~/cache", @"~nobody/cache" ]) { + SCOPED_TRACE(path.UTF8String); + EXPECT_EQ( + resolve_bookmark_root(path, create).error(), Error::InvalidArgument); + } EXPECT_EQ( resolve_bookmark_root(nil, create).error(), Error::InvalidArgument); } @@ -371,35 +425,66 @@ FileDescriptor file( auto holder = lock_bookmark(root.url.path, key); ASSERT_TRUE(holder.ok()); ASSERT_NE(holder.get(), nullptr); - auto acquire = [path = root.url.path](NSString* next_key) { - @autoreleasepool { - auto result = lock_bookmark(path, next_key); - if (!result.ok()) - return result.error(); - return result.get() != nullptr ? Error::Ok : Error::Internal; - } - }; - std::promise started; - auto starting = started.get_future(); - auto waiter = std::async(std::launch::async, [&] { - started.set_value(); - return acquire(key); - }); - EXPECT_EQ( - starting.wait_for(std::chrono::seconds(5)), std::future_status::ready); + auto waiter = start_bookmark_waiter(root.url.path, key); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); EXPECT_EQ( - waiter.wait_for(std::chrono::milliseconds(100)), + waiter.result.wait_for(std::chrono::milliseconds(100)), std::future_status::timeout); auto independent = - std::async(std::launch::async, acquire, fixture_identity('f')); - EXPECT_EQ( - independent.wait_for(std::chrono::seconds(5)), std::future_status::ready); + start_bookmark_waiter(root.url.path, fixture_identity('f')); + ASSERT_EQ( + independent.result.wait_for(kWaitTimeout), std::future_status::ready); + auto other = independent.result.get(); + ASSERT_TRUE(other.ok()); + ASSERT_NE(other.get(), nullptr); holder.get().reset(); - EXPECT_EQ(waiter.get(), Error::Ok); - EXPECT_EQ(independent.get(), Error::Ok); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto acquired = waiter.result.get(); + ASSERT_TRUE(acquired.ok()); + EXPECT_NE(acquired.get(), nullptr); } -TEST_F(CoreAIBookmarkPathTest, ProcessContentionAllowsIndependentKeyAndRecoversAfterHolderDeath) { +TEST_F(CoreAIBookmarkRawTest, NoncreatingWaiterRechecksEvictedEntry) { + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(write_bookmark(*locked, bytes), Error::Ok); + auto waiter = start_bookmark_waiter(root.url.path, key, false); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + ASSERT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + ASSERT_EQ(unlink(url.fileSystemRepresentation), 0); + locked.reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto absent = waiter.result.get(); + ASSERT_TRUE(absent.ok()); + EXPECT_EQ(absent.get(), nullptr); +} + +TEST_F(CoreAIBookmarkRawTest, WaiterOpensBookmarkDirectoryAfterAcquiringLock) { + auto waiter = start_bookmark_waiter(root.url.path, key); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + ASSERT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + NSURL* bookmarks = [root.url URLByAppendingPathComponent:@"bookmarks"]; + ASSERT_EQ(rmdir(bookmarks.fileSystemRepresentation), 0); + ASSERT_EQ(mkdir(bookmarks.fileSystemRepresentation, 0700), 0); + locked.reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto acquired = waiter.result.get(); + ASSERT_TRUE(acquired.ok()); + ASSERT_NE(acquired.get(), nullptr); + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(write_bookmark(*acquired.get(), bytes), Error::Ok); + EXPECT_TRUE([[NSData dataWithContentsOfURL:url] isEqual:bytes]); +} + +TEST_F( + CoreAIBookmarkPathTest, + ProcessContentionAllowsIndependentKeyAndRecoversAfterHolderDeath) { BookmarkChild holder; ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); ASSERT_TRUE(holder.receive('L')); @@ -423,4 +508,4 @@ FileDescriptor file( EXPECT_EQ(before.st_ino, after.st_ino); } -} // namespace +} // namespace diff --git a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h index f01a9ad6d3b..625f1f48a8a 100644 --- a/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h +++ b/backends/apple/coreai/runtime/test/coreai_filesystem_fixture.h @@ -21,8 +21,6 @@ struct TestDirectory { TestDirectory& operator=(const TestDirectory&) = delete; }; -using BookmarkDirectory = TestDirectory; - ::testing::AssertionResult backup_excluded(NSURL* url); ::testing::AssertionResult set_backup_excluded(NSURL* url, bool excluded); diff --git a/backends/apple/coreai/runtime/test/coreai_host_test.mm b/backends/apple/coreai/runtime/test/coreai_host_test.mm index ebf392c060a..679321c248b 100644 --- a/backends/apple/coreai/runtime/test/coreai_host_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_host_test.mm @@ -14,9 +14,12 @@ int main(int argc, char** argv) { executorch::runtime::runtime_init(); @autoreleasepool { - executorch::backends::coreai::testing::set_bookmark_test_executable(argv[0]); - const int child_result = executorch::backends::coreai::testing::bookmark_child_mode(argc, argv); - if (child_result >= 0) return child_result; + executorch::backends::coreai::testing::set_bookmark_test_executable( + argv[0]); + const int child_result = + executorch::backends::coreai::testing::bookmark_child_mode(argc, argv); + if (child_result >= 0) + return child_result; ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } From e7b15c72f63896e4ccb70116ea69b67f94a6ff57 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Tue, 27 Oct 2026 17:18:05 -0700 Subject: [PATCH 4/6] [CoreAI] Bound bookmark locks with a stable stripe pool Map validated cache keys to 256 on-demand, versioned lock files. Preserve flock exclusion, stable inodes, noncreating maintenance, and per-key data paths. Document quiescent upgrades and retain legacy locks without online deletion. Test Plan: Bookmark-layer coreai_host_test passed, including 1,024-key pool bounds, thread/process collisions, holder-death recovery, invalid stripe files, and legacy preservation. Full-stack validation before placement: 256 native tests, five CTest targets, and 124 Python tests passed. --- backends/apple/coreai/README.md | 33 +++- .../apple/coreai/runtime/coreai_bookmarks.h | 14 +- .../apple/coreai/runtime/coreai_bookmarks.mm | 17 +- .../runtime/test/coreai_bookmark_fixture.h | 1 + .../runtime/test/coreai_bookmark_fixture.mm | 30 ++- .../runtime/test/coreai_bookmark_test.mm | 175 +++++++++++++++++- 6 files changed, 241 insertions(+), 29 deletions(-) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index 6f2bf8be1e0..a51c455660d 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -86,17 +86,34 @@ Each partition derives a key from its selected export digest/bundle, source/AOT mode, platform, Core AI device architecture, default SDK cache namespace, versioned default options and persistent policy. Function bindings and delivery location are excluded. No model bytes are hashed at runtime. Each key determines -a raw bookmark in `bookmarks`, a bundle directory in `staging`, and an empty -coordination file in `locks`. There is no PTE-wide bookmark list or serialized -lifecycle record. +a raw bookmark in `bookmarks`, a bundle directory in `staging`, and one of +256 shared coordination files in `locks`. There is no PTE-wide bookmark list or +serialized lifecycle record. + Raw bookmarks have an 8 MiB backend allocation limit, not an asserted SDK format maximum, and are published atomically because losing one orphans an SDK entry. -Per-key disk locks coordinate loading and eviction across threads and processes; -different keys remain independent. Lock ownership is maintained by the OS, and -acquiring a lock does not flush its file or directory. Lock files are not -removed by the backend, including after process exit. Removing the assets root -externally requires all loads, sessions and maintenance to stop. +A fixed pool of at most 256 disk locks coordinates loading and eviction across +threads and processes. The first byte of the validated cache key selects +`stripe-v1-00.lock` through `stripe-v1-ff.lock` within the lock directory. Files +are created on demand, so new model versions do not grow the pool beyond 256. +The mapping is part of the on-disk format and is not configurable. Different +keys on the same stripe serialize; different stripes remain independent. +Bookmarks and staging remain separate for every key. Runtime +operations hold only one stripe at a time. + +Lock ownership is maintained by the OS, and acquiring a lock does not flush its +file or directory. Stripe files are never removed, including during cache +clearing or after process exit, so waiters and future openers keep using the +same inode. Removing the assets root externally requires all loads, sessions +and maintenance to stop. + +Upgrading from per-key lock files requires stopping all processes using the +root and upgrading them together. Old and new locking schemes must not run +concurrently against the same root. Legacy per-key lock files are ignored and +left untouched; they can be removed separately during a fully quiescent offline +cleanup. The 256-file bound applies to the new stripe pool, not pre-existing +legacy files. Bookmarks and staged bundles need no migration. ## Host Tests diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.h b/backends/apple/coreai/runtime/coreai_bookmarks.h index e0fb78d2b7a..8a0181719f4 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.h +++ b/backends/apple/coreai/runtime/coreai_bookmarks.h @@ -7,6 +7,7 @@ */ #pragma once +#include #include #import "coreai_assets.h" #include "coreai_file.h" @@ -17,6 +18,9 @@ runtime::Result bookmark_key( const Manifest& selected, NSString* platform, NSString* architecture); +// The first key byte selects a stripe in a fixed, versioned lock namespace. +constexpr size_t kBookmarkLockStripeCount = 256; +runtime::Result bookmark_lock_name(NSString* key); // With create=false, a missing root is returned unchanged for no-op // maintenance. runtime::Result resolve_bookmark_root( @@ -48,10 +52,12 @@ class BookmarkLock final { NSString* key_; }; -// Each acquisition opens an independent descriptor; the backend never removes -// lock files. With create=false, missing roots or entries return a null lock. -// Existing entries may require creating coordination files, so the root must -// be writable even with create=false. +// Each acquisition opens an independent descriptor on one of 256 stable +// stripes. Different keys can share a stripe; do not hold multiple +// BookmarkLocks at once. The backend never removes lock files. With +// create=false, missing roots or entries return a null lock. Existing entries +// may require creating coordination files, so the root must be writable even +// with create=false. runtime::Result> lock_bookmark(NSString* root, NSString* key, bool create = true); // Missing is nil. Empty, oversized, inaccessible or nonregular files fail diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.mm b/backends/apple/coreai/runtime/coreai_bookmarks.mm index 7c63b1a5a1b..0257b1502dc 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.mm +++ b/backends/apple/coreai/runtime/coreai_bookmarks.mm @@ -79,6 +79,13 @@ bool digest(id value) { return [key copy]; } +Result bookmark_lock_name(NSString* key) { + ET_CHECK_OR_RETURN_ERROR( + digest(key), InvalidArgument, "Invalid Core AI bookmark key"); + return [NSString + stringWithFormat:@"stripe-v1-%@.lock", [key substringToIndex:2]]; +} + namespace { // Returns -1 when the root does not exist. Result open_root(NSString* root) { @@ -183,8 +190,9 @@ bool digest(id value) { Result> lock_bookmark(NSString* root, NSString* key, bool create) { - ET_CHECK_OR_RETURN_ERROR( - digest(key), InvalidArgument, "Invalid Core AI bookmark key"); + auto name = bookmark_lock_name(key); + if (!name.ok()) + return name.error(); auto resolved = resolve_bookmark_root(root, create); if (!resolved.ok()) return resolved.error(); @@ -209,9 +217,8 @@ bool digest(id value) { if (!lock_result.ok()) return lock_result.error(); FileDescriptor locks(lock_result.get()); - NSString* name = [key stringByAppendingString:@".lock"]; - FileDescriptor file( - open_storage_shared_file(locks.get(), name.fileSystemRepresentation)); + FileDescriptor file(open_storage_shared_file( + locks.get(), name.get().fileSystemRepresentation)); struct stat info; ET_CHECK_OR_RETURN_ERROR( file.get() >= 0 && fstat(file.get(), &info) == 0 && S_ISREG(info.st_mode), diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h index 2208af49ec2..252ed85b08d 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h @@ -19,6 +19,7 @@ namespace executorch::backends::coreai::testing { runtime::Result bookmark_manifest(bool aot = false); runtime::Result test_bookmark_key(const Manifest& manifest); NSURL* test_bookmark_url(NSURL* root, NSString* key); +NSURL* test_bookmark_lock_url(NSURL* root, NSString* key); runtime::Result saved_bookmark(NSURL* root, NSString* key); runtime::Result sized_bookmark(size_t size, uint32_t tag = 1); diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm index ee98ea14c05..694a47670e9 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm @@ -83,6 +83,15 @@ bool send_signal_byte(int fd, char byte) { URLByAppendingPathComponent:[key stringByAppendingString:@".bookmark"]]; } +NSURL* test_bookmark_lock_url(NSURL* root, NSString* key) { + auto name = bookmark_lock_name(key); + EXPECT_TRUE(name.ok()); + if (!name.ok()) + return nil; + return [[root URLByAppendingPathComponent:@"locks"] + URLByAppendingPathComponent:name.get()]; +} + Result saved_bookmark(NSURL* root, NSString* key) { auto locked = lock_bookmark(root.path, key); if (!locked.ok()) @@ -113,7 +122,8 @@ int bookmark_child_mode(int argc, char** argv) { const bool lock = std::strcmp(argv[1], "--bookmark-lock") == 0; const bool other = std::strcmp(argv[3], "other") == 0; const bool same = std::strcmp(argv[3], "same") == 0; - if ((!hold && !lock) || (!other && !same)) + const bool collision = std::strcmp(argv[3], "collision") == 0; + if ((!hold && !lock) || (!other && !same && !collision)) return 2; char* end = nullptr; errno = 0; @@ -131,23 +141,29 @@ int bookmark_child_mode(int argc, char** argv) { auto manifest = bookmark_manifest(); if (!manifest.ok()) return 6; - if (other) { - manifest->bundle_digests = - @{manifest->path.lastPathComponent : fixture_identity('2')}; - } auto key = test_bookmark_key(manifest.get()); if (!key.ok()) return 7; + NSMutableString* selected_key = [key.get() mutableCopy]; + if (other || collision) { + const NSUInteger index = other ? 0 : 63; + [selected_key + replaceCharactersInRange:NSMakeRange(index, 1) + withString:[selected_key characterAtIndex:index] == '0' + ? @"1" + : @"0"]; + } StorageFaultScope scope(StorageOperation::BeforeBookmarkLock); scope.armed = false; const int notify_fd = signal_fd.get(); scope.observe = ^(StorageOperation operation) { - if (lock && same && operation == StorageOperation::BeforeBookmarkLock && + if (lock && (same || collision) && + operation == StorageOperation::BeforeBookmarkLock && !send_signal_byte(notify_fd, 'B')) { _exit(11); } }; - auto locked = lock_bookmark(root, key.get()); + auto locked = lock_bookmark(root, selected_key); if (!locked.ok()) return 8; if (!send_signal_byte(signal_fd.get(), 'L')) diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm index f93dc92d230..00ea4598c34 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -26,6 +27,15 @@ constexpr size_t kMiB = 1024 * 1024; constexpr auto kWaitTimeout = std::chrono::seconds(5); +NSString* changed_key(NSString* key, bool same_stripe) { + NSMutableString* changed = [key mutableCopy]; + const NSUInteger index = same_stripe ? 63 : 0; + [changed replaceCharactersInRange:NSMakeRange(index, 1) + withString:[key characterAtIndex:index] == '0' ? @"1" + : @"0"]; + return changed; +} + struct BookmarkWaiter { std::future attempting; std::future>> result; @@ -82,8 +92,7 @@ void SetUp() override { ASSERT_NE(root.url, nil); } NSURL* lock_url() { - return [[root.url URLByAppendingPathComponent:@"locks"] - URLByAppendingPathComponent:[key stringByAppendingString:@".lock"]]; + return test_bookmark_lock_url(root.url, key); } }; @@ -352,7 +361,7 @@ FileDescriptor file(open( } TEST_F(CoreAIBookmarkPathTest, NoncreatingLookupIgnoresOtherEntries) { - NSString* other_key = fixture_identity('f'); + NSString* other_key = changed_key(key, false); auto other = lock_bookmark(root.url.path, other_key); ASSERT_TRUE(other.ok()); ASSERT_NE(other.get(), nullptr); @@ -421,7 +430,7 @@ FileDescriptor file(open( TEST_F( CoreAIBookmarkPathTest, - ThreadContentionSerializesSameKeyButNotDifferentKeys) { + ThreadContentionSerializesSameKeyButNotDifferentStripes) { auto holder = lock_bookmark(root.url.path, key); ASSERT_TRUE(holder.ok()); ASSERT_NE(holder.get(), nullptr); @@ -432,7 +441,7 @@ FileDescriptor file(open( waiter.result.wait_for(std::chrono::milliseconds(100)), std::future_status::timeout); auto independent = - start_bookmark_waiter(root.url.path, fixture_identity('f')); + start_bookmark_waiter(root.url.path, changed_key(key, false)); ASSERT_EQ( independent.result.wait_for(kWaitTimeout), std::future_status::ready); auto other = independent.result.get(); @@ -508,4 +517,160 @@ FileDescriptor file(open( EXPECT_EQ(before.st_ino, after.st_ino); } +TEST_F(CoreAIBookmarkPathTest, LockNamesUseOnlyValidatedFirstByte) { + EXPECT_EQ(bookmark_lock_name(nil).error(), Error::InvalidArgument); + for (NSString* invalid in + @[ @"", @"../escape", @"00", [fixture_identity('a') uppercaseString] ]) { + EXPECT_EQ(bookmark_lock_name(invalid).error(), Error::InvalidArgument); + } + auto first = bookmark_lock_name(fixture_identity('0')); + auto last = bookmark_lock_name(fixture_identity('f')); + ASSERT_TRUE(first.ok()); + ASSERT_TRUE(last.ok()); + EXPECT_TRUE([first.get() isEqual:@"stripe-v1-00.lock"]); + EXPECT_TRUE([last.get() isEqual:@"stripe-v1-ff.lock"]); + EXPECT_TRUE([test_bookmark_lock_url(root.url, key) + isEqual:test_bookmark_lock_url(root.url, changed_key(key, true))]); +} + +TEST_F(CoreAIBookmarkPathTest, CollidingKeysSerializeAcrossThreads) { + auto holder = lock_bookmark(root.url.path, key); + ASSERT_TRUE(holder.ok()); + NSString* collision = changed_key(key, true); + ASSERT_FALSE([collision isEqual:key]); + auto waiter = start_bookmark_waiter(root.url.path, collision); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + EXPECT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + holder.get().reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto acquired = waiter.result.get(); + ASSERT_TRUE(acquired.ok()); + EXPECT_NE(acquired.get(), nullptr); +} + +TEST_F( + CoreAIBookmarkPathTest, + NoncreatingCollisionMissDoesNotWaitOrCreateFiles) { + auto holder = lock_bookmark(root.url.path, key); + ASSERT_TRUE(holder.ok()); + auto waiter = + start_bookmark_waiter(root.url.path, changed_key(key, true), false); + EXPECT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + holder.get().reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto absent = waiter.result.get(); + ASSERT_TRUE(absent.ok()); + EXPECT_EQ(absent.get(), nullptr); + NSArray* names = [NSFileManager.defaultManager + contentsOfDirectoryAtPath:[root.url URLByAppendingPathComponent:@"locks"] + .path + error:nil]; + ASSERT_NE(names, nil); + EXPECT_EQ(names.count, 1u); +} + +TEST_F(CoreAIBookmarkPathTest, RejectsSymlinkDirectoryAndFifoStripeFiles) { + NSData* bytes = [@"untouched target" dataUsingEncoding:NSUTF8StringEncoding]; + NSURL* target = [root.url URLByAppendingPathComponent:@"target"]; + ASSERT_TRUE([bytes writeToURL:target atomically:NO]); + for (int kind = 0; kind < 3; ++kind) { + TestDirectory isolated; + ASSERT_NE(isolated.url, nil); + NSURL* locks = [isolated.url URLByAppendingPathComponent:@"locks"]; + ASSERT_EQ(mkdir(locks.fileSystemRepresentation, 0700), 0); + NSURL* stripe = test_bookmark_lock_url(isolated.url, key); + if (kind == 0) + ASSERT_EQ( + symlink( + target.fileSystemRepresentation, stripe.fileSystemRepresentation), + 0); + else if (kind == 1) + ASSERT_EQ(mkdir(stripe.fileSystemRepresentation, 0700), 0); + else + ASSERT_EQ(mkfifo(stripe.fileSystemRepresentation, 0600), 0); + EXPECT_EQ( + lock_bookmark(isolated.url.path, key).error(), Error::AccessFailed); + EXPECT_TRUE([[NSData dataWithContentsOfURL:target] isEqual:bytes]); + } +} + +TEST_F(CoreAIBookmarkPathTest, LegacyLockFilesAreLeftUntouched) { + NSURL* locks = [root.url URLByAppendingPathComponent:@"locks"]; + ASSERT_EQ(mkdir(locks.fileSystemRepresentation, 0700), 0); + NSURL* legacy = [locks + URLByAppendingPathComponent:[key stringByAppendingString:@".lock"]]; + NSData* bytes = [@"legacy" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_TRUE([bytes writeToURL:legacy atomically:NO]); + struct stat before {}; + ASSERT_EQ(lstat(legacy.fileSystemRepresentation, &before), 0); + auto locked = lock_bookmark(root.url.path, key); + ASSERT_TRUE(locked.ok()); + locked.get().reset(); + struct stat after {}; + ASSERT_EQ(lstat(legacy.fileSystemRepresentation, &after), 0); + EXPECT_EQ(before.st_ino, after.st_ino); + EXPECT_TRUE([[NSData dataWithContentsOfURL:legacy] isEqual:bytes]); + EXPECT_TRUE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); +} + +TEST_F(CoreAIBookmarkPathTest, CollidingProcessWaiterRecoversAfterHolderDeath) { + BookmarkChild holder; + ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); + ASSERT_TRUE(holder.receive('L')); + struct stat before {}; + ASSERT_EQ(lstat(lock_url().fileSystemRepresentation, &before), 0); + BookmarkChild waiter; + ASSERT_TRUE(waiter.spawn(@"--bookmark-lock", root.url.path, @"collision")); + ASSERT_TRUE(waiter.receive('B')); + ASSERT_TRUE(waiter.expect_blocked()); + ASSERT_TRUE(holder.kill_and_reap()); + ASSERT_TRUE(waiter.receive('L')); + ASSERT_TRUE(waiter.expect_exit(0)); + struct stat after {}; + ASSERT_EQ(lstat(lock_url().fileSystemRepresentation, &after), 0); + EXPECT_EQ(before.st_dev, after.st_dev); + EXPECT_EQ(before.st_ino, after.st_ino); +} + +TEST_F(CoreAIBookmarkPathTest, LockPoolIsBoundedAcrossManyKeys) { + std::array inodes{}; + NSURL* locks = [root.url URLByAppendingPathComponent:@"locks"]; + for (size_t generation = 0; generation < 4; ++generation) { + for (size_t stripe = 0; stripe < kBookmarkLockStripeCount; ++stripe) { + @autoreleasepool { + NSString* entry = + [NSString stringWithFormat:@"%02zx%062zx", stripe, generation]; + auto locked = lock_bookmark(root.url.path, entry); + ASSERT_TRUE(locked.ok()); + struct stat info {}; + NSURL* file = test_bookmark_lock_url(root.url, entry); + ASSERT_EQ(lstat(file.fileSystemRepresentation, &info), 0); + ASSERT_TRUE(S_ISREG(info.st_mode)); + EXPECT_EQ(info.st_size, 0); + if (generation == 0) + inodes[stripe] = info.st_ino; + else + EXPECT_EQ(info.st_ino, inodes[stripe]); + locked.get().reset(); + auto reused = lock_bookmark(root.url.path, entry); + ASSERT_TRUE(reused.ok()); + ASSERT_EQ(lstat(file.fileSystemRepresentation, &info), 0); + EXPECT_EQ(info.st_ino, inodes[stripe]); + } + } + NSArray* children = + [NSFileManager.defaultManager contentsOfDirectoryAtPath:locks.path + error:nil]; + ASSERT_NE(children, nil); + ASSERT_EQ(children.count, kBookmarkLockStripeCount); + for (NSString* name in children) { + EXPECT_TRUE([name hasPrefix:@"stripe-v1-"]); + EXPECT_EQ(name.length, 17u); + } + } +} + } // namespace From 0a3fa9d6291bc57707583439e383dfe9cb16e858 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Wed, 28 Oct 2026 15:33:05 -0700 Subject: [PATCH 5/6] [CoreAI] Reuse the shared directory fast path for bookmarks Summary: Avoid redundant directory creation during bookmark access and keep directory policy consistent with shared storage. Preserve missing-entry behavior and flush only directories created by this call. Test Plan: Ran the native host suite on the rebased bookmark layer: all 95 tests passed, including warm-path fault injection, default permissions, removed-directory recreation, concurrent creation, and lock/durability regressions. --- .../apple/coreai/runtime/coreai_bookmarks.mm | 30 ++-------- .../runtime/test/coreai_bookmark_test.mm | 58 +++++++++++++++++++ 2 files changed, 64 insertions(+), 24 deletions(-) diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.mm b/backends/apple/coreai/runtime/coreai_bookmarks.mm index 0257b1502dc..1213237b9df 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.mm +++ b/backends/apple/coreai/runtime/coreai_bookmarks.mm @@ -102,32 +102,14 @@ bool digest(id value) { // Returns -1 when the child does not exist and create is false. Result open_child(int root_fd, NSString* name, bool create) { - bool created = false; - if (create) { - const int made = retry_eintr( - [&] { return mkdirat(root_fd, name.fileSystemRepresentation, 0700); }); - ET_CHECK_OR_RETURN_ERROR( - made == 0 || errno == EEXIST, - AccessFailed, - "Cannot create Core AI assets directory"); - created = made == 0; - } - const int fd = retry_eintr([&] { - return openat( - root_fd, - name.fileSystemRepresentation, - O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); - }); - if (fd < 0 && errno == ENOENT && !create) - return -1; - ET_CHECK_OR_RETURN_ERROR( - fd >= 0, AccessFailed, "Invalid Core AI assets directory"); - if (created) { - FileDescriptor child(fd); + auto opened = open_storage_directory(root_fd, name, create); + if (!opened.ok()) + return opened.error(); + FileDescriptor child(opened.get().fd); + if (opened.get().created) { ET_CHECK_OK_OR_RETURN_ERROR(sync_storage_directory(root_fd)); - return child.release(); } - return fd; + return child.release(); } Result entry_exists(int root_fd, NSString* key) { diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm index 00ea4598c34..7d9fc96feb6 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm @@ -284,6 +284,64 @@ FileDescriptor file(open( fileExistsAtPath:[expected stringByAppendingPathComponent:key]]); } +TEST_F(CoreAIBookmarkRawTest, CreatesDirectoriesWithDefaultPermissions) { + NSURL* reference = [root.url URLByAppendingPathComponent:@"reference"]; + ASSERT_EQ(mkdir(reference.fileSystemRepresentation, 0777), 0); + struct stat expected; + ASSERT_EQ(stat(reference.fileSystemRepresentation, &expected), 0); + ASSERT_TRUE(prepare_bookmark_staging(*locked).ok()); + for (NSString* name in @[ @"locks", @"bookmarks", @"staging" ]) { + NSURL* directory = [root.url URLByAppendingPathComponent:name]; + struct stat actual; + ASSERT_EQ(stat(directory.fileSystemRepresentation, &actual), 0); + EXPECT_EQ(actual.st_mode & 0777, expected.st_mode & 0777); + } +} + +TEST_F(CoreAIBookmarkRawTest, ReusesDirectoriesWithoutCreation) { + ASSERT_TRUE(prepare_bookmark_staging(*locked).ok()); + ASSERT_EQ( + write_bookmark( + *locked, [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]), + Error::Ok); + locked.reset(); + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + for (bool create : {true, false}) { + auto reused = lock_bookmark(root.url.path, key, create); + ASSERT_TRUE(reused.ok()); + ASSERT_NE(reused.get(), nullptr); + EXPECT_TRUE(prepare_bookmark_staging(*reused.get()).ok()); + } + EXPECT_EQ(fault.matches, 0); +} + +TEST_F(CoreAIBookmarkRawTest, RecreatesMissingStagingParent) { + auto staging = prepare_bookmark_staging(*locked); + ASSERT_TRUE(staging.ok()); + ASSERT_EQ(rmdir(staging.get().fileSystemRepresentation), 0); + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + EXPECT_EQ(prepare_bookmark_staging(*locked).error(), Error::AccessFailed); + EXPECT_EQ(fault.hits, 1); + auto recreated = prepare_bookmark_staging(*locked); + ASSERT_TRUE(recreated.ok()); + EXPECT_TRUE([recreated.get() isEqual:staging.get()]); +} + +TEST_F(CoreAIBookmarkRawTest, AcceptsConcurrentStagingCreation) { + NSURL* staging = [root.url URLByAppendingPathComponent:@"staging"]; + StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); + fault.armed = false; + __block int creations = 0; + fault.observe = ^(StorageOperation operation) { + if (operation == StorageOperation::BeforeCreateDirectory) { + ++creations; + EXPECT_EQ(mkdir(staging.fileSystemRepresentation, 0700), 0); + } + }; + EXPECT_TRUE(prepare_bookmark_staging(*locked).ok()); + EXPECT_EQ(creations, 1); +} + TEST_F(CoreAIBookmarkRawTest, RejectsSymlinkedStagingParent) { TestDirectory target; ASSERT_NE(target.url, nil); From cdfeb59972f679eb95644206c196b9049d2d00e0 Mon Sep 17 00:00:00 2001 From: Scott Roy Date: Thu, 29 Oct 2026 14:04:52 -0700 Subject: [PATCH 6/6] [CoreAI] Centralize cache identity and entry locking Make the cache store own canonical model/device/specialization keys, bounded lock stripes, persistence state and bookmark operations. Separate identity, I/O and contention coverage. Snapshot the resolved root before opening the lock so mutable caller strings cannot diverge from its descriptor-pinned staging root. Test Plan: 104 native tests and CTest passed, including the reproduced mutable-root regression after its approved fix. 113 CoreAI Python tests passed during cleanup. Scoped lint and whitespace checks passed. Generic Buck validation remains unavailable in this checkout. --- backends/apple/coreai/CMakeLists.txt | 12 +- backends/apple/coreai/README.md | 42 +- .../apple/coreai/runtime/coreai_bookmarks.h | 85 +-- .../apple/coreai/runtime/coreai_bookmarks.mm | 321 +++++----- .../apple/coreai/runtime/coreai_cache_key.h | 39 ++ .../apple/coreai/runtime/coreai_cache_key.mm | 190 ++++++ .../coreai/runtime/coreai_fault_injection.h | 1 + .../test/coreai_bookmark_contention_test.mm | 287 +++++++++ .../runtime/test/coreai_bookmark_fixture.h | 15 +- .../runtime/test/coreai_bookmark_fixture.mm | 204 ++++-- .../runtime/test/coreai_bookmark_test.mm | 589 +++++------------- .../runtime/test/coreai_cache_key_test.mm | 209 +++++++ 12 files changed, 1256 insertions(+), 738 deletions(-) create mode 100644 backends/apple/coreai/runtime/coreai_cache_key.h create mode 100644 backends/apple/coreai/runtime/coreai_cache_key.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_bookmark_contention_test.mm create mode 100644 backends/apple/coreai/runtime/test/coreai_cache_key_test.mm diff --git a/backends/apple/coreai/CMakeLists.txt b/backends/apple/coreai/CMakeLists.txt index b7a6cce7226..3d36ecb2b78 100644 --- a/backends/apple/coreai/CMakeLists.txt +++ b/backends/apple/coreai/CMakeLists.txt @@ -11,9 +11,13 @@ endif() enable_language(OBJCXX) set(_coreai_runtime_sources - runtime/coreai_bookmarks.mm runtime/coreai_cache_root.mm - runtime/coreai_fault_injection.mm runtime/coreai_manifest.mm - runtime/coreai_validation.mm runtime/coreai_storage.mm + runtime/coreai_bookmarks.mm + runtime/coreai_cache_key.mm + 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) @@ -37,7 +41,9 @@ if(EXECUTORCH_BUILD_TESTS AND CMAKE_SYSTEM_NAME STREQUAL "Darwin") runtime/test/coreai_cache_root_test.mm runtime/test/coreai_storage_test.mm runtime/test/coreai_filesystem_fixture.mm + runtime/test/coreai_cache_key_test.mm runtime/test/coreai_bookmark_test.mm + runtime/test/coreai_bookmark_contention_test.mm runtime/test/coreai_bookmark_fixture.mm ${_coreai_runtime_sources} ) diff --git a/backends/apple/coreai/README.md b/backends/apple/coreai/README.md index a51c455660d..f4f90c49708 100644 --- a/backends/apple/coreai/README.md +++ b/backends/apple/coreai/README.md @@ -82,25 +82,24 @@ 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. -Each partition derives a key from its selected export digest/bundle, source/AOT -mode, platform, Core AI device architecture, default SDK cache namespace, -versioned default options and persistent policy. Function bindings and delivery -location are excluded. No model bytes are hashed at runtime. Each key determines -a raw bookmark in `bookmarks`, a bundle directory in `staging`, and one of -256 shared coordination files in `locks`. There is no PTE-wide bookmark list or -serialized lifecycle record. - +Each selected bundle has a model key derived from its export digest, bundle +name, and source/AOT mode. A readable variant key combines that model key with +the platform and Core AI device architecture, plus a hash of a versioned +specialization identity supplied as plain data by the SDK boundary. Function +bindings and delivery location are excluded, and no model bytes are hashed at +runtime. Each variant key determines a raw bookmark in `bookmarks`, a bundle +directory in `staging`, and one of 256 shared coordination files in `locks`. +There is no PTE-wide bookmark list or serialized lifecycle record. Raw bookmarks have an 8 MiB backend allocation limit, not an asserted SDK format maximum, and are published atomically because losing one orphans an SDK entry. A fixed pool of at most 256 disk locks coordinates loading and eviction across -threads and processes. The first byte of the validated cache key selects -`stripe-v1-00.lock` through `stripe-v1-ff.lock` within the lock directory. Files -are created on demand, so new model versions do not grow the pool beyond 256. -The mapping is part of the on-disk format and is not configurable. Different -keys on the same stripe serialize; different stripes remain independent. -Bookmarks and staging remain separate for every key. Runtime -operations hold only one stripe at a time. +threads and processes. A SHA-256 hash of the full validated cache key selects a +stripe modulo the configured pool size. Files are created on demand, so new +model versions do not grow the pool beyond 256. Different keys on the same +stripe serialize; different stripes remain independent. Bookmarks and staging +remain separate for every key. Runtime operations hold only one stripe at a +time. Lock ownership is maintained by the OS, and acquiring a lock does not flush its file or directory. Stripe files are never removed, including during cache @@ -108,20 +107,13 @@ clearing or after process exit, so waiters and future openers keep using the same inode. Removing the assets root externally requires all loads, sessions and maintenance to stop. -Upgrading from per-key lock files requires stopping all processes using the -root and upgrading them together. Old and new locking schemes must not run -concurrently against the same root. Legacy per-key lock files are ignored and -left untouched; they can be removed separately during a fully quiescent offline -cleanup. The 256-file bound applies to the new stripe pool, not pre-existing -legacy files. Bookmarks and staged bundles need no migration. - ## Host Tests On macOS, `EXECUTORCH_BUILD_COREAI=ON` with `EXECUTORCH_BUILD_TESTS=ON` registers the standalone `coreai_host_test` executable with CTest. It tests -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. +manifest parsing, AOT asset selection, cache identity, bookmark I/O, lock +contention, 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_bookmarks.h b/backends/apple/coreai/runtime/coreai_bookmarks.h index 8a0181719f4..23b364b45dc 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.h +++ b/backends/apple/coreai/runtime/coreai_bookmarks.h @@ -7,65 +7,72 @@ */ #pragma once -#include +#import +#include #include -#import "coreai_assets.h" #include "coreai_file.h" namespace executorch::backends::coreai { -runtime::Result bookmark_key( - const Manifest& selected, - NSString* platform, - NSString* architecture); -// The first key byte selects a stripe in a fixed, versioned lock namespace. -constexpr size_t kBookmarkLockStripeCount = 256; -runtime::Result bookmark_lock_name(NSString* key); -// With create=false, a missing root is returned unchanged for no-op +// With OpenExisting, a missing root is returned unchanged for no-op // maintenance. runtime::Result resolve_bookmark_root( NSString* path, - bool create = true); + StorageCreationMode mode); -class BookmarkLock final { +enum class CacheEntryPersistence { + BookmarkCapable, + StagingOnly, +}; + +class CacheEntryLock final { public: - BookmarkLock(const BookmarkLock&) = delete; - BookmarkLock& operator=(const BookmarkLock&) = delete; + CacheEntryLock(const CacheEntryLock&) = delete; + CacheEntryLock& operator=(const CacheEntryLock&) = delete; + + CacheEntryPersistence persistence() const { + return persistence_; + } + // Missing is nil. Empty, oversized, inaccessible, or nonregular files fail + // closed. + runtime::Result read_bookmark() const; + runtime::Error write_bookmark(NSData* data) const; + // Returns the staging parent. Source preparation appends key and bundle. + runtime::Result prepare_staging_root() const; private: - friend runtime::Result> - lock_bookmark(NSString* root, NSString* key, bool create); - friend runtime::Result read_bookmark(const BookmarkLock& lock); - friend runtime::Error write_bookmark(const BookmarkLock& lock, NSData* data); - friend runtime::Result prepare_bookmark_staging( - const BookmarkLock& lock); - BookmarkLock( + friend runtime::Result> + acquire_cache_entry_lock( + NSString* root, + NSString* key, + StorageCreationMode mode); + + CacheEntryLock( int root, int bookmarks, - int file, + int lock_file, NSString* root_path, - NSString* key); + NSString* key, + CacheEntryPersistence persistence); + FileDescriptor root_; FileDescriptor bookmarks_; - FileDescriptor file_; + FileDescriptor lock_file_; NSString* root_path_; NSString* key_; + CacheEntryPersistence persistence_; }; -// Each acquisition opens an independent descriptor on one of 256 stable -// stripes. Different keys can share a stripe; do not hold multiple -// BookmarkLocks at once. The backend never removes lock files. With -// create=false, missing roots or entries return a null lock. Existing entries -// may require creating coordination files, so the root must be writable even -// with create=false. -runtime::Result> -lock_bookmark(NSString* root, NSString* key, bool create = true); -// Missing is nil. Empty, oversized, inaccessible or nonregular files fail -// closed. -runtime::Result read_bookmark(const BookmarkLock& lock); -// A staging-only lock has no bookmark directory; writes return InvalidArgument. -runtime::Error write_bookmark(const BookmarkLock& lock, NSData* data); -// Prepared only for cold loads; source preparation appends key/bundle. -runtime::Result prepare_bookmark_staging(const BookmarkLock& lock); +// Each acquisition opens an independent descriptor on one stable stripe. +// Different keys can share a stripe; callers must not hold multiple entry locks +// at once. Stripe files are never removed by the backend. +runtime::Result> lock_cache_entry( + NSString* root, + NSString* key); +// Missing roots and entries return a null lock without changing storage. +// Existing staging-only entries return a lock with StagingOnly persistence. +runtime::Result> lock_existing_cache_entry( + NSString* root, + NSString* key); } // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_bookmarks.mm b/backends/apple/coreai/runtime/coreai_bookmarks.mm index 1213237b9df..8243fb77df2 100644 --- a/backends/apple/coreai/runtime/coreai_bookmarks.mm +++ b/backends/apple/coreai/runtime/coreai_bookmarks.mm @@ -6,9 +6,13 @@ * LICENSE file in the root directory of this source tree. */ #import "coreai_bookmarks.h" + +#import "coreai_cache_key.h" +#import "coreai_cache_root.h" +#import "coreai_fault_injection.h" #import "coreai_storage.h" +#import "coreai_validation.h" -#include #include #include #include @@ -19,92 +23,37 @@ namespace { using runtime::Error; using runtime::Result; -// An allocation bound, not an SDK bookmark format limit. + constexpr size_t kBookmarkByteLimit = 8 * 1024 * 1024; +NSString* const kBookmarksDirectory = @"bookmarks"; +NSString* const kStagingDirectory = @"staging"; +NSString* const kLocksDirectory = @"locks"; +NSString* const kBookmarkExtension = @".bookmark"; -bool text(id value) { - return [value isKindOfClass:NSString.class] && [value length] > 0 && - [value length] <= 4096 && - [value rangeOfString:@"\0"].location == NSNotFound; -} -bool component(id value) { - return text(value) && ![value containsString:@"/"] && - ![value containsString:@"\\"] && ![value isEqual:@"."] && - ![value isEqual:@".."]; -} -bool digest(id value) { - return text(value) && [value length] == 64 && - [value rangeOfCharacterFromSet:[[NSCharacterSet - characterSetWithCharactersInString: - @"0123456789abcdef"] invertedSet]] - .location == NSNotFound; -} NSString* bookmark_name(NSString* key) { - return [key stringByAppendingString:@".bookmark"]; -} -} // namespace - -Result bookmark_key( - const Manifest& selected, - NSString* platform, - NSString* architecture) { - NSString* bundle = selected.path.lastPathComponent; - NSString* exported_digest = selected.bundle_digests[bundle]; - ET_CHECK_OR_RETURN_ERROR( - digest(exported_digest) && component(bundle) && text(platform) && - text(architecture), - DelegateInvalidCompatibility, - "Core AI bookmark key requires a selected bundle and SDK architecture"); - NSArray* fields = @[ - @"1", - exported_digest, - selected.aot_compiled ? @"aot" : @"source", - bundle, - platform, - architecture, - @"AIModelCache.default", - @"default-options-v1", - @"persistent" - ]; - NSData* encoded = [NSJSONSerialization dataWithJSONObject:fields - options:0 - error:nil]; - ET_CHECK_OR_RETURN_ERROR( - encoded != nil, InvalidExternalData, "Cannot encode bookmark key"); - unsigned char bytes[CC_SHA256_DIGEST_LENGTH]; - CC_SHA256(encoded.bytes, static_cast(encoded.length), bytes); - NSMutableString* key = [NSMutableString stringWithCapacity:64]; - for (unsigned char byte : bytes) - [key appendFormat:@"%02x", byte]; - return [key copy]; + return [key stringByAppendingString:kBookmarkExtension]; } -Result bookmark_lock_name(NSString* key) { - ET_CHECK_OR_RETURN_ERROR( - digest(key), InvalidArgument, "Invalid Core AI bookmark key"); - return [NSString - stringWithFormat:@"stripe-v1-%@.lock", [key substringToIndex:2]]; -} - -namespace { // Returns -1 when the root does not exist. Result open_root(NSString* root) { const int fd = retry_eintr([&] { return open( root.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC); }); - if (fd < 0 && errno == ENOENT) + if (fd < 0 && errno == ENOENT) { return -1; + } ET_CHECK_OR_RETURN_ERROR( fd >= 0, AccessFailed, "Cannot open Core AI assets root"); return fd; } -// Returns -1 when the child does not exist and create is false. -Result open_child(int root_fd, NSString* name, bool create) { - auto opened = open_storage_directory(root_fd, name, create); - if (!opened.ok()) +// Returns -1 when the child does not exist in OpenExisting mode. +Result open_child(int root_fd, NSString* name, StorageCreationMode mode) { + auto opened = open_storage_directory(root_fd, name, mode); + if (!opened.ok()) { return opened.error(); + } FileDescriptor child(opened.get().fd); if (opened.get().created) { ET_CHECK_OK_OR_RETURN_ERROR(sync_storage_directory(root_fd)); @@ -113,24 +62,23 @@ bool digest(id value) { } Result entry_exists(int root_fd, NSString* key) { - for (NSString* name in @[ @"bookmarks", @"staging" ]) { - auto opened = open_child(root_fd, name, false); - if (!opened.ok()) + for (NSString* directory_name in + @[ kBookmarksDirectory, kStagingDirectory ]) { + auto opened = + open_child(root_fd, directory_name, StorageCreationMode::OpenExisting); + if (!opened.ok()) { return opened.error(); - FileDescriptor child(opened.get()); - if (child.get() < 0) + } + FileDescriptor directory(opened.get()); + if (directory.get() < 0) { continue; - NSString* entry = [name isEqual:@"bookmarks"] ? bookmark_name(key) : key; + } + NSString* entry = + [directory_name isEqual:kBookmarksDirectory] ? bookmark_name(key) : key; struct stat info; - const int result = retry_eintr([&] { - return fstatat( - child.get(), - entry.fileSystemRepresentation, - &info, - AT_SYMLINK_NOFOLLOW); - }); - if (result == 0) + if (lstat_at(directory.get(), entry.fileSystemRepresentation, &info) == 0) { return true; + } ET_CHECK_OR_RETURN_ERROR( errno == ENOENT, AccessFailed, "Cannot inspect Core AI cache entry"); } @@ -138,112 +86,137 @@ bool digest(id value) { } } // namespace -Result resolve_bookmark_root(NSString* path, bool create) { - ET_CHECK_OR_RETURN_ERROR( - text(path) && [path hasPrefix:@"/"], - InvalidArgument, - "Core AI assets require an absolute root path"); - if (create) { - return prepare_storage_root(path, true); +Result> acquire_cache_entry_lock( + NSString* root, + NSString* key, + StorageCreationMode mode) { + auto lock_name = cache_entry_lock_name(key); + if (!lock_name.ok()) { + return lock_name.error(); } - struct stat info; - const int result = - retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); - if (result < 0 && errno == ENOENT) - return path; - ET_CHECK_OR_RETURN_ERROR( - result == 0 && S_ISDIR(info.st_mode), - AccessFailed, - "Invalid Core AI assets root"); - return path; -} - -BookmarkLock::BookmarkLock( - int root, - int bookmarks, - int file, - NSString* root_path, - NSString* key) - : root_(root), - bookmarks_(bookmarks), - file_(file), - root_path_(root_path), - key_(key) {} - -Result> -lock_bookmark(NSString* root, NSString* key, bool create) { - auto name = bookmark_lock_name(key); - if (!name.ok()) - return name.error(); - auto resolved = resolve_bookmark_root(root, create); - if (!resolved.ok()) + auto resolved = resolve_bookmark_root(root, mode); + if (!resolved.ok()) { return resolved.error(); - root = resolved.get(); + } + root = [resolved.get() copy]; auto opened = open_root(root); - if (!opened.ok()) + if (!opened.ok()) { return opened.error(); + } FileDescriptor directory(opened.get()); + const bool create = mode == StorageCreationMode::Create; if (directory.get() < 0) { ET_CHECK_OR_RETURN_ERROR( !create, AccessFailed, "Core AI assets root disappeared"); - return std::unique_ptr(); + return std::unique_ptr(); } if (!create) { auto exists = entry_exists(directory.get(), key); - if (!exists.ok()) + if (!exists.ok()) { return exists.error(); - if (!exists.get()) - return std::unique_ptr(); + } + if (!exists.get()) { + return std::unique_ptr(); + } } - auto lock_result = open_child(directory.get(), @"locks", true); - if (!lock_result.ok()) - return lock_result.error(); - FileDescriptor locks(lock_result.get()); - FileDescriptor file(open_storage_shared_file( - locks.get(), name.get().fileSystemRepresentation)); + auto locks = + open_child(directory.get(), kLocksDirectory, StorageCreationMode::Create); + if (!locks.ok()) { + return locks.error(); + } + FileDescriptor lock_directory(locks.get()); + FileDescriptor lock_file(open_storage_shared_file( + lock_directory.get(), lock_name.get().fileSystemRepresentation)); struct stat info; ET_CHECK_OR_RETURN_ERROR( - file.get() >= 0 && fstat(file.get(), &info) == 0 && S_ISREG(info.st_mode), + lock_file.get() >= 0 && fstat(lock_file.get(), &info) == 0 && + S_ISREG(info.st_mode), AccessFailed, - "Invalid Core AI bookmark lock"); + "Invalid Core AI cache entry lock"); ET_CHECK_OR_RETURN_ERROR( - storage_fault(StorageOperation::BeforeBookmarkLock) == 0, + fault_point(FaultPoint::BeforeCacheEntryLock) == 0, AccessFailed, - "Cannot begin Core AI bookmark lock acquisition"); + "Cannot begin Core AI cache entry lock acquisition"); ET_CHECK_OR_RETURN_ERROR( - retry_eintr([&] { return flock(file.get(), LOCK_EX); }) == 0, + retry_eintr([&] { return flock(lock_file.get(), LOCK_EX); }) == 0, AccessFailed, - "Cannot lock Core AI bookmark"); + "Cannot lock Core AI cache entry"); if (!create) { auto exists = entry_exists(directory.get(), key); - if (!exists.ok()) + if (!exists.ok()) { return exists.error(); - if (!exists.get()) - return std::unique_ptr(); + } + if (!exists.get()) { + return std::unique_ptr(); + } + } + auto bookmarks = open_child(directory.get(), kBookmarksDirectory, mode); + if (!bookmarks.ok()) { + return bookmarks.error(); } - auto bookmark_result = open_child(directory.get(), @"bookmarks", create); - if (!bookmark_result.ok()) - return bookmark_result.error(); - FileDescriptor bookmarks(bookmark_result.get()); - return std::unique_ptr(new BookmarkLock( + FileDescriptor bookmark_directory(bookmarks.get()); + const CacheEntryPersistence persistence = bookmark_directory.get() >= 0 + ? CacheEntryPersistence::BookmarkCapable + : CacheEntryPersistence::StagingOnly; + return std::unique_ptr(new CacheEntryLock( directory.release(), - bookmarks.release(), - file.release(), + bookmark_directory.release(), + lock_file.release(), root, - [key copy])); + [key copy], + persistence)); +} + +Result resolve_bookmark_root( + NSString* path, + StorageCreationMode mode) { + ET_CHECK_OR_RETURN_ERROR( + nonempty_string(path) && [path hasPrefix:@"/"], + InvalidArgument, + "Core AI assets require an absolute root path"); + if (mode == StorageCreationMode::Create) { + return prepare_storage_root(path, mode); + } + struct stat info; + const int result = + retry_eintr([&] { return stat(path.fileSystemRepresentation, &info); }); + if (result < 0 && errno == ENOENT) { + return path; + } + ET_CHECK_OR_RETURN_ERROR( + result == 0 && S_ISDIR(info.st_mode), + AccessFailed, + "Invalid Core AI assets root"); + return path; } -Result read_bookmark(const BookmarkLock& lock) { - if (lock.bookmarks_.get() < 0) +CacheEntryLock::CacheEntryLock( + int root, + int bookmarks, + int lock_file, + NSString* root_path, + NSString* key, + CacheEntryPersistence persistence) + : root_(root), + bookmarks_(bookmarks), + lock_file_(lock_file), + root_path_(root_path), + key_(key), + persistence_(persistence) {} + +Result CacheEntryLock::read_bookmark() const { + if (persistence_ == CacheEntryPersistence::StagingOnly) { return static_cast(nil); + } FileDescriptor file(retry_eintr([&] { return openat( - lock.bookmarks_.get(), - bookmark_name(lock.key_).fileSystemRepresentation, + bookmarks_.get(), + bookmark_name(key_).fileSystemRepresentation, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_CLOEXEC); })); - if (file.get() < 0 && errno == ENOENT) + if (file.get() < 0 && errno == ENOENT) { return static_cast(nil); + } ET_CHECK_OR_RETURN_ERROR( file.get() >= 0, AccessFailed, "Cannot open Core AI bookmark"); struct stat info; @@ -254,14 +227,17 @@ FileDescriptor file(retry_eintr([&] { InvalidExternalData, "Invalid Core AI bookmark file"); NSMutableData* bytes = [NSMutableData dataWithLength:info.st_size]; + ET_CHECK_OR_RETURN_ERROR( + bytes != nil, MemoryAllocationFailed, "Cannot allocate Core AI bookmark"); size_t offset = 0; while (offset < bytes.length) { - ssize_t count = ::read( + const ssize_t count = ::read( file.get(), static_cast(bytes.mutableBytes) + offset, bytes.length - offset); - if (count < 0 && errno == EINTR) + if (count < 0 && errno == EINTR) { continue; + } ET_CHECK_OR_RETURN_ERROR( count > 0, AccessFailed, "Cannot read Core AI bookmark"); offset += count; @@ -269,26 +245,39 @@ FileDescriptor file(retry_eintr([&] { return bytes; } -Error write_bookmark(const BookmarkLock& lock, NSData* data) { +Error CacheEntryLock::write_bookmark(NSData* data) const { ET_CHECK_OR_RETURN_ERROR( [data isKindOfClass:NSData.class] && data.length > 0 && data.length <= kBookmarkByteLimit, InvalidExternalData, "Invalid Core AI bookmark data"); ET_CHECK_OR_RETURN_ERROR( - lock.bookmarks_.get() >= 0, + persistence_ == CacheEntryPersistence::BookmarkCapable, InvalidArgument, - "Cannot write through a staging-only Core AI bookmark lock"); - return publish_storage_data( - lock.bookmarks_.get(), bookmark_name(lock.key_), data); + "Cannot write through a staging-only Core AI cache entry lock"); + return publish_storage_data(bookmarks_.get(), bookmark_name(key_), data); } -Result prepare_bookmark_staging(const BookmarkLock& lock) { - auto staging = open_child(lock.root_.get(), @"staging", true); - if (!staging.ok()) +Result CacheEntryLock::prepare_staging_root() const { + auto staging = + open_child(root_.get(), kStagingDirectory, StorageCreationMode::Create); + if (!staging.ok()) { return staging.error(); + } FileDescriptor directory(staging.get()); - return [lock.root_path_ stringByAppendingPathComponent:@"staging"]; + return [root_path_ stringByAppendingPathComponent:kStagingDirectory]; +} + +Result> lock_cache_entry( + NSString* root, + NSString* key) { + return acquire_cache_entry_lock(root, key, StorageCreationMode::Create); +} + +Result> lock_existing_cache_entry( + NSString* root, + NSString* key) { + return acquire_cache_entry_lock(root, key, StorageCreationMode::OpenExisting); } } // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_cache_key.h b/backends/apple/coreai/runtime/coreai_cache_key.h new file mode 100644 index 00000000000..4ea30a6e1c0 --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_cache_key.h @@ -0,0 +1,39 @@ +/* + * 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 { + +class Manifest; +class SelectedBundle; + +bool is_cache_key(NSString* key); +NSString* cache_key_model_prefix(NSString* key); + +// Model identity excludes device and specialization so maintenance can find all +// variants of one exported bundle. +runtime::Result cache_model_key( + const Manifest& manifest, + NSString* bundle_name); +runtime::Result cache_model_key(const SelectedBundle& bundle); +// specialization_identity is stable, versioned plain data supplied by the SDK +// boundary. This module does not depend on SDK types. +runtime::Result cache_variant_key( + const SelectedBundle& bundle, + NSString* platform, + NSString* architecture, + NSString* specialization_identity); + +constexpr size_t kCacheEntryLockStripeCount = 256; +runtime::Result cache_entry_lock_name(NSString* key); + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_cache_key.mm b/backends/apple/coreai/runtime/coreai_cache_key.mm new file mode 100644 index 00000000000..87b0f9a9f2b --- /dev/null +++ b/backends/apple/coreai/runtime/coreai_cache_key.mm @@ -0,0 +1,190 @@ +/* + * 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_key.h" + +#include +#include +#import "coreai_manifest.h" +#import "coreai_validation.h" + +namespace executorch::backends::coreai { +namespace { +using runtime::Result; + +constexpr NSUInteger kHashLength = 2 * CC_SHA256_DIGEST_LENGTH; +constexpr NSUInteger kFixedLength = 2 * (kHashLength + 1); +constexpr NSUInteger kBookmarkExtensionLength = sizeof(".bookmark") - 1; +constexpr NSUInteger kMaxKeyLength = NAME_MAX - kBookmarkExtensionLength; + +static_assert(kCacheEntryLockStripeCount > 0); +static_assert(kCacheEntryLockStripeCount <= 256); + +bool valid_device(NSString* device) { + if (![device isKindOfClass:NSString.class] || + device.length > kMaxKeyLength - kFixedLength) { + return false; + } + const NSUInteger separator = [device rangeOfString:@"_"].location; + if (separator == NSNotFound || separator == 0 || + separator + 1 == device.length) { + return false; + } + for (NSUInteger i = 0; i < device.length; ++i) { + const unichar c = [device characterAtIndex:i]; + if (i == separator || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9')) { + continue; + } + if (i < separator || (c != '_' && c != '-')) { + return false; + } + } + return true; +} + +Result encode_json(id value) { + NSError* error = nil; + NSData* data = [NSJSONSerialization dataWithJSONObject:value + options:0 + error:&error]; + ET_CHECK_OR_RETURN_ERROR( + data != nil, + InvalidExternalData, + "Cannot encode Core AI cache identity: %s", + error.localizedDescription.UTF8String); + return data; +} + +Result hash_data(NSData* data) { + ET_CHECK_OR_RETURN_ERROR( + data != nil && data.length <= UINT_MAX, + InvalidExternalData, + "Cannot hash Core AI cache identity"); + unsigned char bytes[CC_SHA256_DIGEST_LENGTH]; + CC_SHA256(data.bytes, static_cast(data.length), bytes); + NSMutableString* result = [NSMutableString stringWithCapacity:kHashLength]; + for (unsigned char byte : bytes) { + [result appendFormat:@"%02x", byte]; + } + return [result copy]; +} + +Result +model_key(NSString* digest, NSString* bundle_name, bool aot_compiled) { + ET_CHECK_OR_RETURN_ERROR( + lowercase_hex(digest, kHashLength) && relative_path(bundle_name) && + ![bundle_name containsString:@"/"], + DelegateInvalidCompatibility, + "Invalid Core AI bundle identity"); + auto encoded = encode_json( + @[ @"1", digest, aot_compiled ? @"aot" : @"source", bundle_name ]); + if (!encoded.ok()) { + return encoded.error(); + } + return hash_data(encoded.get()); +} + +Result utf8_data(NSString* value) { + ET_CHECK_OR_RETURN_ERROR( + nonempty_string(value), + DelegateInvalidCompatibility, + "Missing Core AI cache specialization"); + NSData* data = [value dataUsingEncoding:NSUTF8StringEncoding]; + ET_CHECK_OR_RETURN_ERROR( + data != nil, InvalidExternalData, "Cannot encode Core AI specialization"); + return data; +} +} // namespace + +bool is_cache_key(NSString* key) { + if (![key isKindOfClass:NSString.class] || key.length <= kFixedLength || + key.length > kMaxKeyLength) { + return false; + } + const NSUInteger options_start = key.length - kHashLength; + if ([key characterAtIndex:kHashLength] != '-' || + [key characterAtIndex:options_start - 1] != '-') { + return false; + } + NSString* model = [key substringToIndex:kHashLength]; + NSString* options = [key substringFromIndex:options_start]; + if (!lowercase_hex(model, kHashLength) || + !lowercase_hex(options, kHashLength)) { + return false; + } + return valid_device( + [key substringWithRange:NSMakeRange( + kHashLength + 1, key.length - kFixedLength)]); +} + +NSString* cache_key_model_prefix(NSString* key) { + return is_cache_key(key) ? [key substringToIndex:kHashLength] : nil; +} + +Result cache_model_key( + const Manifest& manifest, + NSString* bundle_name) { + return model_key( + manifest.bundle_digests[bundle_name], bundle_name, manifest.aot_compiled); +} + +Result cache_model_key(const SelectedBundle& bundle) { + return model_key(bundle.digest, bundle.bundle_name, bundle.aot_compiled); +} + +Result cache_variant_key( + const SelectedBundle& bundle, + NSString* platform, + NSString* architecture, + NSString* specialization_identity) { + auto model = cache_model_key(bundle); + if (!model.ok()) { + return model.error(); + } + ET_CHECK_OR_RETURN_ERROR( + nonempty_string(platform) && nonempty_string(architecture) && + ![platform containsString:@"_"], + DelegateInvalidCompatibility, + "Invalid Core AI cache context"); + NSString* device = + [NSString stringWithFormat:@"%@_%@", platform, architecture]; + ET_CHECK_OR_RETURN_ERROR( + valid_device(device), + DelegateInvalidCompatibility, + "Invalid Core AI cache device"); + auto specialization = utf8_data(specialization_identity); + if (!specialization.ok()) { + return specialization.error(); + } + auto options = hash_data(specialization.get()); + if (!options.ok()) { + return options.error(); + } + return [NSString + stringWithFormat:@"%@-%@-%@", model.get(), device, options.get()]; +} + +Result cache_entry_lock_name(NSString* key) { + ET_CHECK_OR_RETURN_ERROR( + is_cache_key(key), InvalidArgument, "Invalid Core AI cache key"); + NSData* encoded = [key dataUsingEncoding:NSUTF8StringEncoding]; + auto hash = hash_data(encoded); + if (!hash.ok()) { + return hash.error(); + } + unsigned int first_byte = 0; + const bool parsed = + [[NSScanner scannerWithString:[hash.get() substringToIndex:2]] + scanHexInt:&first_byte]; + ET_CHECK_OR_RETURN_ERROR( + parsed, InvalidExternalData, "Cannot derive Core AI cache lock stripe"); + const size_t stripe = first_byte % kCacheEntryLockStripeCount; + return [NSString stringWithFormat:@"stripe-v1-%02zx.lock", stripe]; +} + +} // namespace executorch::backends::coreai diff --git a/backends/apple/coreai/runtime/coreai_fault_injection.h b/backends/apple/coreai/runtime/coreai_fault_injection.h index cba60c5dfb8..84947db16c8 100644 --- a/backends/apple/coreai/runtime/coreai_fault_injection.h +++ b/backends/apple/coreai/runtime/coreai_fault_injection.h @@ -17,6 +17,7 @@ enum class FaultPoint { FullSync, Rename, BeforeCreateDirectory, + BeforeCacheEntryLock, }; #if defined(COREAI_ASSETS_TESTING) && COREAI_ASSETS_TESTING diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_contention_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_contention_test.mm new file mode 100644 index 00000000000..d45614c2f38 --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_contention_test.mm @@ -0,0 +1,287 @@ +/* + * 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 +#include +#include +#include "coreai_bookmark_fixture.h" +#include "coreai_fault_scope.h" +#include "coreai_filesystem_fixture.h" + +using namespace executorch::runtime; +using namespace executorch::backends::coreai; +using namespace executorch::backends::coreai::testing; + +namespace { +constexpr auto kWaitTimeout = std::chrono::seconds(5); + +struct CacheEntryWaiter { + std::future attempting; + std::future>> result; +}; + +CacheEntryWaiter start_cache_entry_waiter( + NSString* path, + NSString* key, + bool existing_only = false) { + std::promise attempting; + std::promise>> result; + CacheEntryWaiter waiter{attempting.get_future(), result.get_future()}; + // Detached promise-backed work cannot block test teardown if acquisition + // stalls after a failed assertion. + std::thread([path, + key, + existing_only, + attempting = std::move(attempting), + result = std::move(result)]() mutable { + @autoreleasepool { + StorageFaultScope scope(FaultPoint::BeforeCacheEntryLock); + scope.armed = false; + auto* signal = &attempting; + scope.observe = ^(FaultPoint point) { + if (point == FaultPoint::BeforeCacheEntryLock) { + signal->set_value(); + } + }; + result.set_value( + existing_only ? lock_existing_cache_entry(path, key) + : lock_cache_entry(path, key)); + } + }).detach(); + return waiter; +} + +class CoreAIBookmarkContentionTest : public ::testing::Test { + protected: + TestDirectory root; + std::unique_ptr bundle; + NSString* key = nil; + + void SetUp() override { + ASSERT_NE(root.url, nil); + auto selected = bookmark_bundle(); + ASSERT_TRUE(selected.ok()); + bundle = std::make_unique(selected.get()); + auto result = test_cache_key(*bundle); + ASSERT_TRUE(result.ok()); + key = result.get(); + } + + NSURL* lock_url() { + return test_cache_lock_url(root.url, key); + } +}; + +TEST_F( + CoreAIBookmarkContentionTest, + ThreadContentionSerializesSameKeyButNotDifferentStripes) { + auto holder = lock_cache_entry(root.url.path, key); + ASSERT_TRUE(holder.ok()); + ASSERT_NE(holder.get(), nullptr); + auto waiter = start_cache_entry_waiter(root.url.path, key); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + EXPECT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + + auto independent_key = different_cache_key(*bundle, key, false); + ASSERT_TRUE(independent_key.ok()); + auto independent = + start_cache_entry_waiter(root.url.path, independent_key.get()); + ASSERT_EQ( + independent.result.wait_for(kWaitTimeout), std::future_status::ready); + auto other = independent.result.get(); + ASSERT_TRUE(other.ok()); + ASSERT_NE(other.get(), nullptr); + + holder.get().reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto acquired = waiter.result.get(); + ASSERT_TRUE(acquired.ok()); + EXPECT_NE(acquired.get(), nullptr); +} + +TEST_F(CoreAIBookmarkContentionTest, CollidingKeysSerializeAcrossThreads) { + auto collision = different_cache_key(*bundle, key, true); + ASSERT_TRUE(collision.ok()); + auto holder = lock_cache_entry(root.url.path, key); + ASSERT_TRUE(holder.ok()); + auto waiter = start_cache_entry_waiter(root.url.path, collision.get()); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + EXPECT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + holder.get().reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto acquired = waiter.result.get(); + ASSERT_TRUE(acquired.ok()); + EXPECT_NE(acquired.get(), nullptr); +} + +TEST_F( + CoreAIBookmarkContentionTest, + MissingCollidingLookupDoesNotWaitOrCreateAnotherLock) { + auto collision = different_cache_key(*bundle, key, true); + ASSERT_TRUE(collision.ok()); + auto holder = lock_cache_entry(root.url.path, key); + ASSERT_TRUE(holder.ok()); + auto waiter = start_cache_entry_waiter(root.url.path, collision.get(), true); + EXPECT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + holder.get().reset(); + auto absent = waiter.result.get(); + ASSERT_TRUE(absent.ok()); + EXPECT_EQ(absent.get(), nullptr); + NSArray* names = [NSFileManager.defaultManager + contentsOfDirectoryAtPath:[root.url URLByAppendingPathComponent:@"locks"] + .path + error:nil]; + ASSERT_NE(names, nil); + EXPECT_EQ(names.count, 1u); +} + +TEST_F(CoreAIBookmarkContentionTest, ExistingWaiterRechecksEvictedEntry) { + auto holder = lock_cache_entry(root.url.path, key); + ASSERT_TRUE(holder.ok()); + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(holder.get()->write_bookmark(bytes), Error::Ok); + auto waiter = start_cache_entry_waiter(root.url.path, key, true); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + ASSERT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + NSURL* bookmark = test_bookmark_url(root.url, key); + ASSERT_EQ(unlink(bookmark.fileSystemRepresentation), 0); + holder.get().reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto absent = waiter.result.get(); + ASSERT_TRUE(absent.ok()); + EXPECT_EQ(absent.get(), nullptr); +} + +TEST_F( + CoreAIBookmarkContentionTest, + WaiterOpensBookmarkDirectoryAfterAcquiringLock) { + auto holder = lock_cache_entry(root.url.path, key); + ASSERT_TRUE(holder.ok()); + auto waiter = start_cache_entry_waiter(root.url.path, key); + ASSERT_EQ( + waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); + ASSERT_EQ( + waiter.result.wait_for(std::chrono::milliseconds(100)), + std::future_status::timeout); + NSURL* bookmarks = [root.url URLByAppendingPathComponent:@"bookmarks"]; + ASSERT_EQ(rmdir(bookmarks.fileSystemRepresentation), 0); + ASSERT_EQ(mkdir(bookmarks.fileSystemRepresentation, 0700), 0); + holder.get().reset(); + ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); + auto acquired = waiter.result.get(); + ASSERT_TRUE(acquired.ok()); + ASSERT_NE(acquired.get(), nullptr); + NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; + ASSERT_EQ(acquired.get()->write_bookmark(bytes), Error::Ok); + EXPECT_TRUE([[NSData dataWithContentsOfURL:test_bookmark_url(root.url, key)] + isEqual:bytes]); +} + +TEST_F( + CoreAIBookmarkContentionTest, + ProcessContentionAllowsIndependentKeyAndRecoversAfterHolderDeath) { + BookmarkChild holder; + ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); + ASSERT_TRUE(holder.receive('L')); + NSURL* file = lock_url(); + struct stat before {}; + ASSERT_EQ(stat(file.fileSystemRepresentation, &before), 0); + BookmarkChild waiter; + ASSERT_TRUE(waiter.spawn(@"--bookmark-lock", root.url.path, @"same")); + ASSERT_TRUE(waiter.receive('B')); + ASSERT_TRUE(waiter.expect_blocked()); + BookmarkChild independent; + ASSERT_TRUE(independent.spawn(@"--bookmark-lock", root.url.path, @"other")); + ASSERT_TRUE(independent.receive('L')); + ASSERT_TRUE(independent.expect_exit(0)); + ASSERT_TRUE(holder.kill_and_reap()); + ASSERT_TRUE(waiter.receive('L')); + ASSERT_TRUE(waiter.expect_exit(0)); + struct stat after {}; + ASSERT_EQ(stat(file.fileSystemRepresentation, &after), 0); + EXPECT_EQ(before.st_dev, after.st_dev); + EXPECT_EQ(before.st_ino, after.st_ino); +} + +TEST_F( + CoreAIBookmarkContentionTest, + CollidingProcessWaiterRecoversAfterHolderDeath) { + BookmarkChild holder; + ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); + ASSERT_TRUE(holder.receive('L')); + struct stat before {}; + ASSERT_EQ(lstat(lock_url().fileSystemRepresentation, &before), 0); + BookmarkChild waiter; + ASSERT_TRUE(waiter.spawn(@"--bookmark-lock", root.url.path, @"collision")); + ASSERT_TRUE(waiter.receive('B')); + ASSERT_TRUE(waiter.expect_blocked()); + ASSERT_TRUE(holder.kill_and_reap()); + ASSERT_TRUE(waiter.receive('L')); + ASSERT_TRUE(waiter.expect_exit(0)); + struct stat after {}; + ASSERT_EQ(lstat(lock_url().fileSystemRepresentation, &after), 0); + EXPECT_EQ(before.st_dev, after.st_dev); + EXPECT_EQ(before.st_ino, after.st_ino); +} + +TEST_F(CoreAIBookmarkContentionTest, LockInventoryIsBoundedAndStable) { + NSMutableDictionary* representatives = + [NSMutableDictionary dictionary]; + for (NSUInteger index = 0; + index < 100000 && representatives.count < kCacheEntryLockStripeCount; + ++index) { + auto candidate = test_cache_key( + *bundle, [NSString stringWithFormat:@"inventory-%lu", index]); + ASSERT_TRUE(candidate.ok()); + auto name = cache_entry_lock_name(candidate.get()); + ASSERT_TRUE(name.ok()); + representatives[name.get()] = candidate.get(); + } + ASSERT_EQ(representatives.count, kCacheEntryLockStripeCount); + + NSMutableDictionary* inodes = + [NSMutableDictionary dictionary]; + for (NSString* name in representatives) { + auto locked = lock_cache_entry(root.url.path, representatives[name]); + ASSERT_TRUE(locked.ok()); + NSURL* file = [[root.url URLByAppendingPathComponent:@"locks"] + URLByAppendingPathComponent:name]; + struct stat info {}; + ASSERT_EQ(lstat(file.fileSystemRepresentation, &info), 0); + ASSERT_TRUE(S_ISREG(info.st_mode)); + EXPECT_EQ(info.st_size, 0); + inodes[name] = @(info.st_ino); + } + NSURL* locks = [root.url URLByAppendingPathComponent:@"locks"]; + NSArray* children = + [NSFileManager.defaultManager contentsOfDirectoryAtPath:locks.path + error:nil]; + ASSERT_EQ(children.count, kCacheEntryLockStripeCount); + + for (NSString* name in representatives) { + auto locked = lock_cache_entry(root.url.path, representatives[name]); + ASSERT_TRUE(locked.ok()); + struct stat info {}; + NSURL* file = [locks URLByAppendingPathComponent:name]; + ASSERT_EQ(lstat(file.fileSystemRepresentation, &info), 0); + EXPECT_EQ(info.st_ino, inodes[name].longLongValue); + } +} + +} // namespace diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h index 252ed85b08d..90e43b5b617 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.h @@ -13,13 +13,22 @@ #include #include #include "coreai_bookmarks.h" +#include "coreai_cache_key.h" +#include "coreai_manifest.h" namespace executorch::backends::coreai::testing { -runtime::Result bookmark_manifest(bool aot = false); -runtime::Result test_bookmark_key(const Manifest& manifest); +runtime::Result bookmark_bundle(bool aot = false); +runtime::Result test_cache_key(const SelectedBundle& bundle); +runtime::Result test_cache_key( + const SelectedBundle& bundle, + NSString* specialization_identity); +runtime::Result different_cache_key( + const SelectedBundle& bundle, + NSString* key, + bool same_stripe); NSURL* test_bookmark_url(NSURL* root, NSString* key); -NSURL* test_bookmark_lock_url(NSURL* root, NSString* key); +NSURL* test_cache_lock_url(NSURL* root, NSString* key); runtime::Result saved_bookmark(NSURL* root, NSString* key); runtime::Result sized_bookmark(size_t size, uint32_t tag = 1); diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm index 694a47670e9..d40351abeb8 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_fixture.mm @@ -49,33 +49,75 @@ bool send_signal_byte(int fd, char byte) { while (const int remaining = remaining_ms(deadline)) { pollfd entry{fd, POLLOUT, 0}; const int ready = poll(&entry, 1, remaining); - if (ready < 0 && errno == EINTR) + if (ready < 0 && errno == EINTR) { continue; - if (ready <= 0 || !(entry.revents & POLLOUT)) + } + if (ready <= 0 || !(entry.revents & POLLOUT)) { return false; + } const ssize_t written = write(fd, &byte, 1); - if (written == 1) + if (written == 1) { return true; - if (written < 0 && (errno == EINTR || errno == EAGAIN)) + } + if (written < 0 && (errno == EINTR || errno == EAGAIN)) { continue; + } return false; } return false; } } // namespace -Result bookmark_manifest(bool aot) { +Result bookmark_bundle(bool aot) { NSData* encoded = encode(aot ? aot_manifest_dict() : manifest_dict()); - if (encoded == nil) + if (encoded == nil) { return Error::InvalidExternalData; + } auto parsed = parse_manifest(encoded); - if (!parsed.ok()) + if (!parsed.ok()) { return parsed.error(); + } return select_assets(parsed.get(), @"arch_b", @"macOS"); } -Result test_bookmark_key(const Manifest& manifest) { - return bookmark_key(manifest, @"macOS", @"arch_b"); +Result test_cache_key(const SelectedBundle& bundle) { + return test_cache_key(bundle, @"default"); +} + +Result test_cache_key( + const SelectedBundle& bundle, + NSString* specialization_identity) { + return cache_variant_key( + bundle, @"macOS", @"arch_b", specialization_identity); +} + +Result different_cache_key( + const SelectedBundle& bundle, + NSString* key, + bool same_stripe) { + auto original_name = cache_entry_lock_name(key); + if (!original_name.ok()) { + return original_name.error(); + } + for (NSUInteger index = 0; index < 10000; ++index) { + NSString* identity = + [NSString stringWithFormat:@"specialization-%lu", (unsigned long)index]; + auto candidate = test_cache_key(bundle, identity); + if (!candidate.ok()) { + return candidate.error(); + } + if ([candidate.get() isEqual:key]) { + continue; + } + auto candidate_name = cache_entry_lock_name(candidate.get()); + if (!candidate_name.ok()) { + return candidate_name.error(); + } + if ([candidate_name.get() isEqual:original_name.get()] == same_stripe) { + return candidate.get(); + } + } + return Error::NotFound; } NSURL* test_bookmark_url(NSURL* root, NSString* key) { @@ -83,28 +125,32 @@ bool send_signal_byte(int fd, char byte) { URLByAppendingPathComponent:[key stringByAppendingString:@".bookmark"]]; } -NSURL* test_bookmark_lock_url(NSURL* root, NSString* key) { - auto name = bookmark_lock_name(key); +NSURL* test_cache_lock_url(NSURL* root, NSString* key) { + auto name = cache_entry_lock_name(key); EXPECT_TRUE(name.ok()); - if (!name.ok()) + if (!name.ok()) { return nil; + } return [[root URLByAppendingPathComponent:@"locks"] URLByAppendingPathComponent:name.get()]; } Result saved_bookmark(NSURL* root, NSString* key) { - auto locked = lock_bookmark(root.path, key); - if (!locked.ok()) + auto locked = lock_cache_entry(root.path, key); + if (!locked.ok()) { return locked.error(); - return read_bookmark(*locked.get()); + } + return locked.get()->read_bookmark(); } Result sized_bookmark(size_t size, uint32_t tag) { - if (size < sizeof(tag)) + if (size < sizeof(tag)) { return Error::InvalidArgument; + } NSMutableData* bytes = [NSMutableData dataWithLength:size]; - if (bytes == nil) + if (bytes == nil) { return Error::MemoryAllocationFailed; + } std::memcpy(bytes.mutableBytes, &tag, sizeof(tag)); return [bytes copy]; } @@ -114,67 +160,77 @@ void set_bookmark_test_executable(const char* path) { } int bookmark_child_mode(int argc, char** argv) { - if (argc < 2 || std::strncmp(argv[1], "--bookmark-", 11) != 0) + if (argc < 2 || std::strncmp(argv[1], "--bookmark-", 11) != 0) { return -1; - if (argc != 5) + } + if (argc != 5) { return 1; + } const bool hold = std::strcmp(argv[1], "--bookmark-hold") == 0; const bool lock = std::strcmp(argv[1], "--bookmark-lock") == 0; const bool other = std::strcmp(argv[3], "other") == 0; const bool same = std::strcmp(argv[3], "same") == 0; const bool collision = std::strcmp(argv[3], "collision") == 0; - if ((!hold && !lock) || (!other && !same && !collision)) + if ((!hold && !lock) || (!other && !same && !collision)) { return 2; + } char* end = nullptr; errno = 0; const long value = std::strtol(argv[4], &end, 10); if (errno != 0 || end == argv[4] || *end != '\0' || value < 0 || - value > INT_MAX) + value > INT_MAX) { return 3; + } FileDescriptor signal_fd(static_cast(value)); - if (fcntl(signal_fd.get(), F_GETFD) < 0) + if (fcntl(signal_fd.get(), F_GETFD) < 0) { return 4; + } signal(SIGPIPE, SIG_IGN); NSString* root = [NSString stringWithUTF8String:argv[2]]; - if (root == nil) + if (root == nil) { return 5; - auto manifest = bookmark_manifest(); - if (!manifest.ok()) + } + auto bundle = bookmark_bundle(); + if (!bundle.ok()) { return 6; - auto key = test_bookmark_key(manifest.get()); - if (!key.ok()) + } + auto key = test_cache_key(bundle.get()); + if (!key.ok()) { return 7; - NSMutableString* selected_key = [key.get() mutableCopy]; + } + NSString* selected_key = key.get(); if (other || collision) { - const NSUInteger index = other ? 0 : 63; - [selected_key - replaceCharactersInRange:NSMakeRange(index, 1) - withString:[selected_key characterAtIndex:index] == '0' - ? @"1" - : @"0"]; - } - StorageFaultScope scope(StorageOperation::BeforeBookmarkLock); + auto changed = different_cache_key(bundle.get(), key.get(), collision); + if (!changed.ok()) { + return 8; + } + selected_key = changed.get(); + } + StorageFaultScope scope(FaultPoint::BeforeCacheEntryLock); scope.armed = false; const int notify_fd = signal_fd.get(); - scope.observe = ^(StorageOperation operation) { + scope.observe = ^(FaultPoint point) { if (lock && (same || collision) && - operation == StorageOperation::BeforeBookmarkLock && + point == FaultPoint::BeforeCacheEntryLock && !send_signal_byte(notify_fd, 'B')) { - _exit(11); + _exit(12); } }; - auto locked = lock_bookmark(root, selected_key); - if (!locked.ok()) - return 8; - if (!send_signal_byte(signal_fd.get(), 'L')) + auto locked = lock_cache_entry(root, selected_key); + if (!locked.ok()) { return 9; - if (close(signal_fd.release()) != 0) + } + if (!send_signal_byte(signal_fd.get(), 'L')) { return 10; + } + if (close(signal_fd.release()) != 0) { + return 11; + } if (hold) { - // The parent owns this PID and always kills and reaps it, including on - // failure. - for (;;) + // The parent owns this PID and always kills and reaps it. + for (;;) { pause(); + } } return 0; } @@ -182,8 +238,9 @@ int bookmark_child_mode(int argc, char** argv) { BookmarkChild::~BookmarkChild() { if (pid_ > 0) { const auto cleaned = kill_and_reap(); - if (!cleaned) + if (!cleaned) { ADD_FAILURE() << cleaned.message(); + } } if (signal_fd_ >= 0 && close(signal_fd_) != 0) { ADD_FAILURE() << "Cannot close child signal descriptor: " << errno; @@ -196,8 +253,9 @@ int bookmark_child_mode(int argc, char** argv) { return AssertionFailure() << "Invalid bookmark child setup"; } int descriptors[2]; - if (pipe(descriptors) != 0) + if (pipe(descriptors) != 0) { return AssertionFailure() << "pipe: " << errno; + } FileDescriptor reader(descriptors[0]); FileDescriptor writer(descriptors[1]); for (int fd : descriptors) { @@ -208,8 +266,9 @@ int bookmark_child_mode(int argc, char** argv) { } posix_spawn_file_actions_t actions; int error = posix_spawn_file_actions_init(&actions); - if (error != 0) + if (error != 0) { return AssertionFailure() << "spawn actions: " << error; + } error = posix_spawn_file_actions_adddup2(&actions, writer.get(), kChildSignalFD); std::string descriptor = std::to_string(kChildSignalFD); @@ -230,8 +289,9 @@ int bookmark_child_mode(int argc, char** argv) { return AssertionFailure() << "posix_spawn: " << error; } signal_fd_ = reader.release(); - if (destroyed != 0) + if (destroyed != 0) { return AssertionFailure() << "destroy spawn actions: " << destroyed; + } return AssertionSuccess(); } @@ -240,19 +300,24 @@ int bookmark_child_mode(int argc, char** argv) { while (const int remaining = remaining_ms(deadline)) { pollfd entry{signal_fd_, POLLIN, 0}; const int ready = poll(&entry, 1, remaining); - if (ready < 0 && errno == EINTR) + if (ready < 0 && errno == EINTR) { continue; - if (ready < 0) + } + if (ready < 0) { return AssertionFailure() << "poll: " << errno; - if (ready == 0) + } + if (ready == 0) { break; - if (!(entry.revents & POLLIN)) + } + if (!(entry.revents & POLLIN)) { return AssertionFailure() << "Child closed signal pipe before " << expected; + } char byte = 0; const ssize_t count = read(signal_fd_, &byte, 1); - if (count < 0 && (errno == EINTR || errno == EAGAIN)) + if (count < 0 && (errno == EINTR || errno == EAGAIN)) { continue; + } if (count != 1 || byte != expected) { return AssertionFailure() << "Expected child signal " << expected << ", got " << int(byte); @@ -268,20 +333,24 @@ int bookmark_child_mode(int argc, char** argv) { while (const int remaining = remaining_ms(deadline)) { pollfd entry{signal_fd_, POLLIN, 0}; const int ready = poll(&entry, 1, remaining); - if (ready < 0 && errno == EINTR) + if (ready < 0 && errno == EINTR) { continue; - if (ready < 0) + } + if (ready < 0) { return AssertionFailure() << "poll: " << errno; - if (ready > 0) + } + if (ready > 0) { return AssertionFailure() << "Child completed while lock was held"; + } return AssertionSuccess(); } return AssertionSuccess(); } ::testing::AssertionResult BookmarkChild::reap(int& status) { - if (pid_ <= 0) + if (pid_ <= 0) { return AssertionFailure() << "No owned child to reap"; + } const auto deadline = Clock::now() + kChildTimeout; while (const int remaining = remaining_ms(deadline)) { const pid_t result = waitpid(pid_, &status, WNOHANG); @@ -289,10 +358,12 @@ int bookmark_child_mode(int argc, char** argv) { pid_ = -1; return AssertionSuccess(); } - if (result < 0 && errno == EINTR) + if (result < 0 && errno == EINTR) { continue; - if (result < 0) + } + if (result < 0) { return AssertionFailure() << "waitpid: " << errno; + } poll(nullptr, 0, std::min(remaining, 10)); } return AssertionFailure() << "Timed out reaping child " << pid_; @@ -301,8 +372,9 @@ int bookmark_child_mode(int argc, char** argv) { ::testing::AssertionResult BookmarkChild::expect_exit(int expected) { int status = 0; auto result = reap(status); - if (!result) + if (!result) { return result; + } if (!WIFEXITED(status) || WEXITSTATUS(status) != expected) { return AssertionFailure() << "Expected child exit " << expected << ", wait status " << status; @@ -311,14 +383,16 @@ int bookmark_child_mode(int argc, char** argv) { } ::testing::AssertionResult BookmarkChild::kill_and_reap() { - if (pid_ <= 0) + if (pid_ <= 0) { return AssertionFailure() << "No owned child to kill"; + } const int killed = kill(pid_, SIGKILL); const int error = errno; int status = 0; auto result = reap(status); - if (!result) + if (!result) { return result; + } if (killed != 0 || !WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) { return AssertionFailure() << "Expected SIGKILL, kill errno " << (killed == 0 ? 0 : error) diff --git a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm index 7d9fc96feb6..949c5b4ade8 100644 --- a/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm +++ b/backends/apple/coreai/runtime/test/coreai_bookmark_test.mm @@ -9,14 +9,9 @@ #include #include #include -#include -#include -#include -#include #include "coreai_bookmark_fixture.h" #include "coreai_fault_scope.h" #include "coreai_filesystem_fixture.h" -#include "coreai_manifest_fixture.h" #include "coreai_storage.h" using namespace executorch::runtime; @@ -25,100 +20,55 @@ namespace { constexpr size_t kMiB = 1024 * 1024; -constexpr auto kWaitTimeout = std::chrono::seconds(5); - -NSString* changed_key(NSString* key, bool same_stripe) { - NSMutableString* changed = [key mutableCopy]; - const NSUInteger index = same_stripe ? 63 : 0; - [changed replaceCharactersInRange:NSMakeRange(index, 1) - withString:[key characterAtIndex:index] == '0' ? @"1" - : @"0"]; - return changed; -} - -struct BookmarkWaiter { - std::future attempting; - std::future>> result; -}; - -BookmarkWaiter -start_bookmark_waiter(NSString* path, NSString* key, bool create = true) { - std::promise attempting; - std::promise>> result; - BookmarkWaiter waiter{attempting.get_future(), result.get_future()}; - // Promise-backed futures cannot block teardown if acquisition stalls. - std::thread([path, - key, - create, - attempting = std::move(attempting), - result = std::move(result)]() mutable { - @autoreleasepool { - StorageFaultScope scope(StorageOperation::BeforeBookmarkLock); - scope.armed = false; - auto* signal = &attempting; - scope.observe = ^(StorageOperation operation) { - if (operation == StorageOperation::BeforeBookmarkLock) { - signal->set_value(); - } - }; - result.set_value(lock_bookmark(path, key, create)); - } - }).detach(); - return waiter; -} -class CoreAIBookmarkIdentityTest : public ::testing::Test { +class CoreAIBookmarkPathTest : public ::testing::Test { protected: - Manifest manifest; + TestDirectory root; + std::unique_ptr bundle; NSString* key = nil; void SetUp() override { - auto selected = bookmark_manifest(); + ASSERT_NE(root.url, nil); + auto selected = bookmark_bundle(); ASSERT_TRUE(selected.ok()); - manifest = selected.get(); - auto result = test_bookmark_key(manifest); + bundle = std::make_unique(selected.get()); + auto result = test_cache_key(*bundle); ASSERT_TRUE(result.ok()); key = result.get(); } -}; -class CoreAIBookmarkPathTest : public CoreAIBookmarkIdentityTest { - protected: - TestDirectory root; - void SetUp() override { - CoreAIBookmarkIdentityTest::SetUp(); - if (HasFatalFailure()) - return; - ASSERT_NE(root.url, nil); - } NSURL* lock_url() { - return test_bookmark_lock_url(root.url, key); + return test_cache_lock_url(root.url, key); } }; -class CoreAIBookmarkRawTest : public CoreAIBookmarkPathTest { +class CoreAIBookmarkIOTest : public CoreAIBookmarkPathTest { protected: - std::unique_ptr locked; + std::unique_ptr locked; NSURL* url = nil; + void SetUp() override { CoreAIBookmarkPathTest::SetUp(); - if (HasFatalFailure()) + if (HasFatalFailure()) { return; - auto result = lock_bookmark(root.url.path, key); + } + auto result = lock_cache_entry(root.url.path, key); ASSERT_TRUE(result.ok()); locked = std::move(result.get()); ASSERT_NE(locked, nullptr); + ASSERT_EQ(locked->persistence(), CacheEntryPersistence::BookmarkCapable); url = test_bookmark_url(root.url, key); } }; ::testing::AssertionResult bookmark_bytes_equal( - const BookmarkLock& lock, + const CacheEntryLock& lock, NSData* expected) { - auto result = read_bookmark(lock); - if (!result.ok()) + auto result = lock.read_bookmark(); + if (!result.ok()) { return ::testing::AssertionFailure() << "Bookmark read error " << int(result.error()); + } if (expected == nil ? result.get() != nil : ![result.get() isEqual:expected]) { return ::testing::AssertionFailure() << "Bookmark bytes differ"; @@ -130,67 +80,32 @@ void SetUp() override { NSURL* directory = [root URLByAppendingPathComponent:@"bookmarks"]; FileDescriptor fd(open( directory.fileSystemRepresentation, O_RDONLY | O_DIRECTORY | O_CLOEXEC)); - if (fd.get() < 0) + if (fd.get() < 0) { return ::testing::AssertionFailure() << "open bookmark directory: " << errno; + } auto children = storage_children(fd.get()); - if (!children.ok()) + if (!children.ok()) { return ::testing::AssertionFailure() << "Cannot enumerate bookmark directory"; + } for (NSString* name in children.get()) { - if ([name hasPrefix:@".tmp-"]) + if ([name hasPrefix:@".tmp-"]) { return ::testing::AssertionFailure() << "Leaked " << name.UTF8String; + } } return ::testing::AssertionSuccess(); } -TEST_F(CoreAIBookmarkIdentityTest, IgnoresFunctionBindingsAndDeliveryLocation) { - EXPECT_EQ(key.length, 64u); - manifest.function = @"another_function"; - manifest.inputs = @[ @"different_input" ]; - manifest.outputs = @[ @"different_output" ]; - manifest.hash = @"another_delivery_location"; - auto changed = test_bookmark_key(manifest); - ASSERT_TRUE(changed.ok()); - EXPECT_TRUE([changed.get() isEqual:key]); -} - -TEST_F( - CoreAIBookmarkIdentityTest, - KeySeparatesPlatformArchitectureWeightsAndAotContext) { - auto platform = bookmark_key(manifest, @"iOS", @"arch_b"); - auto architecture = bookmark_key(manifest, @"macOS", @"arch_a"); - Manifest reweighted = manifest; - reweighted.bundle_digests = @{@"model.aimodel" : fixture_identity('2')}; - auto weights = test_bookmark_key(reweighted); - auto aot_manifest = bookmark_manifest(true); - ASSERT_TRUE(aot_manifest.ok()); - auto aot = test_bookmark_key(aot_manifest.get()); - for (auto* changed : {&platform, &architecture, &weights, &aot}) { - ASSERT_TRUE(changed->ok()); - EXPECT_FALSE([changed->get() isEqual:key]); - } -} - -TEST_F(CoreAIBookmarkIdentityTest, RejectsMissingArchitecture) { - EXPECT_FALSE(bookmark_key(manifest, @"macOS", nil).ok()); - EXPECT_FALSE(bookmark_key(manifest, @"macOS", @"").ok()); -} - -TEST_F(CoreAIBookmarkIdentityTest, RejectsUnselectedBundle) { - manifest.path = @"../escape"; - EXPECT_FALSE(test_bookmark_key(manifest).ok()); -} - -TEST_F(CoreAIBookmarkRawTest, MissingBookmarkIsAnEmptySuccessfulRead) { +TEST_F(CoreAIBookmarkIOTest, MissingBookmarkIsAnEmptySuccessfulRead) { EXPECT_TRUE(bookmark_bytes_equal(*locked, nil)); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:url.path]); } -TEST_F(CoreAIBookmarkRawTest, RoundTripsMaximumBookmarkAndReopens) { +TEST_F(CoreAIBookmarkIOTest, RoundTripsMaximumBookmarkAndReopens) { auto bytes = sized_bookmark(8 * kMiB); ASSERT_TRUE(bytes.ok()); - ASSERT_EQ(write_bookmark(*locked, bytes.get()), Error::Ok); + ASSERT_EQ(locked->write_bookmark(bytes.get()), Error::Ok); EXPECT_TRUE(bookmark_bytes_equal(*locked, bytes.get())); EXPECT_TRUE([[NSData dataWithContentsOfURL:url] isEqual:bytes.get()]); locked.reset(); @@ -199,97 +114,111 @@ FileDescriptor fd(open( EXPECT_TRUE([saved.get() isEqual:bytes.get()]); } -TEST_F(CoreAIBookmarkRawTest, InvalidWritesPreserveMaximumBookmark) { +TEST_F(CoreAIBookmarkIOTest, InvalidWritesPreserveExistingBookmark) { auto maximum = sized_bookmark(8 * kMiB); ASSERT_TRUE(maximum.ok()); - ASSERT_EQ(write_bookmark(*locked, maximum.get()), Error::Ok); + ASSERT_EQ(locked->write_bookmark(maximum.get()), Error::Ok); auto oversized = sized_bookmark(8 * kMiB + 1); ASSERT_TRUE(oversized.ok()); for (NSData* invalid : {[NSData data], oversized.get()}) { SCOPED_TRACE(invalid.length); - EXPECT_EQ(write_bookmark(*locked, invalid), Error::InvalidExternalData); + EXPECT_EQ(locked->write_bookmark(invalid), Error::InvalidExternalData); EXPECT_TRUE([[NSData dataWithContentsOfURL:url] isEqual:maximum.get()]); EXPECT_TRUE(bookmark_bytes_equal(*locked, maximum.get())); EXPECT_TRUE(no_temporary_bookmarks(root.url)); } } -TEST_F(CoreAIBookmarkRawTest, RejectsEmptyStoredBookmark) { +TEST_F(CoreAIBookmarkIOTest, RejectsEmptyStoredBookmark) { FileDescriptor file(open( url.fileSystemRepresentation, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0600)); ASSERT_GE(file.get(), 0); - EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); + EXPECT_EQ(locked->read_bookmark().error(), Error::InvalidExternalData); } -TEST_F(CoreAIBookmarkRawTest, RejectsSymlinkedBookmarkWithoutReadingTarget) { +TEST_F(CoreAIBookmarkIOTest, RejectsSymlinkWithoutReadingTarget) { NSURL* target = [root.url URLByAppendingPathComponent:@"target"]; NSData* bytes = [@"target bytes" dataUsingEncoding:NSUTF8StringEncoding]; ASSERT_TRUE([bytes writeToURL:target atomically:NO]); ASSERT_EQ( symlink(target.fileSystemRepresentation, url.fileSystemRepresentation), 0); - EXPECT_EQ(read_bookmark(*locked).error(), Error::AccessFailed); - EXPECT_TRUE(([[NSData dataWithContentsOfURL:target] isEqual:bytes])); + EXPECT_EQ(locked->read_bookmark().error(), Error::AccessFailed); + EXPECT_TRUE([[NSData dataWithContentsOfURL:target] isEqual:bytes]); } -TEST_F(CoreAIBookmarkRawTest, RejectsDirectoryAtBookmarkPath) { +TEST_F(CoreAIBookmarkIOTest, RejectsNonregularStoredBookmarks) { ASSERT_EQ(mkdir(url.fileSystemRepresentation, 0700), 0); - EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); -} - -TEST_F(CoreAIBookmarkRawTest, RejectsFifoWithoutWaitingForWriter) { + EXPECT_EQ(locked->read_bookmark().error(), Error::InvalidExternalData); + ASSERT_EQ(rmdir(url.fileSystemRepresentation), 0); ASSERT_EQ(mkfifo(url.fileSystemRepresentation, 0600), 0); - EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); + EXPECT_EQ(locked->read_bookmark().error(), Error::InvalidExternalData); } -TEST_F(CoreAIBookmarkRawTest, RejectsInaccessibleBookmark) { +TEST_F(CoreAIBookmarkIOTest, RejectsInaccessibleBookmark) { if (geteuid() == 0) { GTEST_SKIP() << "Root can bypass file read permissions"; } NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_EQ(write_bookmark(*locked, bytes), Error::Ok); + ASSERT_EQ(locked->write_bookmark(bytes), Error::Ok); ASSERT_EQ(chmod(url.fileSystemRepresentation, 0000), 0); - EXPECT_EQ(read_bookmark(*locked).error(), Error::AccessFailed); + EXPECT_EQ(locked->read_bookmark().error(), Error::AccessFailed); ASSERT_EQ(chmod(url.fileSystemRepresentation, 0600), 0); EXPECT_TRUE(bookmark_bytes_equal(*locked, bytes)); } -TEST_F(CoreAIBookmarkRawTest, NoncreatingLookupReadsExistingBookmark) { +TEST_F(CoreAIBookmarkIOTest, RejectsOversizedStoredBookmark) { + FileDescriptor file(open( + url.fileSystemRepresentation, + O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, + 0600)); + ASSERT_GE(file.get(), 0); + ASSERT_EQ(ftruncate(file.get(), 8 * kMiB + 1), 0); + ASSERT_EQ(close(file.release()), 0); + EXPECT_EQ(locked->read_bookmark().error(), Error::InvalidExternalData); + struct stat info {}; + ASSERT_EQ(lstat(url.fileSystemRepresentation, &info), 0); + EXPECT_EQ(info.st_size, static_cast(8 * kMiB + 1)); +} + +TEST_F(CoreAIBookmarkIOTest, ExistingLookupReadsWithoutPreparingStaging) { NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_EQ(write_bookmark(*locked, bytes), Error::Ok); + ASSERT_EQ(locked->write_bookmark(bytes), Error::Ok); locked.reset(); - auto existing = lock_bookmark(root.url.path, key, false); + auto existing = lock_existing_cache_entry(root.url.path, key); ASSERT_TRUE(existing.ok()); ASSERT_NE(existing.get(), nullptr); + EXPECT_EQ( + existing.get()->persistence(), CacheEntryPersistence::BookmarkCapable); EXPECT_TRUE(bookmark_bytes_equal(*existing.get(), bytes)); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:[root.url URLByAppendingPathComponent:@"staging"].path]); } -TEST_F(CoreAIBookmarkRawTest, PreparesSharedStagingParentOnDemand) { +TEST_F(CoreAIBookmarkIOTest, PreparesSharedStagingParentOnDemand) { NSString* expected = [root.url URLByAppendingPathComponent:@"staging"].path; EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:expected]); - auto staging = prepare_bookmark_staging(*locked); + auto staging = locked->prepare_staging_root(); ASSERT_TRUE(staging.ok()); EXPECT_TRUE([staging.get() isEqual:expected]); struct stat info {}; ASSERT_EQ(lstat(staging.get().fileSystemRepresentation, &info), 0); EXPECT_TRUE(S_ISDIR(info.st_mode)); - auto repeated = prepare_bookmark_staging(*locked); + auto repeated = locked->prepare_staging_root(); ASSERT_TRUE(repeated.ok()); EXPECT_TRUE([repeated.get() isEqual:expected]); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:[expected stringByAppendingPathComponent:key]]); } -TEST_F(CoreAIBookmarkRawTest, CreatesDirectoriesWithDefaultPermissions) { +TEST_F(CoreAIBookmarkIOTest, CreatesDirectoriesWithDefaultPermissions) { NSURL* reference = [root.url URLByAppendingPathComponent:@"reference"]; ASSERT_EQ(mkdir(reference.fileSystemRepresentation, 0777), 0); struct stat expected; ASSERT_EQ(stat(reference.fileSystemRepresentation, &expected), 0); - ASSERT_TRUE(prepare_bookmark_staging(*locked).ok()); + ASSERT_TRUE(locked->prepare_staging_root().ok()); for (NSString* name in @[ @"locks", @"bookmarks", @"staging" ]) { NSURL* directory = [root.url URLByAppendingPathComponent:name]; struct stat actual; @@ -298,51 +227,54 @@ FileDescriptor file(open( } } -TEST_F(CoreAIBookmarkRawTest, ReusesDirectoriesWithoutCreation) { - ASSERT_TRUE(prepare_bookmark_staging(*locked).ok()); +TEST_F(CoreAIBookmarkIOTest, ReusesDirectoriesWithoutCreation) { + ASSERT_TRUE(locked->prepare_staging_root().ok()); ASSERT_EQ( - write_bookmark( - *locked, [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]), + locked->write_bookmark( + [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]), Error::Ok); locked.reset(); - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); - for (bool create : {true, false}) { - auto reused = lock_bookmark(root.url.path, key, create); - ASSERT_TRUE(reused.ok()); - ASSERT_NE(reused.get(), nullptr); - EXPECT_TRUE(prepare_bookmark_staging(*reused.get()).ok()); - } + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); + auto created = lock_cache_entry(root.url.path, key); + ASSERT_TRUE(created.ok()); + ASSERT_NE(created.get(), nullptr); + EXPECT_TRUE(created.get()->prepare_staging_root().ok()); + created.get().reset(); + auto existing = lock_existing_cache_entry(root.url.path, key); + ASSERT_TRUE(existing.ok()); + ASSERT_NE(existing.get(), nullptr); + EXPECT_TRUE(existing.get()->prepare_staging_root().ok()); EXPECT_EQ(fault.matches, 0); } -TEST_F(CoreAIBookmarkRawTest, RecreatesMissingStagingParent) { - auto staging = prepare_bookmark_staging(*locked); +TEST_F(CoreAIBookmarkIOTest, RecreatesMissingStagingParent) { + auto staging = locked->prepare_staging_root(); ASSERT_TRUE(staging.ok()); ASSERT_EQ(rmdir(staging.get().fileSystemRepresentation), 0); - StorageFaultScope fault(StorageOperation::BeforeCreateDirectory); - EXPECT_EQ(prepare_bookmark_staging(*locked).error(), Error::AccessFailed); + StorageFaultScope fault(FaultPoint::BeforeCreateDirectory); + EXPECT_EQ(locked->prepare_staging_root().error(), Error::AccessFailed); EXPECT_EQ(fault.hits, 1); - auto recreated = prepare_bookmark_staging(*locked); + auto recreated = locked->prepare_staging_root(); ASSERT_TRUE(recreated.ok()); EXPECT_TRUE([recreated.get() isEqual:staging.get()]); } -TEST_F(CoreAIBookmarkRawTest, AcceptsConcurrentStagingCreation) { +TEST_F(CoreAIBookmarkIOTest, AcceptsConcurrentStagingCreation) { NSURL* staging = [root.url URLByAppendingPathComponent:@"staging"]; - 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 point) { + if (point == FaultPoint::BeforeCreateDirectory) { ++creations; EXPECT_EQ(mkdir(staging.fileSystemRepresentation, 0700), 0); } }; - EXPECT_TRUE(prepare_bookmark_staging(*locked).ok()); + EXPECT_TRUE(locked->prepare_staging_root().ok()); EXPECT_EQ(creations, 1); } -TEST_F(CoreAIBookmarkRawTest, RejectsSymlinkedStagingParent) { +TEST_F(CoreAIBookmarkIOTest, RejectsInvalidStagingParent) { TestDirectory target; ASSERT_NE(target.url, nil); NSURL* staging = [root.url URLByAppendingPathComponent:@"staging"]; @@ -351,62 +283,59 @@ FileDescriptor file(open( target.url.fileSystemRepresentation, staging.fileSystemRepresentation), 0); - EXPECT_EQ(prepare_bookmark_staging(*locked).error(), Error::AccessFailed); - NSArray* contents = - [NSFileManager.defaultManager contentsOfDirectoryAtPath:target.url.path - error:nil]; - ASSERT_NE(contents, nil); - EXPECT_EQ(contents.count, 0u); -} - -TEST_F(CoreAIBookmarkRawTest, RejectsFileAtStagingParent) { - NSURL* staging = [root.url URLByAppendingPathComponent:@"staging"]; + EXPECT_EQ(locked->prepare_staging_root().error(), Error::AccessFailed); + ASSERT_EQ(unlink(staging.fileSystemRepresentation), 0); NSData* bytes = [@"not a directory" dataUsingEncoding:NSUTF8StringEncoding]; ASSERT_TRUE([bytes writeToURL:staging atomically:NO]); - EXPECT_EQ(prepare_bookmark_staging(*locked).error(), Error::AccessFailed); - EXPECT_TRUE(([[NSData dataWithContentsOfURL:staging] isEqual:bytes])); -} - -TEST_F(CoreAIBookmarkRawTest, RejectsOversizedStoredBookmark) { - FileDescriptor file(open( - url.fileSystemRepresentation, - O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, - 0600)); - ASSERT_GE(file.get(), 0); - ASSERT_EQ(ftruncate(file.get(), 8 * kMiB + 1), 0); - ASSERT_EQ(close(file.release()), 0); - EXPECT_EQ(read_bookmark(*locked).error(), Error::InvalidExternalData); - struct stat info {}; - ASSERT_EQ(lstat(url.fileSystemRepresentation, &info), 0); - EXPECT_EQ(info.st_size, static_cast(8 * kMiB + 1)); + EXPECT_EQ(locked->prepare_staging_root().error(), Error::AccessFailed); + EXPECT_TRUE([[NSData dataWithContentsOfURL:staging] isEqual:bytes]); } TEST_F(CoreAIBookmarkPathTest, AllowsTemporaryRootsAndRejectsInvalidKeys) { - auto allowed = lock_bookmark(root.url.path, key); + auto allowed = lock_cache_entry(root.url.path, key); ASSERT_TRUE(allowed.ok()); ASSERT_NE(allowed.get(), nullptr); - EXPECT_FALSE(lock_bookmark(root.url.path, @"../escape").ok()); + EXPECT_FALSE(lock_cache_entry(root.url.path, @"../escape").ok()); +} + +TEST_F(CoreAIBookmarkPathTest, SnapshotsMutableRootAtAcquisition) { + TestDirectory other; + ASSERT_NE(other.url, nil); + NSMutableString* path = [root.url.path mutableCopy]; + auto locked = lock_cache_entry(path, key); + ASSERT_TRUE(locked.ok()); + ASSERT_NE(locked.get(), nullptr); + [path setString:other.url.path]; + + auto staging = locked.get()->prepare_staging_root(); + ASSERT_TRUE(staging.ok()); + NSString* expected = [root.url URLByAppendingPathComponent:@"staging"].path; + NSString* unrelated = [other.url URLByAppendingPathComponent:@"staging"].path; + EXPECT_TRUE([NSFileManager.defaultManager fileExistsAtPath:expected]); + EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:unrelated]); + EXPECT_TRUE([staging.get() isEqual:expected]) + << "Returned staging path: " << staging.get().UTF8String + << "; descriptor-pinned staging path: " << expected.UTF8String; } TEST_F(CoreAIBookmarkPathTest, ExcludesRootWithoutCreatingStaging) { - auto locked = lock_bookmark(root.url.path, key); + auto locked = lock_cache_entry(root.url.path, key); ASSERT_TRUE(locked.ok()); EXPECT_TRUE(backup_excluded(root.url)); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:[root.url URLByAppendingPathComponent:@"staging"].path]); } -TEST_F( - CoreAIBookmarkPathTest, - NoncreatingLookupLeavesMissingRootAndEntryUntouched) { +TEST_F(CoreAIBookmarkPathTest, MissingLookupLeavesRootAndEntryUntouched) { ASSERT_TRUE(set_backup_excluded(root.url, false)); NSString* missing = [root.url URLByAppendingPathComponent:@"missing"].path; for (NSString* path in @[ root.url.path, missing ]) { SCOPED_TRACE(path.UTF8String); - auto resolved = resolve_bookmark_root(path, false); + auto resolved = + resolve_bookmark_root(path, StorageCreationMode::OpenExisting); ASSERT_TRUE(resolved.ok()); EXPECT_TRUE([resolved.get() isEqual:path]); - auto absent = lock_bookmark(path, key, false); + auto absent = lock_existing_cache_entry(path, key); ASSERT_TRUE(absent.ok()); EXPECT_EQ(absent.get(), nullptr); } @@ -418,33 +347,34 @@ FileDescriptor file(open( EXPECT_FALSE(backup_excluded(root.url)); } -TEST_F(CoreAIBookmarkPathTest, NoncreatingLookupIgnoresOtherEntries) { - NSString* other_key = changed_key(key, false); - auto other = lock_bookmark(root.url.path, other_key); +TEST_F(CoreAIBookmarkPathTest, MissingLookupIgnoresOtherEntries) { + auto other_key = different_cache_key(*bundle, key, false); + ASSERT_TRUE(other_key.ok()); + auto other = lock_cache_entry(root.url.path, other_key.get()); ASSERT_TRUE(other.ok()); - ASSERT_NE(other.get(), nullptr); NSData* bytes = [@"other" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_EQ(write_bookmark(*other.get(), bytes), Error::Ok); - auto absent = lock_bookmark(root.url.path, key, false); + ASSERT_EQ(other.get()->write_bookmark(bytes), Error::Ok); + auto absent = lock_existing_cache_entry(root.url.path, key); ASSERT_TRUE(absent.ok()); EXPECT_EQ(absent.get(), nullptr); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); EXPECT_TRUE(bookmark_bytes_equal(*other.get(), bytes)); } -TEST_F(CoreAIBookmarkPathTest, NoncreatingLookupLocksStagingOnlyEntry) { +TEST_F(CoreAIBookmarkPathTest, StagingOnlyLookupHasExplicitPersistence) { NSURL* staged = [[root.url URLByAppendingPathComponent:@"staging"] URLByAppendingPathComponent:key]; ASSERT_TRUE([NSFileManager.defaultManager createDirectoryAtURL:staged withIntermediateDirectories:YES attributes:nil error:nil]); - auto existing = lock_bookmark(root.url.path, key, false); + auto existing = lock_existing_cache_entry(root.url.path, key); ASSERT_TRUE(existing.ok()); ASSERT_NE(existing.get(), nullptr); + EXPECT_EQ(existing.get()->persistence(), CacheEntryPersistence::StagingOnly); EXPECT_TRUE(bookmark_bytes_equal(*existing.get(), nil)); NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; - EXPECT_EQ(write_bookmark(*existing.get(), bytes), Error::InvalidArgument); + EXPECT_EQ(existing.get()->write_bookmark(bytes), Error::InvalidArgument); EXPECT_TRUE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); EXPECT_FALSE([NSFileManager.defaultManager fileExistsAtPath:[root.url URLByAppendingPathComponent:@"bookmarks"] @@ -454,183 +384,43 @@ FileDescriptor file(open( TEST_F(CoreAIBookmarkPathTest, ResolvesAndCreatesRootWithBackupExclusion) { ASSERT_TRUE(set_backup_excluded(root.url, false)); NSString* path = [root.url URLByAppendingPathComponent:@"nested/assets"].path; - auto resolved = resolve_bookmark_root(path); + auto resolved = resolve_bookmark_root(path, StorageCreationMode::Create); ASSERT_TRUE(resolved.ok()); EXPECT_TRUE([resolved.get() isEqual:path]); EXPECT_TRUE(backup_excluded([NSURL fileURLWithPath:resolved.get()])); EXPECT_FALSE(backup_excluded(root.url)); - auto existing = resolve_bookmark_root(path, false); + auto existing = + resolve_bookmark_root(path, StorageCreationMode::OpenExisting); ASSERT_TRUE(existing.ok()); EXPECT_TRUE([existing.get() isEqual:path]); } TEST_F(CoreAIBookmarkPathTest, RejectsInvalidResolvedRoots) { - for (bool create : {false, true}) { + for (StorageCreationMode mode : + {StorageCreationMode::OpenExisting, StorageCreationMode::Create}) { for (NSString* path in @[ @"relative", @"~", @"~/cache", @"~nobody/cache" ]) { SCOPED_TRACE(path.UTF8String); EXPECT_EQ( - resolve_bookmark_root(path, create).error(), Error::InvalidArgument); + resolve_bookmark_root(path, mode).error(), Error::InvalidArgument); } - EXPECT_EQ( - resolve_bookmark_root(nil, create).error(), Error::InvalidArgument); + EXPECT_EQ(resolve_bookmark_root(nil, mode).error(), Error::InvalidArgument); } NSURL* file = [root.url URLByAppendingPathComponent:@"file"]; ASSERT_TRUE([[NSData data] writeToURL:file atomically:NO]); EXPECT_EQ( - resolve_bookmark_root(file.path, false).error(), Error::AccessFailed); + resolve_bookmark_root(file.path, StorageCreationMode::OpenExisting) + .error(), + Error::AccessFailed); EXPECT_EQ( resolve_bookmark_root( - [file.path stringByAppendingPathComponent:@"child"], false) + [file.path stringByAppendingPathComponent:@"child"], + StorageCreationMode::OpenExisting) .error(), Error::AccessFailed); } -TEST_F( - CoreAIBookmarkPathTest, - ThreadContentionSerializesSameKeyButNotDifferentStripes) { - auto holder = lock_bookmark(root.url.path, key); - ASSERT_TRUE(holder.ok()); - ASSERT_NE(holder.get(), nullptr); - auto waiter = start_bookmark_waiter(root.url.path, key); - ASSERT_EQ( - waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); - EXPECT_EQ( - waiter.result.wait_for(std::chrono::milliseconds(100)), - std::future_status::timeout); - auto independent = - start_bookmark_waiter(root.url.path, changed_key(key, false)); - ASSERT_EQ( - independent.result.wait_for(kWaitTimeout), std::future_status::ready); - auto other = independent.result.get(); - ASSERT_TRUE(other.ok()); - ASSERT_NE(other.get(), nullptr); - holder.get().reset(); - ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); - auto acquired = waiter.result.get(); - ASSERT_TRUE(acquired.ok()); - EXPECT_NE(acquired.get(), nullptr); -} - -TEST_F(CoreAIBookmarkRawTest, NoncreatingWaiterRechecksEvictedEntry) { - NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_EQ(write_bookmark(*locked, bytes), Error::Ok); - auto waiter = start_bookmark_waiter(root.url.path, key, false); - ASSERT_EQ( - waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); - ASSERT_EQ( - waiter.result.wait_for(std::chrono::milliseconds(100)), - std::future_status::timeout); - ASSERT_EQ(unlink(url.fileSystemRepresentation), 0); - locked.reset(); - ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); - auto absent = waiter.result.get(); - ASSERT_TRUE(absent.ok()); - EXPECT_EQ(absent.get(), nullptr); -} - -TEST_F(CoreAIBookmarkRawTest, WaiterOpensBookmarkDirectoryAfterAcquiringLock) { - auto waiter = start_bookmark_waiter(root.url.path, key); - ASSERT_EQ( - waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); - ASSERT_EQ( - waiter.result.wait_for(std::chrono::milliseconds(100)), - std::future_status::timeout); - NSURL* bookmarks = [root.url URLByAppendingPathComponent:@"bookmarks"]; - ASSERT_EQ(rmdir(bookmarks.fileSystemRepresentation), 0); - ASSERT_EQ(mkdir(bookmarks.fileSystemRepresentation, 0700), 0); - locked.reset(); - ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); - auto acquired = waiter.result.get(); - ASSERT_TRUE(acquired.ok()); - ASSERT_NE(acquired.get(), nullptr); - NSData* bytes = [@"bookmark" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_EQ(write_bookmark(*acquired.get(), bytes), Error::Ok); - EXPECT_TRUE([[NSData dataWithContentsOfURL:url] isEqual:bytes]); -} - -TEST_F( - CoreAIBookmarkPathTest, - ProcessContentionAllowsIndependentKeyAndRecoversAfterHolderDeath) { - BookmarkChild holder; - ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); - ASSERT_TRUE(holder.receive('L')); - NSURL* file = lock_url(); - struct stat before {}; - ASSERT_EQ(stat(file.fileSystemRepresentation, &before), 0); - BookmarkChild waiter; - ASSERT_TRUE(waiter.spawn(@"--bookmark-lock", root.url.path, @"same")); - ASSERT_TRUE(waiter.receive('B')); - ASSERT_TRUE(waiter.expect_blocked()); - BookmarkChild independent; - ASSERT_TRUE(independent.spawn(@"--bookmark-lock", root.url.path, @"other")); - ASSERT_TRUE(independent.receive('L')); - ASSERT_TRUE(independent.expect_exit(0)); - ASSERT_TRUE(holder.kill_and_reap()); - ASSERT_TRUE(waiter.receive('L')); - ASSERT_TRUE(waiter.expect_exit(0)); - struct stat after {}; - ASSERT_EQ(stat(file.fileSystemRepresentation, &after), 0); - EXPECT_EQ(before.st_dev, after.st_dev); - EXPECT_EQ(before.st_ino, after.st_ino); -} - -TEST_F(CoreAIBookmarkPathTest, LockNamesUseOnlyValidatedFirstByte) { - EXPECT_EQ(bookmark_lock_name(nil).error(), Error::InvalidArgument); - for (NSString* invalid in - @[ @"", @"../escape", @"00", [fixture_identity('a') uppercaseString] ]) { - EXPECT_EQ(bookmark_lock_name(invalid).error(), Error::InvalidArgument); - } - auto first = bookmark_lock_name(fixture_identity('0')); - auto last = bookmark_lock_name(fixture_identity('f')); - ASSERT_TRUE(first.ok()); - ASSERT_TRUE(last.ok()); - EXPECT_TRUE([first.get() isEqual:@"stripe-v1-00.lock"]); - EXPECT_TRUE([last.get() isEqual:@"stripe-v1-ff.lock"]); - EXPECT_TRUE([test_bookmark_lock_url(root.url, key) - isEqual:test_bookmark_lock_url(root.url, changed_key(key, true))]); -} - -TEST_F(CoreAIBookmarkPathTest, CollidingKeysSerializeAcrossThreads) { - auto holder = lock_bookmark(root.url.path, key); - ASSERT_TRUE(holder.ok()); - NSString* collision = changed_key(key, true); - ASSERT_FALSE([collision isEqual:key]); - auto waiter = start_bookmark_waiter(root.url.path, collision); - ASSERT_EQ( - waiter.attempting.wait_for(kWaitTimeout), std::future_status::ready); - EXPECT_EQ( - waiter.result.wait_for(std::chrono::milliseconds(100)), - std::future_status::timeout); - holder.get().reset(); - ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); - auto acquired = waiter.result.get(); - ASSERT_TRUE(acquired.ok()); - EXPECT_NE(acquired.get(), nullptr); -} - -TEST_F( - CoreAIBookmarkPathTest, - NoncreatingCollisionMissDoesNotWaitOrCreateFiles) { - auto holder = lock_bookmark(root.url.path, key); - ASSERT_TRUE(holder.ok()); - auto waiter = - start_bookmark_waiter(root.url.path, changed_key(key, true), false); - EXPECT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); - holder.get().reset(); - ASSERT_EQ(waiter.result.wait_for(kWaitTimeout), std::future_status::ready); - auto absent = waiter.result.get(); - ASSERT_TRUE(absent.ok()); - EXPECT_EQ(absent.get(), nullptr); - NSArray* names = [NSFileManager.defaultManager - contentsOfDirectoryAtPath:[root.url URLByAppendingPathComponent:@"locks"] - .path - error:nil]; - ASSERT_NE(names, nil); - EXPECT_EQ(names.count, 1u); -} - -TEST_F(CoreAIBookmarkPathTest, RejectsSymlinkDirectoryAndFifoStripeFiles) { +TEST_F(CoreAIBookmarkPathTest, RejectsInvalidStripeFiles) { NSData* bytes = [@"untouched target" dataUsingEncoding:NSUTF8StringEncoding]; NSURL* target = [root.url URLByAppendingPathComponent:@"target"]; ASSERT_TRUE([bytes writeToURL:target atomically:NO]); @@ -639,96 +429,21 @@ FileDescriptor file(open( ASSERT_NE(isolated.url, nil); NSURL* locks = [isolated.url URLByAppendingPathComponent:@"locks"]; ASSERT_EQ(mkdir(locks.fileSystemRepresentation, 0700), 0); - NSURL* stripe = test_bookmark_lock_url(isolated.url, key); - if (kind == 0) + NSURL* stripe = test_cache_lock_url(isolated.url, key); + if (kind == 0) { ASSERT_EQ( symlink( target.fileSystemRepresentation, stripe.fileSystemRepresentation), 0); - else if (kind == 1) + } else if (kind == 1) { ASSERT_EQ(mkdir(stripe.fileSystemRepresentation, 0700), 0); - else + } else { ASSERT_EQ(mkfifo(stripe.fileSystemRepresentation, 0600), 0); + } EXPECT_EQ( - lock_bookmark(isolated.url.path, key).error(), Error::AccessFailed); + lock_cache_entry(isolated.url.path, key).error(), Error::AccessFailed); EXPECT_TRUE([[NSData dataWithContentsOfURL:target] isEqual:bytes]); } } -TEST_F(CoreAIBookmarkPathTest, LegacyLockFilesAreLeftUntouched) { - NSURL* locks = [root.url URLByAppendingPathComponent:@"locks"]; - ASSERT_EQ(mkdir(locks.fileSystemRepresentation, 0700), 0); - NSURL* legacy = [locks - URLByAppendingPathComponent:[key stringByAppendingString:@".lock"]]; - NSData* bytes = [@"legacy" dataUsingEncoding:NSUTF8StringEncoding]; - ASSERT_TRUE([bytes writeToURL:legacy atomically:NO]); - struct stat before {}; - ASSERT_EQ(lstat(legacy.fileSystemRepresentation, &before), 0); - auto locked = lock_bookmark(root.url.path, key); - ASSERT_TRUE(locked.ok()); - locked.get().reset(); - struct stat after {}; - ASSERT_EQ(lstat(legacy.fileSystemRepresentation, &after), 0); - EXPECT_EQ(before.st_ino, after.st_ino); - EXPECT_TRUE([[NSData dataWithContentsOfURL:legacy] isEqual:bytes]); - EXPECT_TRUE([NSFileManager.defaultManager fileExistsAtPath:lock_url().path]); -} - -TEST_F(CoreAIBookmarkPathTest, CollidingProcessWaiterRecoversAfterHolderDeath) { - BookmarkChild holder; - ASSERT_TRUE(holder.spawn(@"--bookmark-hold", root.url.path, @"same")); - ASSERT_TRUE(holder.receive('L')); - struct stat before {}; - ASSERT_EQ(lstat(lock_url().fileSystemRepresentation, &before), 0); - BookmarkChild waiter; - ASSERT_TRUE(waiter.spawn(@"--bookmark-lock", root.url.path, @"collision")); - ASSERT_TRUE(waiter.receive('B')); - ASSERT_TRUE(waiter.expect_blocked()); - ASSERT_TRUE(holder.kill_and_reap()); - ASSERT_TRUE(waiter.receive('L')); - ASSERT_TRUE(waiter.expect_exit(0)); - struct stat after {}; - ASSERT_EQ(lstat(lock_url().fileSystemRepresentation, &after), 0); - EXPECT_EQ(before.st_dev, after.st_dev); - EXPECT_EQ(before.st_ino, after.st_ino); -} - -TEST_F(CoreAIBookmarkPathTest, LockPoolIsBoundedAcrossManyKeys) { - std::array inodes{}; - NSURL* locks = [root.url URLByAppendingPathComponent:@"locks"]; - for (size_t generation = 0; generation < 4; ++generation) { - for (size_t stripe = 0; stripe < kBookmarkLockStripeCount; ++stripe) { - @autoreleasepool { - NSString* entry = - [NSString stringWithFormat:@"%02zx%062zx", stripe, generation]; - auto locked = lock_bookmark(root.url.path, entry); - ASSERT_TRUE(locked.ok()); - struct stat info {}; - NSURL* file = test_bookmark_lock_url(root.url, entry); - ASSERT_EQ(lstat(file.fileSystemRepresentation, &info), 0); - ASSERT_TRUE(S_ISREG(info.st_mode)); - EXPECT_EQ(info.st_size, 0); - if (generation == 0) - inodes[stripe] = info.st_ino; - else - EXPECT_EQ(info.st_ino, inodes[stripe]); - locked.get().reset(); - auto reused = lock_bookmark(root.url.path, entry); - ASSERT_TRUE(reused.ok()); - ASSERT_EQ(lstat(file.fileSystemRepresentation, &info), 0); - EXPECT_EQ(info.st_ino, inodes[stripe]); - } - } - NSArray* children = - [NSFileManager.defaultManager contentsOfDirectoryAtPath:locks.path - error:nil]; - ASSERT_NE(children, nil); - ASSERT_EQ(children.count, kBookmarkLockStripeCount); - for (NSString* name in children) { - EXPECT_TRUE([name hasPrefix:@"stripe-v1-"]); - EXPECT_EQ(name.length, 17u); - } - } -} - } // namespace diff --git a/backends/apple/coreai/runtime/test/coreai_cache_key_test.mm b/backends/apple/coreai/runtime/test/coreai_cache_key_test.mm new file mode 100644 index 00000000000..368e0fc6fa4 --- /dev/null +++ b/backends/apple/coreai/runtime/test/coreai_cache_key_test.mm @@ -0,0 +1,209 @@ +/* + * 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_key.h" + +#include +#include +#include "coreai_bookmark_fixture.h" +#include "coreai_manifest_fixture.h" + +using namespace executorch::runtime; +using namespace executorch::backends::coreai; +using namespace executorch::backends::coreai::testing; + +namespace { + +Result selected_bundle(NSMutableDictionary* dictionary) { + auto manifest = parse_manifest(encode(dictionary)); + if (!manifest.ok()) { + return manifest.error(); + } + return select_assets(manifest.get(), @"arch_b", @"macOS"); +} + +NSString* key_for_device(NSString* device) { + NSString* model = [@"" stringByPaddingToLength:64 + withString:@"a" + startingAtIndex:0]; + NSString* options = [@"" stringByPaddingToLength:64 + withString:@"b" + startingAtIndex:0]; + return [NSString stringWithFormat:@"%@-%@-%@", model, device, options]; +} + +TEST(CoreAICacheKeyIdentityTest, IgnoresBindingsAndDeliveryLocation) { + auto baseline = bookmark_bundle(); + ASSERT_TRUE(baseline.ok()); + auto expected = test_cache_key(baseline.get()); + ASSERT_TRUE(expected.ok()); + + auto dictionary = manifest_dict(); + dictionary[@"hash"] = @"cd"; + dictionary[@"path"] = @"cd/model.aimodel"; + dictionary[@"function"] = @"another_function"; + dictionary[@"input_names"] = @[ @"different_input" ]; + dictionary[@"output_names"] = @[ @"different_output" ]; + auto changed = selected_bundle(dictionary); + ASSERT_TRUE(changed.ok()); + auto actual = test_cache_key(changed.get()); + ASSERT_TRUE(actual.ok()); + EXPECT_TRUE([actual.get() isEqual:expected.get()]); +} + +TEST(CoreAICacheKeyIdentityTest, SeparatesExportedBundleIdentity) { + auto baseline = bookmark_bundle(); + ASSERT_TRUE(baseline.ok()); + auto expected = test_cache_key(baseline.get()); + ASSERT_TRUE(expected.ok()); + + auto digest_changed = manifest_dict(); + digest_changed[@"bundle_digests"] = + @{@"model.aimodel" : fixture_identity('2')}; + auto reweighted = selected_bundle(digest_changed); + ASSERT_TRUE(reweighted.ok()); + + auto aot = bookmark_bundle(true); + ASSERT_TRUE(aot.ok()); + for (const SelectedBundle* bundle : {&reweighted.get(), &aot.get()}) { + auto actual = test_cache_key(*bundle); + ASSERT_TRUE(actual.ok()); + EXPECT_FALSE([actual.get() isEqual:expected.get()]); + } +} + +TEST(CoreAICacheKeyIdentityTest, SeparatesBundleNameWithSameDigestAndMode) { + auto dictionary = aot_manifest_dict(); + dictionary[@"bundle_digests"] = @{ + @"model.arch_a.aimodelc" : fixture_identity('b'), + @"model.arch_b.aimodelc" : fixture_identity('b') + }; + auto manifest = parse_manifest(encode(dictionary)); + ASSERT_TRUE(manifest.ok()); + auto first = select_assets(manifest.get(), @"arch_a", @"macOS"); + auto second = select_assets(manifest.get(), @"arch_b", @"macOS"); + ASSERT_TRUE(first.ok()); + ASSERT_TRUE(second.ok()); + auto first_key = cache_model_key(first.get()); + auto second_key = cache_model_key(second.get()); + ASSERT_TRUE(first_key.ok()); + ASSERT_TRUE(second_key.ok()); + EXPECT_FALSE([first_key.get() isEqual:second_key.get()]); +} + +TEST(CoreAICacheKeyIdentityTest, SeparatesDeviceAndSpecializationIdentity) { + auto bundle = bookmark_bundle(); + ASSERT_TRUE(bundle.ok()); + auto baseline = test_cache_key(bundle.get()); + auto platform = + cache_variant_key(bundle.get(), @"iOS", @"arch_b", @"default"); + auto architecture = + cache_variant_key(bundle.get(), @"macOS", @"arch_a", @"default"); + auto specialization = cache_variant_key( + bundle.get(), + @"macOS", + @"arch_b", + @"{\"version\":1,\"compute\":[\"cpu\"]}"); + ASSERT_TRUE(baseline.ok()); + for (auto* changed : {&platform, &architecture, &specialization}) { + ASSERT_TRUE(changed->ok()); + EXPECT_FALSE([changed->get() isEqual:baseline.get()]); + EXPECT_TRUE([cache_key_model_prefix(changed->get()) + isEqual:cache_key_model_prefix(baseline.get())]); + } +} + +TEST(CoreAICacheKeyIdentityTest, RejectsIncompletePlainIdentity) { + auto bundle = bookmark_bundle(); + ASSERT_TRUE(bundle.ok()); + EXPECT_FALSE( + cache_variant_key(bundle.get(), nil, @"arch_b", @"default").ok()); + EXPECT_FALSE(cache_variant_key(bundle.get(), @"macOS", @"", @"default").ok()); + EXPECT_FALSE(cache_variant_key(bundle.get(), @"macOS", @"arch_b", nil).ok()); + EXPECT_FALSE( + cache_variant_key(bundle.get(), @"mac_OS", @"arch_b", @"default").ok()); +} + +TEST(CoreAICacheKeyFormatTest, AcceptsReadableDeviceComponents) { + for (NSString* device in + @[ @"macOS_h17p", @"iOS_arch_b", @"visionOS_h17p-high" ]) { + NSString* key = key_for_device(device); + EXPECT_TRUE(is_cache_key(key)); + EXPECT_TRUE( + [cache_key_model_prefix(key) isEqual:[key substringToIndex:64]]); + } +} + +TEST(CoreAICacheKeyFormatTest, RejectsUnsafeOrIncompleteDevices) { + for (NSString* device in @[ + @"", + @"iOS", + @"iOS_", + @"_h17p", + @"iOS_../h17p", + @"iOS_h17p\\extra", + @"iOS_h17p extra", + @"iOS_h\u00e9", + @"iOS_h17p.bookmark" + ]) { + NSString* key = key_for_device(device); + EXPECT_FALSE(is_cache_key(key)); + EXPECT_EQ(cache_key_model_prefix(key), nil); + } +} + +TEST(CoreAICacheKeyFormatTest, LeavesRoomForBookmarkExtension) { + constexpr NSUInteger max_device_length = NAME_MAX - 130 - 9; + NSString* device = [@"iOS_" stringByPaddingToLength:max_device_length + withString:@"a" + startingAtIndex:0]; + NSString* key = key_for_device(device); + ASSERT_EQ(key.length + 9, NAME_MAX); + EXPECT_TRUE(is_cache_key(key)); + EXPECT_FALSE( + is_cache_key(key_for_device([device stringByAppendingString:@"a"]))); +} + +TEST(CoreAICacheKeyFormatTest, RequiresExactHashBoundaries) { + NSString* key = key_for_device(@"iOS_h17p"); + EXPECT_FALSE(is_cache_key(nil)); + EXPECT_FALSE(is_cache_key(@"")); + EXPECT_FALSE(is_cache_key([key substringToIndex:64])); + EXPECT_FALSE(is_cache_key([key substringFromIndex:1])); + EXPECT_FALSE(is_cache_key([key stringByAppendingString:@"0"])); + EXPECT_FALSE(is_cache_key(key.uppercaseString)); + EXPECT_FALSE(is_cache_key([key stringByAppendingString:@".bookmark"])); + EXPECT_FALSE( + is_cache_key([key stringByReplacingCharactersInRange:NSMakeRange(64, 1) + withString:@"_"])); +} + +TEST(CoreAICacheKeyFormatTest, LockStripesStayWithinConfiguredPool) { + auto bundle = bookmark_bundle(); + ASSERT_TRUE(bundle.ok()); + NSMutableSet* names = [NSMutableSet set]; + for (NSUInteger index = 0; index < 4096; ++index) { + auto key = test_cache_key( + bundle.get(), [NSString stringWithFormat:@"identity-%lu", index]); + ASSERT_TRUE(key.ok()); + auto name = cache_entry_lock_name(key.get()); + ASSERT_TRUE(name.ok()); + ASSERT_TRUE([name.get() hasPrefix:@"stripe-v1-"]); + ASSERT_TRUE([name.get() hasSuffix:@".lock"]); + NSString* encoded = + [name.get() substringWithRange:NSMakeRange(10, name.get().length - 15)]; + unsigned int stripe = 0; + ASSERT_TRUE([[NSScanner scannerWithString:encoded] scanHexInt:&stripe]); + EXPECT_LT(stripe, kCacheEntryLockStripeCount); + [names addObject:name.get()]; + } + EXPECT_GT(names.count, 1u); + EXPECT_LE(names.count, kCacheEntryLockStripeCount); +} + +} // namespace