From 7b6b3a45354a5898ef7c397e16aad7750b609e5b Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Tue, 22 May 2018 11:32:36 +0200 Subject: [PATCH] base: fix destruction of async env sessions When an environment session is provided by a async service such as a sibling component, the session metadata must be preserved until end of the lifetime of the session at the server has been acknowledged by the server. Since the session meta data of env sessions are always part of the 'Child' object, the destruction of this object must be deferred until this point. --- repos/base/include/base/child.h | 28 ++++++++++ repos/base/include/base/local_connection.h | 8 ++- repos/base/src/lib/base/child.cc | 13 +++++ repos/os/run/init.run | 62 +++++++++++++++++++++- repos/os/src/init/child.cc | 24 ++++++--- repos/os/src/init/child.h | 10 +++- repos/os/src/init/main.cc | 22 ++++++-- 7 files changed, 150 insertions(+), 17 deletions(-) diff --git a/repos/base/include/base/child.h b/repos/base/include/base/child.h index 385f1aba1a..3f680c86e3 100644 --- a/repos/base/include/base/child.h +++ b/repos/base/include/base/child.h @@ -451,12 +451,21 @@ class Genode::Child : protected Rpc_object, { session.ready_callback = this; session.async_client_notify = true; + _service.initiate_request(session); if (session.phase == Session_state::SERVICE_DENIED) error(_child._policy.name(), ": environment ", CONNECTION::service_name(), " session denied " "(", session.args(), ")"); + + /* + * If the env session is provided by an async service, + * we have to wake up the server when closing the env + * session. + */ + if (session.phase == Session_state::CLOSE_REQUESTED) + _service.wakeup(); } /** @@ -575,6 +584,10 @@ class Genode::Child : protected Rpc_object, Capability cap() const { return _connection.constructed() ? _connection->cap() : Capability(); } + + bool alive() const { return _connection.constructed() && _connection->alive(); } + + void close() { if (_connection.constructed()) _connection->close(); } }; Env_connection _pd { *this, Env::pd(), _policy.name() }; @@ -666,6 +679,21 @@ class Genode::Child : protected Rpc_object, */ void initiate_env_sessions(); + /** + * Return true if the child is safe to be destroyed + * + * The child must not be destroyed until all environment sessions + * are closed at the respective servers. Otherwise, the session state, + * which is kept as part of the child object may be gone before + * the close request reaches the server. + */ + bool env_sessions_closed() const + { + if (_linker.constructed() && _linker->alive()) return false; + + return !_cpu.alive() && !_log.alive() && !_binary.alive(); + } + /** * Quota unconditionally consumed by the child's environment */ diff --git a/repos/base/include/base/local_connection.h b/repos/base/include/base/local_connection.h index 1969e3a8f6..335d5f2367 100644 --- a/repos/base/include/base/local_connection.h +++ b/repos/base/include/base/local_connection.h @@ -97,13 +97,15 @@ struct Genode::Local_connection_base : Noncopyable "after ", (int)NUM_ATTEMPTS, " attempts"); } - ~Local_connection_base() + void close() { if (_session_state->alive()) { _session_state->phase = Session_state::CLOSE_REQUESTED; _session_state->service().initiate_request(*_session_state); } } + + ~Local_connection_base() { close(); } }; @@ -172,6 +174,10 @@ class Genode::Local_connection : Local_connection_base { service.wakeup(); } + + bool alive() const { return _session_state->alive(); } + + using Local_connection_base::close; }; #endif /* _INCLUDE__BASE__LOCAL_CONNECTION_H_ */ diff --git a/repos/base/src/lib/base/child.cc b/repos/base/src/lib/base/child.cc index 26faf641f2..42d45b0dd0 100644 --- a/repos/base/src/lib/base/child.cc +++ b/repos/base/src/lib/base/child.cc @@ -817,6 +817,16 @@ void Child::close_all_sessions() } catch (Child_policy::Nonexistent_id_space) { } + /* + * Issue close requests to the providers of the environment sessions, + * which may be async services. Don't close the PD session since it + * is still needed for reverting session quotas. + */ + _log.close(); + _binary.close(); + if (_linker.constructed()) + _linker->close(); + /* * Remove statically created env sessions from the child's ID space. */ @@ -836,6 +846,9 @@ void Child::close_all_sessions() }; while (_id_space.apply_any(close_fn)); + + if (!KERNEL_SUPPORTS_EAGER_CHILD_DESTRUCTION) + _cpu._connection.destruct(); } diff --git a/repos/os/run/init.run b/repos/os/run/init.run index 36289e4ebf..d712877828 100644 --- a/repos/os/run/init.run +++ b/repos/os/run/init.run @@ -426,7 +426,7 @@ append config { - + @@ -1051,6 +1051,64 @@ append config { + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1094,5 +1152,5 @@ build_boot_image $boot_modules append qemu_args " -nographic " -run_genode_until {.*child "test-init" exited with exit value 0.*} 150 +run_genode_until {.*child "test-init" exited with exit value 0.*} 170 diff --git a/repos/os/src/init/child.cc b/repos/os/src/init/child.cc index c2df046a88..c7e81ef34c 100644 --- a/repos/os/src/init/child.cc +++ b/repos/os/src/init/child.cc @@ -15,6 +15,14 @@ #include +void Init::Child::destroy_services() +{ + _child_services.for_each([&] (Routed_service &service) { + if (service.has_id_space(_session_requester.id_space())) + destroy(_alloc, &service); }); +} + + Init::Child::Apply_config_result Init::Child::apply_config(Xml_node start_node) { @@ -267,6 +275,9 @@ void Init::Child::apply_ram_downgrade() void Init::Child::report_state(Xml_generator &xml, Report_detail const &detail) const { + /* true if it's safe to call the PD for requesting resource information */ + bool const pd_alive = !abandoned() && !_exited; + xml.node("child", [&] () { xml.attribute("name", _unique_name); @@ -290,7 +301,8 @@ void Init::Child::report_state(Xml_generator &xml, Report_detail const &detail) xml.attribute("assigned", String<32> { Number_of_bytes(_resources.assigned_ram_quota.value) }); - generate_ram_info(xml, _child.ram()); + if (pd_alive) + generate_ram_info(xml, _child.ram()); if (_requested_resources.constructed() && _requested_resources->ram.value) xml.attribute("requested", String<32>(_requested_resources->ram)); @@ -302,7 +314,8 @@ void Init::Child::report_state(Xml_generator &xml, Report_detail const &detail) xml.attribute("assigned", String<32>(_resources.assigned_cap_quota)); - generate_caps_info(xml, _child.pd()); + if (pd_alive) + generate_caps_info(xml, _child.pd()); if (_requested_resources.constructed() && _requested_resources->caps.value) xml.attribute("requested", String<32>(_requested_resources->caps)); @@ -687,9 +700,4 @@ Init::Child::Child(Env &env, } -Init::Child::~Child() -{ - _child_services.for_each([&] (Routed_service &service) { - if (service.has_id_space(_session_requester.id_space())) - destroy(_alloc, &service); }); -} +Init::Child::~Child() { } diff --git a/repos/os/src/init/child.h b/repos/os/src/init/child.h index 888d8b49f9..24a6ec707f 100644 --- a/repos/os/src/init/child.h +++ b/repos/os/src/init/child.h @@ -383,7 +383,7 @@ class Init::Child : Child_policy, Routed_service::Wakeup service.name() == node.attribute_value("name", Service::Name())) exists = true; }); - return exists; + return exists && !abandoned(); } void _add_service(Xml_node service) @@ -410,6 +410,8 @@ class Init::Child : Child_policy, Routed_service::Wakeup bool _exited { false }; int _exit_value { -1 }; + void _destroy_services(); + public: /** @@ -491,8 +493,14 @@ class Init::Child : Child_policy, Routed_service::Wakeup service.abandon(); }); } + void destroy_services(); + + void close_all_sessions() { _child.close_all_sessions(); } + bool abandoned() const { return _state == STATE_ABANDONED; } + bool env_sessions_closed() const { return _child.env_sessions_closed(); } + enum Apply_config_result { MAY_HAVE_SIDE_EFFECTS, NO_SIDE_EFFECTS }; /** diff --git a/repos/os/src/init/main.cc b/repos/os/src/init/main.cc index c2ac2f6a91..64ebb86a4b 100644 --- a/repos/os/src/init/main.cc +++ b/repos/os/src/init/main.cc @@ -273,7 +273,7 @@ void Init::Main::_update_children_config() node.attribute_value("name", Child_policy::Name()); _children.for_each_child([&] (Child &child) { - if (child.name() == start_node_name) { + if (!child.abandoned() && child.name() == start_node_name) { switch (child.apply_config(node)) { case Child::NO_SIDE_EFFECTS: break; case Child::MAY_HAVE_SIDE_EFFECTS: side_effects = true; break; @@ -318,7 +318,16 @@ void Init::Main::_handle_config() /* kill abandoned children */ _children.for_each_child([&] (Child &child) { - if (child.abandoned()) { + + if (!child.abandoned()) + return; + + /* make the child's services unavailable */ + child.destroy_services(); + child.close_all_sessions(); + + /* destroy child once all environment sessions are gone */ + if (child.env_sessions_closed()) { _children.remove(&child); destroy(_heap, &child); } @@ -341,7 +350,8 @@ void Init::Main::_handle_config() /* skip start node if corresponding child already exists */ bool exists = false; _children.for_each_child([&] (Child const &child) { - if (child.name() == start_node.attribute_value("name", Child_policy::Name())) + if (!child.abandoned() + && child.name() == start_node.attribute_value("name", Child_policy::Name())) exists = true; }); if (exists) { return; @@ -409,13 +419,15 @@ void Init::Main::_handle_config() * Initiate RAM sessions of all new children */ _children.for_each_child([&] (Child &child) { - child.initiate_env_ram_session(); }); + if (!child.abandoned()) + child.initiate_env_ram_session(); }); /* * Initiate remaining environment sessions of all new children */ _children.for_each_child([&] (Child &child) { - child.initiate_env_sessions(); }); + if (!child.abandoned()) + child.initiate_env_sessions(); }); /* * (Re-)distribute RAM among the childen, given their resource assignments