[Ecls-list] [RFC PATCH] Fix GC for threads + mprotect signal handling.

[Alexander Gavrilov]

When ECL uses mprotect signal handling, it allocates the
environment structure using mmap; thus it is no longer
traversed by the GC during collection. Since some objects,
e.g. the table of local bindings, are added to the structure
in advance, they are likely to be freed if a garbage
collection occurs before the thread is fully started.
In order to avoid this, it is necessary to register roots.

Signed-off-by: Alexander Gavrilov <angavrilov at gmail.com>
---

	Note that if even with this change ecl_mark_env does
	something important, there is still a race condition:

	< Master calls GC_pthread_create, it blocks
		> GC registers the child and wakes up master
		> Child initializes env
		> Child causes a garbage collection
	< Master adds the child to cl_core.processes

	I think that cl_core.processes probably should not
	be used for garbage collection purposes at all.

	Last Saturday the scheduler on my computer got into
	a mood where this race was triggered pretty predictably,
	which allowed me to debug this problem. After making
	this patch I haven't seen any more crashes or weird
	errors about undefined symbols.

	Alexander

	P.S. The finalization part is not tested, because my
	  program never destroys its threads. It also may be
	  a good idea to add only some important fields as
	  roots, but I don't know which exactly these are.

	P.P.S. It may be a good idea to allocate the full
	  environment only while the thread is running, and
	  keep persistent pointers in the process object.


 src/c/alloc_2.d  |    6 ++++++
 src/c/main.d     |   19 +++++++++++++++++++
 src/c/threads.d  |    3 +++
 src/h/internal.h |    1 +
 4 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/src/c/alloc_2.d b/src/c/alloc_2.d
index 7aad223..4380a15 100644
--- a/src/c/alloc_2.d
+++ b/src/c/alloc_2.d
@@ -460,6 +460,12 @@ standard_finalizer(cl_object o)
 		ecl_enable_interrupts_env(the_env);
 		break;
 	}
+#ifdef ECL_USE_MPROTECT
+	case t_process: {
+		_ecl_free_env(o->process.env);
+		break;
+	}
+#endif /* MPROTECT */
 #endif
 	default:;
 	}
diff --git a/src/c/main.d b/src/c/main.d
index 046df01..0d49da4 100644
--- a/src/c/main.d
+++ b/src/c/main.d
@@ -293,6 +293,14 @@ _ecl_alloc_env()
 			MAP_ANON | MAP_PRIVATE, -1, 0);
 	if (output == MAP_FAILED)
 		ecl_internal_error("Unable to allocate environment structure.");
+#if defined(ECL_THREADS) && defined(GBC_BOEHM)
+	/* The thread library allows creation of indefinitely retainable
+	 * and reusable process objects, which don't always exist on the
+	 * active process list. Thus it is necessary to add the area
+	 * to permanent roots.
+	 */
+	GC_add_roots((void*)output,(void*)(output+1));
+#endif
 #else
 	static struct cl_env_struct first_env;
 	if (!ecl_get_option(ECL_OPT_BOOTED)) {
@@ -312,6 +320,17 @@ _ecl_alloc_env()
 	return output;
 }
 
+void
+_ecl_free_env(cl_env_ptr env)
+{
+#if defined(ECL_USE_MPROTECT)
+#if defined(ECL_THREADS) && defined(GBC_BOEHM)
+	GC_remove_roots((void*)env,(void*)(env+1));
+#endif
+	munmap(env, sizeof(*env));
+#endif
+}
+
 int
 cl_shutdown(void)
 {
diff --git a/src/c/threads.d b/src/c/threads.d
index 01ae6fa..92fa4c6 100644
--- a/src/c/threads.d
+++ b/src/c/threads.d
@@ -179,6 +179,9 @@ alloc_process(cl_object name)
 	process->process.interrupt = Cnil;
 	process->process.env = _ecl_alloc_env();
 	process->process.env->own_process = process;
+#ifdef ECL_USE_MPROTECT
+	si_set_finalizer(process, Ct);
+#endif
 	return process;
 }
 
diff --git a/src/h/internal.h b/src/h/internal.h
index e6faa77..1f244f5 100644
--- a/src/h/internal.h
+++ b/src/h/internal.h
@@ -52,6 +52,7 @@ extern void ecl_init_env(cl_env_ptr);
 extern void init_lib_LSP(cl_object);
 
 extern cl_env_ptr _ecl_alloc_env(void);
+extern void _ecl_free_env(cl_env_ptr);
 
 /* alloc.d/alloc_2.d */
 
-- 
1.6.3.rc3.12.gb7937