diff --git a/garnet/bin/appmgr/appmgr.cc b/garnet/bin/appmgr/appmgr.cc index 22493645e76ede67c5635f48c182dc7987a557e4..07dcaa9253e6d505620c4b31826faafdd6666450 100644 --- a/garnet/bin/appmgr/appmgr.cc +++ b/garnet/bin/appmgr/appmgr.cc @@ -33,7 +33,9 @@ Appmgr::Appmgr(async_dispatcher_t* dispatcher, AppmgrArgs args) nullptr, kRootLabel, "/data", "/data/cache", args.environment_services, args.run_virtual_console, fuchsia::sys::EnvironmentOptions{}); - root_realm_ = std::make_unique<Realm>(std::move(realm_args)); + root_realm_ = Realm::Create(std::move(realm_args)); + + FXL_CHECK(root_realm_) << "Cannot create root realm "; // 2. Publish outgoing directories. // Publish the root realm's hub directory as 'hub/' and the first nested diff --git a/garnet/bin/appmgr/environment_controller_impl.cc b/garnet/bin/appmgr/environment_controller_impl.cc index ca7b122313364b6207ad05272a88b8f9ed8f653b..50030e10f9a40b8c6957dbcc3e3bb82fce37ae45 100644 --- a/garnet/bin/appmgr/environment_controller_impl.cc +++ b/garnet/bin/appmgr/environment_controller_impl.cc @@ -4,10 +4,11 @@ #include "garnet/bin/appmgr/environment_controller_impl.h" -#include <utility> - +#include <lib/async/default.h> #include <lib/fit/function.h> +#include <utility> + #include "garnet/bin/appmgr/realm.h" namespace component { @@ -15,7 +16,9 @@ namespace component { EnvironmentControllerImpl::EnvironmentControllerImpl( fidl::InterfaceRequest<fuchsia::sys::EnvironmentController> request, std::unique_ptr<Realm> realm) - : binding_(this), realm_(std::move(realm)) { + : binding_(this), + realm_(std::move(realm)), + wait_(this, realm_->job().get(), ZX_TASK_TERMINATED) { if (request.is_valid()) { binding_.Bind(std::move(request)); binding_.set_error_handler([this](zx_status_t status) { @@ -24,14 +27,40 @@ EnvironmentControllerImpl::EnvironmentControllerImpl( // |this| at the end of the previous statement. }); } + zx_status_t status = wait_.Begin(async_get_default_dispatcher()); + FXL_DCHECK(status == ZX_OK); +} + +// Called when job terminates, regardless of if Kill() was invoked. +void EnvironmentControllerImpl::Handler(async_dispatcher_t* dispatcher, + async::WaitBase* wait, + zx_status_t status, + const zx_packet_signal* signal) { + FXL_DCHECK(status == ZX_OK); + FXL_DCHECK((signal->observed & ZX_TASK_TERMINATED) == ZX_TASK_TERMINATED) + << signal->observed; + + ExtractEnvironmentController(); + + // The destructor of the temporary returned by ExtractComponent destroys + // |this| at the end of the previous statement. +} + +std::unique_ptr<EnvironmentControllerImpl> +EnvironmentControllerImpl::ExtractEnvironmentController() { + if (realm_) { + wait_.Cancel(); + auto self = realm_->parent()->ExtractChild(realm_.get()); + realm_ = nullptr; + return self; + } + return nullptr; } EnvironmentControllerImpl::~EnvironmentControllerImpl() = default; void EnvironmentControllerImpl::Kill(KillCallback callback) { - std::unique_ptr<EnvironmentControllerImpl> self = - realm_->parent()->ExtractChild(realm_.get()); - realm_ = nullptr; + auto self = ExtractEnvironmentController(); callback(); // The |self| destructor destroys |this| when we unwind this stack frame. } diff --git a/garnet/bin/appmgr/environment_controller_impl.h b/garnet/bin/appmgr/environment_controller_impl.h index 73c05cb224ec57c90904d1785b0a0f8559212a90..cbdc98db3a7a83b667c129f2fe21e4c3ae21eab3 100644 --- a/garnet/bin/appmgr/environment_controller_impl.h +++ b/garnet/bin/appmgr/environment_controller_impl.h @@ -5,10 +5,12 @@ #ifndef GARNET_BIN_APPMGR_ENVIRONMENT_CONTROLLER_IMPL_H_ #define GARNET_BIN_APPMGR_ENVIRONMENT_CONTROLLER_IMPL_H_ +#include <fuchsia/sys/cpp/fidl.h> +#include <lib/async/cpp/wait.h> +#include <lib/fidl/cpp/binding.h> + #include <memory> -#include <fuchsia/sys/cpp/fidl.h> -#include "lib/fidl/cpp/binding.h" #include "src/lib/fxl/macros.h" namespace component { @@ -32,9 +34,20 @@ class EnvironmentControllerImpl : public fuchsia::sys::EnvironmentController { void OnCreated(); private: + // Kills realm and returns self object extracted form parent realm. + std::unique_ptr<EnvironmentControllerImpl> ExtractEnvironmentController(); + + void Handler(async_dispatcher_t* dispatcher, async::WaitBase* wait, + zx_status_t status, const zx_packet_signal* signal); + fidl::Binding<fuchsia::sys::EnvironmentController> binding_; + std::unique_ptr<Realm> realm_; + async::WaitMethod<EnvironmentControllerImpl, + &EnvironmentControllerImpl::Handler> + wait_; + FXL_DISALLOW_COPY_AND_ASSIGN(EnvironmentControllerImpl); }; diff --git a/garnet/bin/appmgr/integration_tests/realm_integration_test.cc b/garnet/bin/appmgr/integration_tests/realm_integration_test.cc index 0c1d74a0285da3d142b1230ef0b24a957938b76e..46a07117428d6d8db287b10e079ad240cca4e782 100644 --- a/garnet/bin/appmgr/integration_tests/realm_integration_test.cc +++ b/garnet/bin/appmgr/integration_tests/realm_integration_test.cc @@ -2,17 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "garnet/bin/appmgr/component_controller_impl.h" -#include "garnet/bin/appmgr/realm.h" - -#include <stdint.h> -#include <stdio.h> -#include <unistd.h> -#include <fstream> -#include <memory> -#include <string> -#include <vector> - #include <fidl/examples/echo/cpp/fidl.h> #include <fs/pseudo-dir.h> #include <fs/synchronous-vfs.h> @@ -29,12 +18,23 @@ #include <lib/sys/cpp/testing/enclosing_environment.h> #include <lib/sys/cpp/testing/test_with_environment.h> #include <src/lib/fxl/logging.h> +#include <stdint.h> +#include <stdio.h> #include <test/appmgr/integration/cpp/fidl.h> +#include <unistd.h> +#include <fstream> +#include <memory> +#include <string> +#include <vector> + +#include "garnet/bin/appmgr/component_controller_impl.h" #include "garnet/bin/appmgr/integration_tests/util/data_file_reader_writer_util.h" +#include "garnet/bin/appmgr/realm.h" #include "garnet/bin/appmgr/util.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "src/lib/files/glob.h" #include "src/lib/files/scoped_temp_dir.h" namespace component { @@ -283,6 +283,83 @@ TEST_F(RealmTest, EnvironmentLabelMustBeUnique) { RunLoopUntil([&] { return env_controller_status == ZX_ERR_BAD_STATE; })); } +TEST_F(RealmTest, RealmDiesWhenItsJobDies) { + auto env_services = CreateServices(); + + ASSERT_EQ(ZX_OK, + env_services->AddServiceWithLaunchInfo( + CreateLaunchInfo("fuchsia-pkg://fuchsia.com/" + "echo_server_cpp#meta/echo_server_cpp.cmx"), + fidl::examples::echo::Echo::Name_)); + auto enclosing_environment = + CreateNewEnclosingEnvironment(kRealm, std::move(env_services)); + ASSERT_TRUE(WaitForEnclosingEnvToStart(enclosing_environment.get())); + + // make sure echo service is running. + fidl::examples::echo::EchoPtr echo; + enclosing_environment->ConnectToService(echo.NewRequest()); + const std::string message = "some_msg"; + fidl::StringPtr ret_msg = ""; + echo->EchoString(message, + [&](::fidl::StringPtr retval) { ret_msg = retval; }); + ASSERT_TRUE(RunLoopUntil([&] { return std::string(ret_msg) == message; })); + + fuchsia::sys::JobProviderSyncPtr ptr; + files::Glob glob(std::string("/hub/r/") + kRealm + "/*/job"); + ASSERT_EQ(1u, glob.size()); + const std::string path = *glob.begin(); + fdio_service_connect(path.c_str(), ptr.NewRequest().TakeChannel().release()); + + zx::job job; + EXPECT_EQ(ZX_OK, ptr->GetJob(&job)); + ASSERT_EQ(ZX_OK, job.kill()); + + EXPECT_TRUE( + RunLoopUntil([&] { return !enclosing_environment->is_running(); })); +} + +TEST_F(RealmTest, EmptyRealmDiesWhenItsJobDies) { + auto env_services = CreateServices(); + + auto enclosing_environment = + CreateNewEnclosingEnvironment(kRealm, std::move(env_services)); + ASSERT_TRUE(WaitForEnclosingEnvToStart(enclosing_environment.get())); + + fuchsia::sys::JobProviderSyncPtr ptr; + files::Glob glob(std::string("/hub/r/") + kRealm + "/*/job"); + ASSERT_EQ(1u, glob.size()); + const std::string path = *glob.begin(); + fdio_service_connect(path.c_str(), ptr.NewRequest().TakeChannel().release()); + + zx::job job; + EXPECT_EQ(ZX_OK, ptr->GetJob(&job)); + ASSERT_EQ(ZX_OK, job.kill()); + + EXPECT_TRUE( + RunLoopUntil([&] { return !enclosing_environment->is_running(); })); +} + +TEST_F(RealmTest, KillWorks) { + auto env_services = CreateServices(); + + auto enclosing_environment = + CreateNewEnclosingEnvironment(kRealm, std::move(env_services)); + ASSERT_TRUE(WaitForEnclosingEnvToStart(enclosing_environment.get())); + + std::string hub_path = std::string("/hub/r/") + kRealm; + // make sure realm was created + files::Glob glob1(hub_path); + ASSERT_EQ(1u, glob1.size()); + + bool killed = false; + enclosing_environment->Kill([&] { killed = true; }); + ASSERT_TRUE(RunLoopUntil([&] { return killed; })); + + // make sure realm was really killed + files::Glob glob2(hub_path); + ASSERT_EQ(0u, glob2.size()); +} + class EnvironmentOptionsTest : public RealmTest, public component::testing::DataFileReaderWriterUtil {}; diff --git a/garnet/bin/appmgr/job_provider_impl.cc b/garnet/bin/appmgr/job_provider_impl.cc index 39d0087591a82989f34eefb56b7ee4032083b758..16e4fdf0b63883a3552b4210f2b93c0324da5d54 100644 --- a/garnet/bin/appmgr/job_provider_impl.cc +++ b/garnet/bin/appmgr/job_provider_impl.cc @@ -11,7 +11,7 @@ namespace component { JobProviderImpl::JobProviderImpl(Realm* realm) : realm_(realm) {} void JobProviderImpl::GetJob(GetJobCallback callback) { - callback(realm_->DuplicateJob()); + callback(realm_->DuplicateJobForHub()); } void JobProviderImpl::AddBinding( diff --git a/garnet/bin/appmgr/realm.cc b/garnet/bin/appmgr/realm.cc index ab4dcc51bed74e6c94eb6a66a4bd2df4bf9ba86a..f83cea6b78001866368306f9a10b895346aeb684 100644 --- a/garnet/bin/appmgr/realm.cc +++ b/garnet/bin/appmgr/realm.cc @@ -25,6 +25,7 @@ #include <unistd.h> #include <zircon/process.h> #include <zircon/processargs.h> +#include <zircon/rights.h> #include <zircon/status.h> #include <algorithm> @@ -42,6 +43,7 @@ #include "src/lib/files/directory.h" #include "src/lib/files/file.h" #include "src/lib/files/path.h" +#include "src/lib/fxl/logging.h" #include "src/lib/pkg_url/url_resolver.h" namespace component { @@ -233,35 +235,49 @@ RealmArgs RealmArgs::MakeWithAdditionalServices( .options = std::move(options)}; } -Realm::Realm(RealmArgs args) +std::unique_ptr<Realm> Realm::Create(RealmArgs args) { + if (args.label.empty()) { + FXL_LOG(ERROR) << "Cannot create realm with empty label"; + return nullptr; + } + + // parent_ is null if this is the root application environment. if so, we + // derive from the application manager's job. + zx::unowned<zx::job> parent_job; + if (args.parent) { + parent_job = zx::unowned<zx::job>(args.parent->job_); + } else { + parent_job = zx::unowned<zx::job>(zx::job::default_job()); + } + + zx::job job; + if (zx::job::create(*parent_job, 0u, &job) != ZX_OK) { + FXL_LOG(ERROR) << "Job creation failed. Cannot create realm '" + << args.label; + return nullptr; + } + + return std::make_unique<Realm>(std::move(args), std::move(job)); +} + +Realm::Realm(RealmArgs args, zx::job job) : parent_(args.parent), data_path_(args.data_path), cache_path_(args.cache_path), run_virtual_console_(args.run_virtual_console), + job_(std::move(job)), hub_(fbl::AdoptRef(new fs::PseudoDir())), info_vfs_(async_get_default_dispatcher()), environment_services_(args.environment_services), allow_parent_runners_(args.options.allow_parent_runners), delete_storage_on_death_(args.options.delete_storage_on_death) { - // parent_ is null if this is the root application environment. if so, we - // derive from the application manager's job. - zx::unowned<zx::job> parent_job; - if (parent_) { - parent_job = zx::unowned<zx::job>(parent_->job_); - } else { - parent_job = zx::unowned<zx::job>(zx::job::default_job()); - } - // Only need to create this channel for the root realm. if (parent_ == nullptr) { FXL_CHECK(zx::channel::create(0, &first_nested_realm_svc_server_, &first_nested_realm_svc_client_) == ZX_OK); } - FXL_CHECK(zx::job::create(*parent_job, 0u, &job_) == ZX_OK); - koid_ = std::to_string(fsl::GetKoid(job_.get())); - FXL_CHECK(!args.label.empty()); label_ = args.label.substr(0, fuchsia::sys::kLabelMaxLength); if (args.options.kill_on_oom) { @@ -341,16 +357,16 @@ HubInfo Realm::HubInfo() { return component::HubInfo(label_, koid_, hub_.dir()); } -zx::job Realm::DuplicateJob() const { +zx::job Realm::DuplicateJobForHub() const { zx::job duplicate_job; - zx_status_t status = - job_.duplicate(ZX_RIGHTS_BASIC | ZX_RIGHT_WRITE, &duplicate_job); + // As htis only goes inside /hub, it is fine to give destoy rights + auto flags = ZX_RIGHTS_BASIC | ZX_RIGHT_DESTROY; + zx_status_t status = job_.duplicate(flags | ZX_RIGHT_WRITE, &duplicate_job); if (status == ZX_ERR_INVALID_ARGS) { // In the process of removing WRITE for processes; if duplicate with WRITE // failed, try the new rights. TODO(ZX-2967): Once the transition is // complete, only duplicate with MANAGE_PROCESS. - status = job_.duplicate(ZX_RIGHTS_BASIC | ZX_RIGHT_MANAGE_PROCESS, - &duplicate_job); + status = job_.duplicate(flags | ZX_RIGHT_MANAGE_PROCESS, &duplicate_job); } if (status != ZX_OK) { return zx::job(); @@ -406,8 +422,14 @@ void Realm::CreateNestedEnvironment( environment_services_, /*run_virtual_console=*/false, std::move(options)); } + + auto realm = Realm::Create(std::move(args)); + if (!realm) { + return; + } + auto controller = std::make_unique<EnvironmentControllerImpl>( - std::move(controller_request), std::make_unique<Realm>(std::move(args))); + std::move(controller_request), std::move(realm)); Realm* child = controller->realm(); child->AddBinding(std::move(environment)); diff --git a/garnet/bin/appmgr/realm.h b/garnet/bin/appmgr/realm.h index e39b4efb6ca6d18bdb816f0fb7ddc2597ee8d6db..5e24debe164c0b9929b101287751c3fb217a220c 100644 --- a/garnet/bin/appmgr/realm.h +++ b/garnet/bin/appmgr/realm.h @@ -62,7 +62,11 @@ struct RealmArgs { class Realm : public ComponentContainer<ComponentControllerImpl> { public: - Realm(RealmArgs args); + static std::unique_ptr<Realm> Create(RealmArgs args); + + // Constructor to create a Realm object. Clients should call |Create|. + Realm(RealmArgs args, zx::job job); + ~Realm(); Realm* parent() const { return parent_; } @@ -79,7 +83,9 @@ class Realm : public ComponentContainer<ComponentControllerImpl> { HubInfo HubInfo(); - zx::job DuplicateJob() const; + zx::job DuplicateJobForHub() const; + + const zx::job& job() const { return job_; } void CreateNestedEnvironment( fidl::InterfaceRequest<fuchsia::sys::Environment> environment, @@ -97,9 +103,9 @@ class Realm : public ComponentContainer<ComponentControllerImpl> { ComponentObjectCreatedCallback callback = nullptr); // Removes the child realm from this realm and returns the owning - // reference to the child's controller. The caller of this function typically - // destroys the controller (and hence the environment) shortly after calling - // this function. + // reference to the child's controller. The caller of this function + // typically destroys the controller (and hence the environment) shortly + // after calling this function. std::unique_ptr<EnvironmentControllerImpl> ExtractChild(Realm* child); // Removes the application from this environment and returns the owning