Noux: delayed 'Child' object destruction

With this patch the destruction of Noux 'Child' objects gets delayed
further until the exit signal has been dispatched. This prevents the
self-destruction of the signal dispatcher, which is a member of the
'Child' object.

Fixes #603.

ports/src/noux/child.h

@@ -30,6 +30,8 @@
 #include <cpu_session_component.h>
 #include <child_policy.h>
 #include <io_receptor_registry.h>
+#include <destruct_queue.h>
+#include <destruct_dispatcher.h>
 
 
 namespace Noux {
@@ -83,63 +85,10 @@ namespace Noux {
 	bool is_init_process(Child *child);
 	void init_process_exited();
 
-
-	/**
-	 * Signal context used for child exit
-	 */
-	class Child_exit_dispatcher : public Signal_dispatcher_base
-	{
-		private:
-
-			Child *_child;
-
-		public:
-
-			Child_exit_dispatcher(Child *child) : _child(child) { }
-
-			void dispatch(unsigned)
-			{
-				if (is_init_process(_child)) {
-					PINF("init process exited");
-
-					/* trigger exit of main event loop */
-					init_process_exited();
-				} else {
-					/* destroy 'Noux::Child' */
-					destroy(env()->heap(), _child);
-
-					PINF("destroy %p", _child);
-					PINF("quota: avail=%zd, used=%zd",
-					     env()->ram_session()->avail(),
-					     env()->ram_session()->used());
-				}
-			}
-	};
-
-
-	/**
-	 * Signal context used for removing the child after having executed 'execve'
-	 */
-	class Child_execve_cleanup_dispatcher : public Signal_dispatcher_base
-	{
-		private:
-
-			Child *_child;
-
-		public:
-
-			Child_execve_cleanup_dispatcher(Child *child) : _child(child) { }
-
-			void dispatch(unsigned)
-			{
-				destroy(env()->heap(), _child);
-			}
-	};
-
-
 	class Child : public Rpc_object<Session>,
 	              public File_descriptor_registry,
-	              public Family_member
+	              public Family_member,
+	              public Destruct_queue::Element<Child>
 	{
 		private:
 
@@ -150,11 +99,10 @@ namespace Noux {
 			 */
 			Semaphore _blocker;
 
-			Child_exit_dispatcher     _exit_dispatcher;
+			Allocator                 *_alloc;
-			Signal_context_capability _exit_context_cap;
+			Destruct_queue            &_destruct_queue;
-
+			Destruct_dispatcher        _destruct_dispatcher;
-			Child_execve_cleanup_dispatcher _execve_cleanup_dispatcher;
+			Signal_context_capability  _destruct_context_cap;
-			Signal_context_capability       _execve_cleanup_context_cap;
 
 			Cap_session * const _cap_session;
 
@@ -292,24 +240,26 @@ namespace Noux {
 			 *                               looked up at the virtual file
 			 *                               system
 			 */
-			Child(char const       *name,
+			Child(char const        *name,
-			      Family_member    *parent,
+			      Family_member     *parent,
-			      int               pid,
+			      int                pid,
-			      Signal_receiver  *sig_rec,
+			      Signal_receiver   *sig_rec,
-			      Dir_file_system  *root_dir,
+			      Dir_file_system   *root_dir,
-			      Args              const &args,
+			      Args               const &args,
-			      Sysio::Env        const &env,
+			      Sysio::Env         const &env,
-			      Cap_session      *cap_session,
+			      Cap_session       *cap_session,
-			      Service_registry &parent_services,
+			      Service_registry  &parent_services,
-			      Rpc_entrypoint   &resources_ep,
+			      Rpc_entrypoint    &resources_ep,
-			      bool              forked)
+			      bool               forked,
+			      Allocator         *destruct_alloc,
+			      Destruct_queue    &destruct_queue)
 			:
 				Family_member(pid, parent),
+				Destruct_queue::Element<Child>(destruct_alloc),
 				_sig_rec(sig_rec),
-				_exit_dispatcher(this),
+				_destruct_queue(destruct_queue),
-				_exit_context_cap(sig_rec->manage(&_exit_dispatcher)),
+				_destruct_dispatcher(_destruct_queue, this),
-				_execve_cleanup_dispatcher(this),
+				_destruct_context_cap(sig_rec->manage(&_destruct_dispatcher)),
-				_execve_cleanup_context_cap(sig_rec->manage(&_execve_cleanup_dispatcher)),
 				_cap_session(cap_session),
 				_entrypoint(cap_session, STACK_SIZE, "noux_process", false),
 				_resources(name, resources_ep, false),
@@ -327,7 +277,7 @@ namespace Noux {
 				_child_policy(name, _binary_ds, _args.cap(), _env.cap(),
 				              _entrypoint, _local_noux_service,
 				              _local_rm_service, _parent_services,
-				              *this, *this, _exit_context_cap, _resources.ram),
+				              *this, *this, _destruct_context_cap, _resources.ram),
 				_child(_binary_ds, _resources.ram.cap(), _resources.cpu.cap(),
 				       _resources.rm.cap(), &_entrypoint, &_child_policy)
 			{
@@ -341,8 +291,7 @@ namespace Noux {
 
 			~Child()
 			{
-				_sig_rec->dissolve(&_execve_cleanup_dispatcher);
+				_sig_rec->dissolve(&_destruct_dispatcher);
-				_sig_rec->dissolve(&_exit_dispatcher);
 
 				_entrypoint.dissolve(this);
 
@@ -364,7 +313,14 @@ namespace Noux {
 
 			void submit_exit_signal()
 			{
-				Signal_transmitter(_exit_context_cap).submit();
+				if (is_init_process(this)) {
+					PINF("init process exited");
+
+					/* trigger exit of main event loop */
+					init_process_exited();
+				} else {
+					Signal_transmitter(_destruct_context_cap).submit();
+				}
 			}
 
 			Ram_session_capability ram() const { return _resources.ram.cap(); }

ports/src/noux/child_policy.h

@@ -41,7 +41,7 @@ namespace Noux {
 			Service_registry                   &_parent_services;
 			Family_member                      &_family_member;
 			File_descriptor_registry           &_file_descriptor_registry;
-			Signal_context_capability           _exit_context_cap;
+			Signal_context_capability           _destruct_context_cap;
 			Ram_session                        &_ref_ram_session;
 
 		public:
@@ -56,7 +56,7 @@ namespace Noux {
 			             Service_registry         &parent_services,
 			             Family_member            &family_member,
 			             File_descriptor_registry &file_descriptor_registry,
-			             Signal_context_capability exit_context_cap,
+			             Signal_context_capability destruct_context_cap,
 			             Ram_session              &ref_ram_session)
 			:
 				_name(strncpy(_name_buf, name, sizeof(_name_buf))),
@@ -69,7 +69,7 @@ namespace Noux {
 				_parent_services(parent_services),
 				_family_member(family_member),
 				_file_descriptor_registry(file_descriptor_registry),
-				_exit_context_cap(exit_context_cap),
+				_destruct_context_cap(destruct_context_cap),
 				_ref_ram_session(ref_ram_session)
 			{ }
 
@@ -122,7 +122,7 @@ namespace Noux {
 
 				/* handle exit of the init process */
 				if (_family_member.parent() == 0)
-					Signal_transmitter(_exit_context_cap).submit();
+					Signal_transmitter(_destruct_context_cap).submit();
 			}
 
 			Ram_session *ref_ram_session()

ports/src/noux/destruct_dispatcher.h

@@ -0,0 +1,48 @@
+/*
+ * \brief  Signal_dispatcher which adds a destruct queue element into a
+ *         destruct queue
+ * \author Christian Prochaska
+ * \date   2013-01-03
+ */
+
+/*
+ * Copyright (C) 2013 Genode Labs GmbH
+ *
+ * This file is part of the Genode OS framework, which is distributed
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#ifndef _NOUX__DESTRUCT_DISPATCHER_H_
+#define _NOUX__DESTRUCT_DISPATCHER_H_
+
+/* Genode includes */
+#include <base/signal.h>
+
+/* Noux includes */
+#include <destruct_queue.h>
+
+namespace Noux {
+
+	using namespace Genode;
+
+	class Destruct_dispatcher : public Signal_dispatcher_base
+	{
+		private:
+
+			Destruct_queue               &_destruct_queue;
+			Destruct_queue::Element_base *_element;
+
+		public:
+
+			Destruct_dispatcher(Destruct_queue &destruct_queue,
+			                    Destruct_queue::Element_base *element)
+			: _destruct_queue(destruct_queue), _element(element) { }
+
+			void dispatch(unsigned)
+			{
+				_destruct_queue.insert(_element);
+			}
+	};
+}
+
+#endif /* _NOUX__DESTRUCT_DISPATCHER_H_ */

ports/src/noux/destruct_queue.h

@@ -0,0 +1,94 @@
+/*
+ * \brief  Queue for delayed object destruction
+ * \author Christian Prochaska
+ * \date   2013-01-03
+ */
+
+/*
+ * Copyright (C) 2013 Genode Labs GmbH
+ *
+ * This file is part of the Genode OS framework, which is distributed
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#ifndef _NOUX__DESTRUCT_QUEUE_H_
+#define _NOUX__DESTRUCT_QUEUE_H_
+
+/* Genode includes */
+#include <base/allocator.h>
+#include <util/list.h>
+
+namespace Noux {
+
+	class Destruct_queue
+	{
+		public:
+
+			struct Element_base : Genode::List<Element_base>::Element
+			{
+				virtual void destroy() = 0;
+			};
+
+			/*
+			 * When a pointer to an object which inherits 'Element' among other
+			 * base classes gets static-casted to a pointer to the 'Element'
+			 * base object, the resulting address can differ from the start
+			 * address of the inherited object. To be able to pass the start
+			 * address of the inherited object to the allocator, a static-cast
+			 * back to the inherited class needs to be performed. Therefore the
+			 * type of the class inheriting from 'Element' needs to be given as
+			 * template parameter.
+			 */
+			template <typename T>
+			class Element : public Element_base
+			{
+				private:
+
+					Genode::Allocator *_alloc;
+
+				public:
+
+					/**
+					 * Constructor
+					 *
+					 * \param alloc  the allocator which was used to allocate
+					 *               the element
+					 */
+					Element(Genode::Allocator *alloc) : _alloc(alloc) { }
+
+					virtual ~Element() { };
+
+					void destroy()
+					{
+						Genode::destroy(_alloc, static_cast<T*>(this));
+					}
+			};
+
+		private:
+
+			Genode::List<Element_base> _destruct_list;
+			Genode::Lock               _destruct_list_lock;
+
+		public:
+
+			void insert(Element_base *element)
+			{
+				Genode::Lock::Guard guard(_destruct_list_lock);
+				_destruct_list.insert(element);
+			}
+
+			void flush()
+			{
+				Genode::Lock::Guard guard(_destruct_list_lock);
+
+				Element_base *element;
+				while ((element = _destruct_list.first())) {
+					_destruct_list.remove(element);
+					element->destroy();
+				}
+			}
+	};
+
+}
+
+#endif /* _NOUX__DESTRUCT_QUEUE_H_ */

ports/src/noux/main.cc

@@ -28,10 +28,11 @@
 #include <dir_file_system.h>
 #include <user_info.h>
 #include <io_receptor_registry.h>
+#include <destruct_queue.h>
 
 
 static bool trace_syscalls = false;
-
+static bool verbose_quota  = false;
 
 namespace Noux {
 
@@ -272,7 +273,9 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 					                         _cap_session,
 					                         _parent_services,
 					                         _resources.ep,
-					                         false);
+					                         false,
+					                         env()->heap(),
+					                         _destruct_queue);
 
 					/* replace ourself by the new child at the parent */
 					parent()->remove(this);
@@ -281,7 +284,7 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 					_assign_io_channels_to(child);
 
 					/* signal main thread to remove ourself */
-					Genode::Signal_transmitter(_execve_cleanup_context_cap).submit();
+					Genode::Signal_transmitter(_destruct_context_cap).submit();
 
 					/* start executing the new process */
 					child->start();
@@ -486,7 +489,9 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 				                         _cap_session,
 				                         _parent_services,
 				                         _resources.ep,
-				                         true);
+				                         true,
+				                         env()->heap(),
+				                         _destruct_queue);
 
 				Family_member::insert(child);
 
@@ -863,6 +868,7 @@ int main(int argc, char **argv)
 
 	/* create init process */
 	static Genode::Signal_receiver sig_rec;
+	static Destruct_queue destruct_queue;
 
 	init_child = new Noux::Child(name_of_init_process(),
 	                             0,
@@ -874,7 +880,9 @@ int main(int argc, char **argv)
 	                             &cap,
 	                             parent_services,
 	                             resources_ep,
-	                             false);
+	                             false,
+	                             env()->heap(),
+	                             destruct_queue);
 
 	/*
 	 * I/O channels must be dynamically allocated to handle cases where the
@@ -902,6 +910,13 @@ int main(int argc, char **argv)
 
 		for (unsigned i = 0; i < signal.num(); i++)
 			dispatcher->dispatch(1);
+
+		destruct_queue.flush();
+
+		if (verbose_quota)
+			PINF("quota: avail=%zd, used=%zd",
+				 env()->ram_session()->avail(),
+				 env()->ram_session()->used());
 	}
 
 	PINF("-- exiting noux ---");