From 6668f7cbd7c0873b89394175e381294747300709 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 21 Jun 2023 19:08:27 +0000 Subject: [PATCH] Remove process-local tempfs This was incorrectly made read-only in 77d534c93fd9ab9de239c63f1db2607bef9b81b6; instead of implementing it locally, ask the component manager for the directory. --- .../dart_runner/dart_component_controller.cc | 2 +- .../dart_test_component_controller.cc | 2 +- shell/platform/fuchsia/dart_runner/main.cc | 1 - .../fuchsia/dart_runner/meta/common.shard.cml | 5 ++ .../platform/fuchsia/flutter/component_v2.cc | 3 +- shell/platform/fuchsia/flutter/main.cc | 3 - .../fuchsia/runtime/dart/utils/tempfs.cc | 74 ++----------------- .../fuchsia/runtime/dart/utils/tempfs.h | 27 +------ 8 files changed, 19 insertions(+), 98 deletions(-) diff --git a/shell/platform/fuchsia/dart_runner/dart_component_controller.cc b/shell/platform/fuchsia/dart_runner/dart_component_controller.cc index 52c0fd7227d50..6e01006ed725f 100644 --- a/shell/platform/fuchsia/dart_runner/dart_component_controller.cc +++ b/shell/platform/fuchsia/dart_runner/dart_component_controller.cc @@ -182,7 +182,7 @@ bool DartComponentController::CreateAndBindNamespace() { zx_status_get_string(ns_create_status)); } - dart_utils::RunnerTemp::SetupComponent(namespace_); + dart_utils::BindTemp(namespace_); // Bind each directory in start_info's namespace to the controller's namespace // instance. diff --git a/shell/platform/fuchsia/dart_runner/dart_test_component_controller.cc b/shell/platform/fuchsia/dart_runner/dart_test_component_controller.cc index ca69e2c07924d..848a3f48ab255 100644 --- a/shell/platform/fuchsia/dart_runner/dart_test_component_controller.cc +++ b/shell/platform/fuchsia/dart_runner/dart_test_component_controller.cc @@ -195,7 +195,7 @@ bool DartTestComponentController::CreateAndBindNamespace() { zx_status_get_string(ns_create_status)); } - dart_utils::RunnerTemp::SetupComponent(namespace_); + dart_utils::BindTemp(namespace_); // Bind each directory in start_info's namespace to the controller's namespace // instance. diff --git a/shell/platform/fuchsia/dart_runner/main.cc b/shell/platform/fuchsia/dart_runner/main.cc index 329834db90406..33270bc056e6e 100644 --- a/shell/platform/fuchsia/dart_runner/main.cc +++ b/shell/platform/fuchsia/dart_runner/main.cc @@ -62,7 +62,6 @@ int main(int argc, const char** argv) { #endif // defined(AOT_RUNTIME) #endif // !defined(DART_PRODUCT) - dart_utils::RunnerTemp runner_temp; dart_runner::DartRunner runner(context.get()); // Wait to serve until we have finished all of our setup. diff --git a/shell/platform/fuchsia/dart_runner/meta/common.shard.cml b/shell/platform/fuchsia/dart_runner/meta/common.shard.cml index 6cd5a6c31e5e8..6d092b70325c0 100644 --- a/shell/platform/fuchsia/dart_runner/meta/common.shard.cml +++ b/shell/platform/fuchsia/dart_runner/meta/common.shard.cml @@ -13,6 +13,11 @@ }, ], use: [ + // This is used by the Dart VM to communicate from Dart code to C++ code. + { + storage: "tmp", + path: "/tmp", + }, // This is used by the Dart VM to load i18n data. { directory: "config-data", diff --git a/shell/platform/fuchsia/flutter/component_v2.cc b/shell/platform/fuchsia/flutter/component_v2.cc index 7139e8971b308..9a5a882efea73 100644 --- a/shell/platform/fuchsia/flutter/component_v2.cc +++ b/shell/platform/fuchsia/flutter/component_v2.cc @@ -182,8 +182,7 @@ ComponentV2::ComponentV2( return; } - // Setup /tmp to be mapped to a process-local virtual filesystem. - dart_utils::RunnerTemp::SetupComponent(fdio_ns_.get()); + dart_utils::BindTemp(fdio_ns_.get()); // ComponentStartInfo::ns (optional) if (start_info.has_ns()) { diff --git a/shell/platform/fuchsia/flutter/main.cc b/shell/platform/fuchsia/flutter/main.cc index 05f09a8305471..a8830504d64b6 100644 --- a/shell/platform/fuchsia/flutter/main.cc +++ b/shell/platform/fuchsia/flutter/main.cc @@ -40,9 +40,6 @@ int main(int argc, char const* argv[]) { &already_started); } - // Set up the process-wide /tmp virtual filesystem. - dart_utils::RunnerTemp runner_temp; - fml::MessageLoop& loop = fml::MessageLoop::GetCurrent(); flutter_runner::Runner runner(loop.GetTaskRunner(), context.get()); diff --git a/shell/platform/fuchsia/runtime/dart/utils/tempfs.cc b/shell/platform/fuchsia/runtime/dart/utils/tempfs.cc index 151183c348085..18001afec680b 100644 --- a/shell/platform/fuchsia/runtime/dart/utils/tempfs.cc +++ b/shell/platform/fuchsia/runtime/dart/utils/tempfs.cc @@ -4,16 +4,9 @@ #include "tempfs.h" -#include -#include -#include +#include -#include -#include -#include -#include #include -#include #include #include #include @@ -28,60 +21,13 @@ constexpr char kTmpPath[] = "/tmp"; namespace dart_utils { -RunnerTemp::RunnerTemp() - : loop_(std::make_unique( - &kAsyncLoopConfigNoAttachToCurrentThread)) { - loop_->StartThread("RunnerTemp"); - Start(); -} - -RunnerTemp::~RunnerTemp() = default; - -static vfs::PseudoDir tmp_dir; - -void RunnerTemp::Start() { - std::promise finished; - async::PostTask(loop_->dispatcher(), [this, &finished]() { - finished.set_value([this]() { - zx::channel client, server; - if (zx_status_t status = zx::channel::create(0, &client, &server); - status != ZX_OK) { - return status; - } - if (zx_status_t status = - tmp_dir.Serve(fuchsia::io::OpenFlags::RIGHT_READABLE | - fuchsia::io::OpenFlags::RIGHT_WRITABLE | - fuchsia::io::OpenFlags::DIRECTORY, - std::move(server), loop_->dispatcher()); - status != ZX_OK) { - return status; - } - fdio_ns_t* ns; - if (zx_status_t status = fdio_ns_get_installed(&ns); status != ZX_OK) { - return status; - } - if (zx_status_t status = fdio_ns_bind(ns, kTmpPath, client.release()); - status != ZX_OK) { - return status; - } - return ZX_OK; - }()); - }); - if (zx_status_t status = finished.get_future().get(); status != ZX_OK) { - FX_LOGF(ERROR, LOG_TAG, "Failed to install a /tmp virtual filesystem: %s", - zx_status_get_string(status)); - } -} - -void RunnerTemp::SetupComponent(fdio_ns_t* ns) { +void BindTemp(fdio_ns_t* ns) { // TODO(zra): Should isolates share a /tmp file system within a process, or // should isolates each get their own private file system for /tmp? For now, // sharing the process-wide /tmp simplifies hot reload since the hot reload // devfs requires sharing between the service isolate and the app isolates. - zx_status_t status; fdio_flat_namespace_t* rootns; - status = fdio_ns_export_root(&rootns); - if (status != ZX_OK) { + if (zx_status_t status = fdio_ns_export_root(&rootns); status != ZX_OK) { FX_LOGF(ERROR, LOG_TAG, "Failed to export root ns: %s", zx_status_get_string(status)); return; @@ -89,18 +35,14 @@ void RunnerTemp::SetupComponent(fdio_ns_t* ns) { zx_handle_t tmp_dir_handle; for (size_t i = 0; i < rootns->count; i++) { - if (strcmp(rootns->path[i], kTmpPath) == 0) { - tmp_dir_handle = rootns->handle[i]; - } else { - zx_handle_close(rootns->handle[i]); - rootns->handle[i] = ZX_HANDLE_INVALID; + if (std::string_view{rootns->path[i]} == kTmpPath) { + tmp_dir_handle = std::exchange(rootns->handle[i], ZX_HANDLE_INVALID); } } - free(rootns); - rootns = nullptr; + fdio_ns_free_flat_ns(rootns); - status = fdio_ns_bind(ns, kTmpPath, tmp_dir_handle); - if (status != ZX_OK) { + if (zx_status_t status = fdio_ns_bind(ns, kTmpPath, tmp_dir_handle); + status != ZX_OK) { zx_handle_close(tmp_dir_handle); FX_LOGF(ERROR, LOG_TAG, "Failed to bind /tmp directory into isolate namespace: %s", diff --git a/shell/platform/fuchsia/runtime/dart/utils/tempfs.h b/shell/platform/fuchsia/runtime/dart/utils/tempfs.h index bce1ef71b8597..1c89ab42f24f9 100644 --- a/shell/platform/fuchsia/runtime/dart/utils/tempfs.h +++ b/shell/platform/fuchsia/runtime/dart/utils/tempfs.h @@ -5,34 +5,13 @@ #ifndef FLUTTER_SHELL_PLATFORM_FUCHSIA_RUNTIME_DART_UTILS_TEMPFS_H_ #define FLUTTER_SHELL_PLATFORM_FUCHSIA_RUNTIME_DART_UTILS_TEMPFS_H_ -#include - -#include #include namespace dart_utils { -// Sets up /tmp for the dart_runner and flutter_runner. -class RunnerTemp { - public: - // Sets up a virtual filesystem bound to /tmp in the process-wide namespace - // that has the lifetime of this instance. - RunnerTemp(); - ~RunnerTemp(); - - // Take the virtual filesystem mapped into the process-wide namespace for - // /tmp, and map it to /tmp in the given namespace. - static void SetupComponent(fdio_ns_t* ns); - - private: - void Start(); - - std::unique_ptr loop_; - - // Disallow copy and assignment. - RunnerTemp(const RunnerTemp&) = delete; - RunnerTemp& operator=(const RunnerTemp&) = delete; -}; +// Take the virtual filesystem mapped into the process-wide namespace for /tmp, +// and map it to /tmp in the given namespace. +void BindTemp(fdio_ns_t* ns); } // namespace dart_utils