From 1e5e86c1d2702dbdf3a5cd7924bb8f4c7f6d1d02 Mon Sep 17 00:00:00 2001 From: Fabrizio Fabbri Date: Tue, 23 Aug 2016 15:28:23 -0400 Subject: [PATCH 1/2] Fix on several minor issue on thread. - fix #262 to manage CTRL+c on Win. - unregistered thread are left registered and enviroment not cleanup. - manage when a finalizer is invoked before a valid enviroment is available. --- src/c/alloc_2.d | 36 ++++++++++++++++++++++++++++++++++++ src/c/threads/process.d | 2 +- src/c/unixint.d | 24 ++++++++++-------------- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/src/c/alloc_2.d b/src/c/alloc_2.d index 7e785d392..377eaa845 100644 --- a/src/c/alloc_2.d +++ b/src/c/alloc_2.d @@ -1124,9 +1124,45 @@ standard_finalizer(cl_object o) } static void +wrapped_finalizer(cl_object o, cl_object finalizer); + +static void +deferred_finalizer(cl_object o) +{ + wrapped_finalizer(cl_first(o), cl_second(o)); +} + +void wrapped_finalizer(cl_object o, cl_object finalizer) { if (finalizer != ECL_NIL && finalizer != NULL) { + const cl_env_ptr the_env = ecl_process_env(); + if ( !the_env + || + !the_env->own_process + || + the_env->own_process->process.phase < ECL_PROCESS_ACTIVE ) + { + // The finalizer is invoked while we are + // registering or setup a new lisp process. + // As example that may happen when we are doing + // ecl_import_current_thread. + // That mean the finalizer can not be executed right now, + // so in some way we need to queue the finalization. + // When we return from this function the original finalizer + // is no more registered to o, and if o is not anymore reachable + // it will be colleted. + // To prevent this we need to make this object reachable again after + // that roundtrip and postpone the finalization to the next + // garbace colletion. + // Given that this is a rare condition one way to do that is: + GC_finalization_proc ofn; + void *odata; + GC_register_finalizer_no_order(cl_list(2,o,finalizer), + (GC_finalization_proc)deferred_finalizer, 0, + &ofn, &odata); + return; + } CL_NEWENV_BEGIN { if (finalizer != ECL_T) { funcall(2, finalizer, o); diff --git a/src/c/threads/process.d b/src/c/threads/process.d index a53b0ea07..19e98e340 100755 --- a/src/c/threads/process.d +++ b/src/c/threads/process.d @@ -382,7 +382,6 @@ ecl_import_current_thread(cl_object name, cl_object bindings) ecl_set_process_env(env_aux); env = _ecl_alloc_env(0); ecl_set_process_env(env); - env->cleanup = registered; /* Link environment and process together */ env->own_process = process = alloc_process(name, bindings); @@ -392,6 +391,7 @@ ecl_import_current_thread(cl_object name, cl_object bindings) ecl_list_process(process); ecl_init_env(env); + env->cleanup = registered; env->bindings_array = process->process.initial_bindings; env->thread_local_bindings_size = env->bindings_array->vector.dim; env->thread_local_bindings = env->bindings_array->vector.self.t; diff --git a/src/c/unixint.d b/src/c/unixint.d index 087b87c5c..9d375e956 100644 --- a/src/c/unixint.d +++ b/src/c/unixint.d @@ -1122,16 +1122,11 @@ _ecl_w32_exception_filter(struct _EXCEPTION_POINTERS* ep) static cl_object W32_handle_in_new_thread(cl_object signal_code) { - /* XXX: there is some bug present only on windows platform - with importing the current thread. Don't know how to track - it though. */ -#if 0 int outside_ecl = ecl_import_current_thread(@'si::handle-signal', ECL_NIL); mp_process_run_function(4, @'si::handle-signal', @'si::handle-signal', signal_code, ECL_NIL); if (outside_ecl) ecl_release_current_thread(); -#endif /* 0 */ } BOOL WINAPI W32_console_ctrl_handler(DWORD type) @@ -1139,20 +1134,21 @@ BOOL WINAPI W32_console_ctrl_handler(DWORD type) switch (type) { case CTRL_C_EVENT: case CTRL_BREAK_EVENT: { - /* cl_object function = */ - /* ECL_SYM_FUN(@'si::terminal-interrupt'); */ - /* if (function) */ - /* W32_handle_in_new_thread(function); */ + cl_object function = + ECL_SYM_FUN(@'si::terminal-interrupt'); + if (function) + W32_handle_in_new_thread(function); return TRUE; } case CTRL_CLOSE_EVENT: case CTRL_LOGOFF_EVENT: - case CTRL_SHUTDOWN_EVENT: - /* Doing nothing is arguably the most - reasonable. Calling (quit) causes process to exit - and Windows has problems, because "process has - unexpectably died.*/ + case CTRL_SHUTDOWN_EVENT: { + cl_object function = + ECL_SYM_FUN(@'ext::quit'); + if (function) + W32_handle_in_new_thread(function); return TRUE; + } default: return FALSE; } From 28a0f957fe4d75f997f1c0f84e5046e2f2b7b327 Mon Sep 17 00:00:00 2001 From: Fabrizio Fabbri Date: Mon, 29 Aug 2016 03:55:16 -0400 Subject: [PATCH 2/2] Use the project comment style. --- src/c/alloc_2.d | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/c/alloc_2.d b/src/c/alloc_2.d index 377eaa845..b6d5c7f75 100644 --- a/src/c/alloc_2.d +++ b/src/c/alloc_2.d @@ -1143,25 +1143,27 @@ wrapped_finalizer(cl_object o, cl_object finalizer) || the_env->own_process->process.phase < ECL_PROCESS_ACTIVE ) { - // The finalizer is invoked while we are - // registering or setup a new lisp process. - // As example that may happen when we are doing - // ecl_import_current_thread. - // That mean the finalizer can not be executed right now, - // so in some way we need to queue the finalization. - // When we return from this function the original finalizer - // is no more registered to o, and if o is not anymore reachable - // it will be colleted. - // To prevent this we need to make this object reachable again after - // that roundtrip and postpone the finalization to the next - // garbace colletion. - // Given that this is a rare condition one way to do that is: - GC_finalization_proc ofn; - void *odata; - GC_register_finalizer_no_order(cl_list(2,o,finalizer), - (GC_finalization_proc)deferred_finalizer, 0, - &ofn, &odata); - return; + /* + * The finalizer is invoked while we are + * registering or setup a new lisp process. + * As example that may happen when we are doing + * ecl_import_current_thread. + * That mean the finalizer can not be executed right now, + * so in some way we need to queue the finalization. + * When we return from this function the original finalizer + * is no more registered to o, and if o is not anymore reachable + * it will be colleted. + * To prevent this we need to make this object reachable again after + * that roundtrip and postpone the finalization to the next + * garbace colletion. + * Given that this is a rare condition one way to do that is: + */ + GC_finalization_proc ofn; + void *odata; + GC_register_finalizer_no_order(cl_list(2,o,finalizer), + (GC_finalization_proc)deferred_finalizer, 0, + &ofn, &odata); + return; } CL_NEWENV_BEGIN { if (finalizer != ECL_T) {