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

@@ -451,12 +451,21 @@ class Genode::Child : protected Rpc_object<Parent>,
 				{
 					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<Parent>,
 			Capability<SESSION> cap() const {
 				return _connection.constructed() ? _connection->cap()
 				                                 : Capability<SESSION>(); }
+
+			bool alive() const { return _connection.constructed() && _connection->alive(); }
+
+			void close() { if (_connection.constructed()) _connection->close(); }
 		};
 
 		Env_connection<Pd_connection>  _pd     { *this, Env::pd(),     _policy.name() };
@@ -666,6 +679,21 @@ class Genode::Child : protected Rpc_object<Parent>,
 		 */
 		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
 		 */

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_ */

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<Session_state>(close_fn));
+
+	if (!KERNEL_SUPPORTS_EAGER_CHILD_DESTRUCTION)
+		_cpu._connection.destruct();
 }
 
 

repos/os/run/init.run

@@ -426,7 +426,7 @@ append config {
 			<sleep ms="100"/>
 
 
-			<message string="test label rewritiong and binary-name update"/>
+			<message string="test label rewriting and binary-name update"/>
 
 			<init_config>
 				<parent-provides>
@@ -1051,6 +1051,64 @@ append config {
 			</expect_init_state>
 			<sleep ms="150"/>
 
+
+			<message string="test destruction of async env sessions"/>
+
+			<init_config version="async env session">
+				<parent-provides>
+					<service name="ROM"/>
+					<service name="CPU"/>
+					<service name="PD"/>
+					<service name="LOG"/>
+				</parent-provides>
+				<default caps="100"/>
+				<start name="server">
+					<binary name="dummy"/>
+					<resource name="RAM" quantum="1M"/>
+					<provides> <service name="LOG"/> </provides>
+					<config> <log_service/> </config>
+					<route> <any-service> <parent/> </any-service> </route>
+				</start>
+				<start name="dummy" version="1">
+					<resource name="RAM" quantum="1M"/>
+					<config> <log string="started version 1"/> </config>
+					<route>
+						<service name="LOG">
+							<child name="server"/> </service>
+						<any-service> <parent/> </any-service>
+					</route>
+				</start>
+			</init_config>
+			<expect_log string="[init -> server] [dummy] started version 1"/>
+			<sleep ms="150"/>
+			<init_config version="async env session">
+				<parent-provides>
+					<service name="ROM"/>
+					<service name="CPU"/>
+					<service name="PD"/>
+					<service name="LOG"/>
+				</parent-provides>
+				<default caps="100"/>
+				<start name="server">
+					<binary name="dummy"/>
+					<resource name="RAM" quantum="1M"/>
+					<provides> <service name="LOG"/> </provides>
+					<config> <log_service/> </config>
+					<route> <any-service> <parent/> </any-service> </route>
+				</start>
+				<start name="dummy" version="2">
+					<resource name="RAM" quantum="1M"/>
+					<config> <log string="started version 2"/> </config>
+					<route>
+						<service name="LOG">
+							<child name="server"/> </service>
+						<any-service> <parent/> </any-service>
+					</route>
+				</start>
+			</init_config>
+			<expect_log string="[init -> server] [dummy] started version 2"/>
+			<sleep ms="150"/>
+
 			<message string="test complete"/>
 
 		</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
 

repos/os/src/init/child.cc

@@ -15,6 +15,14 @@
 #include <child.h>
 
 
+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() { }

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 };
 
 		/**

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