[netemul] Handling environment names
- Fix issues where sandboxes could hang if set up with duplicate
environment names
- Sandbox service creates a shell environment around each sandbox to
loosen naming restrictions.
Change-Id: I4f5b551bf636eabf2a26f8540fafec3d9a79899e
diff --git a/sdk/lib/sys/cpp/testing/enclosing_environment.cc b/sdk/lib/sys/cpp/testing/enclosing_environment.cc
index 95dfe0e..68609f1 100644
--- a/sdk/lib/sys/cpp/testing/enclosing_environment.cc
+++ b/sdk/lib/sys/cpp/testing/enclosing_environment.cc
@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include <lib/sys/cpp/testing/enclosing_environment.h>
-
#include <fuchsia/debugdata/cpp/fidl.h>
#include <fuchsia/io/cpp/fidl.h>
#include <fuchsia/sys/cpp/fidl.h>
@@ -12,8 +10,10 @@
#include <lib/fidl/cpp/interface_request.h>
#include <lib/fit/function.h>
#include <lib/sys/cpp/service_directory.h>
+#include <lib/sys/cpp/testing/enclosing_environment.h>
#include <lib/vfs/cpp/pseudo_dir.h>
#include <zircon/assert.h>
+
#include <memory>
namespace sys {
@@ -238,11 +238,9 @@
}
void EnclosingEnvironment::SetRunning(bool running) {
- if (running_ != running) {
- running_ = running;
- if (running_changed_callback_) {
- running_changed_callback_(running_);
- }
+ running_ = running;
+ if (running_changed_callback_) {
+ running_changed_callback_(running_);
}
}
diff --git a/src/connectivity/network/testing/netemul/runner/managed_environment.cc b/src/connectivity/network/testing/netemul/runner/managed_environment.cc
index 088fe7e..e9e4e75 100644
--- a/src/connectivity/network/testing/netemul/runner/managed_environment.cc
+++ b/src/connectivity/network/testing/netemul/runner/managed_environment.cc
@@ -30,7 +30,8 @@
}
ManagedEnvironment::ManagedEnvironment(const SandboxEnv::Ptr& sandbox_env)
- : sandbox_env_(sandbox_env) {}
+ : sandbox_env_(sandbox_env), ready_(false) {}
+
sys::testing::EnclosingEnvironment& ManagedEnvironment::environment() {
return *env_;
}
@@ -154,10 +155,23 @@
std::move(services), sub_options);
env_->SetRunningChangedCallback([this](bool running) {
- if (running && running_callback_) {
- running_callback_();
- } else if (!running) {
+ ready_ = true;
+ if (running) {
+ for (auto& r : pending_requests_) {
+ Bind(std::move(r));
+ }
+ pending_requests_.clear();
+ if (running_callback_) {
+ running_callback_();
+ }
+ } else {
FXL_LOG(ERROR) << "Underlying enclosed Environment stopped running";
+ running_callback_ = nullptr;
+ children_.clear();
+ pending_requests_.clear();
+ env_ = nullptr;
+ launcher_ = nullptr;
+ bindings_.CloseAll();
}
});
@@ -181,7 +195,13 @@
void ManagedEnvironment::Bind(
fidl::InterfaceRequest<ManagedEnvironment::FManagedEnvironment> req) {
- bindings_.AddBinding(this, std::move(req));
+ if (ready_) {
+ bindings_.AddBinding(this, std::move(req));
+ } else if (env_) {
+ pending_requests_.push_back(std::move(req));
+ } else {
+ req.Close(ZX_ERR_INTERNAL);
+ }
}
ManagedLoggerCollection& ManagedEnvironment::loggers() {
diff --git a/src/connectivity/network/testing/netemul/runner/managed_environment.h b/src/connectivity/network/testing/netemul/runner/managed_environment.h
index 0ad926a..7afe438 100644
--- a/src/connectivity/network/testing/netemul/runner/managed_environment.h
+++ b/src/connectivity/network/testing/netemul/runner/managed_environment.h
@@ -76,6 +76,8 @@
std::vector<LaunchService> service_config_;
VirtualDevices virtual_devices_;
std::unique_ptr<VirtualData> virtual_data_;
+ std::vector<fidl::InterfaceRequest<FManagedEnvironment>> pending_requests_;
+ bool ready_;
FXL_DISALLOW_COPY_AND_ASSIGN(ManagedEnvironment);
};
diff --git a/src/connectivity/network/testing/netemul/runner/sandbox.cc b/src/connectivity/network/testing/netemul/runner/sandbox.cc
index 5fd998f..5bdbcfa0 100644
--- a/src/connectivity/network/testing/netemul/runner/sandbox.cc
+++ b/src/connectivity/network/testing/netemul/runner/sandbox.cc
@@ -474,19 +474,25 @@
linfo.arguments->insert(linfo.arguments->end(), arguments.begin(),
arguments.end());
- auto& proc = procs_.emplace_back();
auto ticket = procs_.size();
+ auto& proc = procs_.emplace_back();
if (is_test) {
RegisterTest(ticket);
}
+ proc.set_error_handler([this](zx_status_t status) {
+ PostTerminate(TerminationReason::INTERNAL_ERROR);
+ });
+
// we observe test processes return code
proc.events().OnTerminated = [url, this, is_test, ticket](
int64_t code, TerminationReason reason) {
FXL_LOG(INFO) << T::msg << " " << url << " terminated with (" << code
<< ") reason: "
<< sys::HumanReadableTerminationReason(reason);
+ // remove the error handler:
+ procs_[ticket].set_error_handler(nullptr);
if (is_test) {
if (code != 0 || reason != TerminationReason::EXITED) {
// test failed, early bail
@@ -519,6 +525,7 @@
linfo.arguments->insert(linfo.arguments->end(), arguments.begin(),
arguments.end());
+ auto ticket = procs_.size();
auto& proc = procs_.emplace_back();
if ((*launcher)->CreateComponent(std::move(linfo), proc.NewRequest()) !=
@@ -526,13 +533,19 @@
FXL_LOG(ERROR) << "couldn't launch setup: " << url;
bridge.completer.complete_error();
} else {
+ proc.set_error_handler([this](zx_status_t status) {
+ PostTerminate(TerminationReason::INTERNAL_ERROR);
+ });
+
// we observe test processes return code
proc.events().OnTerminated =
- [url, this, completer = std::move(bridge.completer)](
+ [url, this, ticket, completer = std::move(bridge.completer)](
int64_t code, TerminationReason reason) mutable {
FXL_LOG(INFO) << "Setup " << url << " terminated with (" << code
<< ") reason: "
<< sys::HumanReadableTerminationReason(reason);
+ // remove the error handler:
+ procs_[ticket].set_error_handler(nullptr);
if (code == 0 && reason == TerminationReason::EXITED) {
completer.complete_ok();
} else {
diff --git a/src/connectivity/network/testing/netemul/runner/sandbox_service.cc b/src/connectivity/network/testing/netemul/runner/sandbox_service.cc
index 1a52780..38b6442 100644
--- a/src/connectivity/network/testing/netemul/runner/sandbox_service.cc
+++ b/src/connectivity/network/testing/netemul/runner/sandbox_service.cc
@@ -6,6 +6,9 @@
#include <lib/async/task.h>
#include <lib/sys/cpp/service_directory.h>
+#include <src/lib/fxl/strings/string_printf.h>
+
+#include <random>
namespace netemul {
@@ -15,21 +18,36 @@
using OnDestroyedCallback = fit::function<void(const SandboxBinding*)>;
SandboxBinding(fidl::InterfaceRequest<FSandbox> req,
+ fuchsia::sys::EnvironmentControllerPtr env_controller,
+ fuchsia::sys::EnvironmentPtr environment,
std::unique_ptr<async::Loop> loop, SandboxService* parent)
: loop_(std::move(loop)),
- binding_(this, std::move(req), loop_->dispatcher()),
+ binding_(this),
+ parent_env_(std::move(environment)),
+ parent_env_ctlr_(std::move(env_controller)),
parent_(parent) {
binding_.set_error_handler([this](zx_status_t err) {
environments_.clear();
parent_->BindingClosed(this);
});
- auto services = sys::ServiceDirectory::CreateFromNamespace();
- services->Connect(parent_env_.NewRequest(loop_->dispatcher()));
+ parent_env_ctlr_.set_error_handler([this](zx_status_t err) {
+ FXL_LOG(ERROR) << "Lost connection to parent environment";
+ environments_.clear();
+ parent_->BindingClosed(this);
+ });
+
parent_env_.set_error_handler([this](zx_status_t err) {
FXL_LOG(ERROR) << "Lost connection to parent environment";
+ environments_.clear();
+ parent_->BindingClosed(this);
});
+ parent_env_ctlr_.events().OnCreated = [this,
+ req = std::move(req)]() mutable {
+ binding_.Bind(std::move(req), loop_->dispatcher());
+ };
+
if (loop_->StartThread("sandbox-thread") != ZX_OK) {
FXL_LOG(ERROR) << "Failed to start thread for sandbox";
parent_->BindingClosed(this);
@@ -84,6 +102,7 @@
fidl::Binding<FSandbox> binding_;
std::vector<std::unique_ptr<ManagedEnvironment>> environments_;
fuchsia::sys::EnvironmentPtr parent_env_;
+ fuchsia::sys::EnvironmentControllerPtr parent_env_ctlr_;
// Pointer to parent SandboxService. Not owned.
SandboxService* parent_;
@@ -103,23 +122,44 @@
fidl::InterfaceRequestHandler<fuchsia::netemul::sandbox::Sandbox>
SandboxService::GetHandler() {
- return
- [this](fidl::InterfaceRequest<fuchsia::netemul::sandbox::Sandbox> req) {
- // Create each SandboxBinding in its own thread.
- // A common usage pattern for SandboxService is to connect to the
- // service in each test in a rust create test suite. Rust crate tests
- // run in parallel, so enclosing each binding in its own thread will
- // makes a bit more sense to service everything independently.
- auto loop =
- std::make_unique<async::Loop>(&kAsyncLoopConfigNoAttachToThread);
+ return [this](
+ fidl::InterfaceRequest<fuchsia::netemul::sandbox::Sandbox> req) {
+ // Create each SandboxBinding in its own thread.
+ // A common usage pattern for SandboxService is to connect to the
+ // service in each test in a rust create test suite. Rust crate tests
+ // run in parallel, so enclosing each binding in its own thread will
+ // makes a bit more sense to service everything independently.
+ auto loop =
+ std::make_unique<async::Loop>(&kAsyncLoopConfigNoAttachToThread);
- bindings_.push_back(std::make_unique<SandboxBinding>(
- std::move(req), std::move(loop), this));
- };
+ fuchsia::sys::EnvironmentPtr env;
+ fuchsia::sys::EnvironmentControllerPtr env_ctlr;
+ parent_env_->CreateNestedEnvironment(
+ env.NewRequest(loop->dispatcher()),
+ env_ctlr.NewRequest(loop->dispatcher()),
+ fxl::StringPrintf("netemul-%08X-%08X", random_, counter_++), nullptr,
+ fuchsia::sys::EnvironmentOptions{
+ .delete_storage_on_death = false,
+ .kill_on_oom = true,
+ .inherit_parent_services = true,
+ .allow_parent_runners = true,
+ });
+
+ bindings_.push_back(std::make_unique<SandboxBinding>(
+ std::move(req), std::move(env_ctlr), std::move(env), std::move(loop),
+ this));
+ };
}
SandboxService::SandboxService(async_dispatcher_t* dispatcher)
- : dispatcher_(dispatcher) {}
+ : dispatcher_(dispatcher), counter_(0) {
+ std::random_device dev;
+ std::uniform_int_distribution<uint32_t> rd(0, 0xFFFFFFFF);
+ random_ = rd(dev);
+
+ auto services = sys::ServiceDirectory::CreateFromNamespace();
+ services->Connect(parent_env_.NewRequest(dispatcher));
+}
SandboxService::~SandboxService() = default;
diff --git a/src/connectivity/network/testing/netemul/runner/sandbox_service.h b/src/connectivity/network/testing/netemul/runner/sandbox_service.h
index 7cf9a1b..23f06e3 100644
--- a/src/connectivity/network/testing/netemul/runner/sandbox_service.h
+++ b/src/connectivity/network/testing/netemul/runner/sandbox_service.h
@@ -6,6 +6,7 @@
#define SRC_CONNECTIVITY_NETWORK_TESTING_NETEMUL_RUNNER_SANDBOX_SERVICE_H_
#include <fuchsia/netemul/sandbox/cpp/fidl.h>
+#include <fuchsia/sys/cpp/fidl.h>
#include "sandbox.h"
@@ -26,7 +27,10 @@
private:
async_dispatcher_t* dispatcher_;
+ fuchsia::sys::EnvironmentPtr parent_env_;
std::vector<std::unique_ptr<SandboxBinding>> bindings_;
+ uint32_t counter_;
+ uint32_t random_;
FXL_DISALLOW_COPY_AND_ASSIGN(SandboxService);
};
diff --git a/src/connectivity/network/testing/netemul/runner/sandbox_unittest.cc b/src/connectivity/network/testing/netemul/runner/sandbox_unittest.cc
index 1a19d67..f584786 100644
--- a/src/connectivity/network/testing/netemul/runner/sandbox_unittest.cc
+++ b/src/connectivity/network/testing/netemul/runner/sandbox_unittest.cc
@@ -753,5 +753,24 @@
sandbox = nullptr;
}
+TEST_F(SandboxTest, EnvironmentsWithSameNameFail) {
+ SetCmx(R"(
+{
+ "environment" : {
+ "children" : [
+ {
+ "name" : "my-env",
+ "test" : [ "fuchsia-pkg://fuchsia.com/netemul_sandbox_test#meta/dummy_proc.cmx" ]
+ },
+ {
+ "name" : "my-env",
+ "test" : [ "fuchsia-pkg://fuchsia.com/netemul_sandbox_test#meta/dummy_proc.cmx" ]
+ }
+ ]
+ }
+})");
+ RunSandboxInternalError();
+}
+
} // namespace testing
} // namespace netemul
diff --git a/src/connectivity/network/testing/netemul/runner/test/sandbox_service/src/main.rs b/src/connectivity/network/testing/netemul/runner/test/sandbox_service/src/main.rs
index 7f9759d..e8460a5 100644
--- a/src/connectivity/network/testing/netemul/runner/test/sandbox_service/src/main.rs
+++ b/src/connectivity/network/testing/netemul/runner/test/sandbox_service/src/main.rs
@@ -30,7 +30,10 @@
fuchsia_zircon as zx,
};
- fn create_env_with_netstack(sandbox: &SandboxProxy) -> Result<ManagedEnvironmentProxy, Error> {
+ fn create_named_env_with_netstack(
+ sandbox: &SandboxProxy,
+ name: Option<String>,
+ ) -> Result<ManagedEnvironmentProxy, Error> {
let (env, env_server_end) = fidl::endpoints::create_proxy::<ManagedEnvironmentMarker>()?;
let services = vec![LaunchService {
name: String::from("fuchsia.netstack.Netstack"),
@@ -40,7 +43,7 @@
sandbox.create_environment(
env_server_end,
EnvironmentOptions {
- name: None, // don't care about the name, let it be created by itself
+ name: name, // don't care about the name, let it be created by itself
services: Some(services),
devices: None,
inherit_parent_launch_services: Some(false),
@@ -56,6 +59,10 @@
Ok(env)
}
+ fn create_env_with_netstack(sandbox: &SandboxProxy) -> Result<ManagedEnvironmentProxy, Error> {
+ create_named_env_with_netstack(sandbox, None)
+ }
+
async fn create_network<'a>(
env: &'a ManagedEnvironmentProxy,
name: &'a str,
@@ -212,4 +219,42 @@
assert!(clients_1.iter().any(|c| c == "e2"));
assert!(clients_2.iter().any(|c| c == "e3"));
}
+
+ #[fasync::run_singlethreaded]
+ #[test]
+ async fn same_environment_name_fails() {
+ let sandbox =
+ client::connect_to_service::<SandboxMarker>().expect("Can't connect to sandbox");
+ let env_name = Some("env_a".to_string());
+
+ // environment creation should work for both, but doing anything on
+ // env2 should fail:
+ let env1 =
+ create_named_env_with_netstack(&sandbox, env_name.clone()).expect("can't create env 1");
+ let env2 = create_named_env_with_netstack(&sandbox, env_name).expect("can't create env 2");
+
+ let _net1 =
+ await!(create_network(&env1, "network")).expect("failed to create network on env 1");
+ let _net2 = await!(create_network(&env2, "network2"))
+ .expect_err("should've failed to create network on env 2");
+ }
+
+ #[fasync::run_singlethreaded]
+ #[test]
+ async fn same_environment_name_succeeds_in_different_sandboxes() {
+ let sandbox =
+ client::connect_to_service::<SandboxMarker>().expect("Can't connect to sandbox");
+ let sandbox2 =
+ client::connect_to_service::<SandboxMarker>().expect("Can't connect to sandbox 2");
+ let env_name = Some("env_a".to_string());
+
+ let env1 =
+ create_named_env_with_netstack(&sandbox, env_name.clone()).expect("can't create env 1");
+ let env2 = create_named_env_with_netstack(&sandbox2, env_name).expect("can't create env 2");
+
+ let _net1 =
+ await!(create_network(&env1, "network")).expect("failed to create network on env 1");
+ let _net2 =
+ await!(create_network(&env2, "network2")).expect("failed to create network on env 2");
+ }
}
