Re: [PATCH] Support threads in modules

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1079 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am Sa., 10. Juni 2017 um
21:58 Uhr:

>
>
>> Once again: there _are_ legitimate situations in Emacs when for a
>> short time current_thread is NULL.  If you assume that these
>> situations don't happen, your code will sooner or later crash and
>> burn.  I'm saying this because I once thought current_thread should be
>> non-NULL at all times, and my code which assumed that did crash.
>>
>
> I've reviewed the threads code, and I'm quite sure that current_thread can
> never be NULL during check_thread. current_thread is only NULL between
> exiting a Lisp thread and resuming execution in another thread after
> reacquiring the GIL. The evaluator doesn't run during that phase, and
> therefore no module functions can run.
> current_thread could only be NULL during check_thread if called from a
> thread that is not a Lisp thread (i.e. an OS thread created by the module).
> That's exactly one of the undefined behavior situations that the assertions
> are meant to prevent.
>
>
Here's a new patch, which implements option (1).

[-- Attachment #1.2: Type: text/html, Size: 1717 bytes --]

[-- Attachment #2: 0001-Support-threads-in-modules.txt --]
[-- Type: text/plain, Size: 4840 bytes --]

From 8160c7d914882b40badf9eb8e4792da1b313745e Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sat, 22 Apr 2017 16:51:44 +0200
Subject: [PATCH] Support threads in modules

Rather than checking for the main thread, check for the current
thread.

* emacs-module.c (check_thread): New function.
(MODULE_FUNCTION_BEGIN_NO_CATCH, module_get_environment)
(module_non_local_exit_check, module_non_local_exit_clear)
(module_non_local_exit_get, module_non_local_exit_signal)
(module_non_local_exit_throw, module_is_not_nil, module_eq): Use it.
---
 src/emacs-module.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index c0f2c3fcd0..afb75e351d 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -30,6 +30,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "coding.h"
 #include "keyboard.h"
 #include "syssignal.h"
+#include "thread.h"
 
 #include <intprops.h>
 #include <verify.h>
@@ -94,7 +95,7 @@ struct module_fun_env;
 static Lisp_Object value_to_lisp (emacs_value);
 static emacs_value lisp_to_value (Lisp_Object);
 static enum emacs_funcall_exit module_non_local_exit_check (emacs_env *);
-static void check_main_thread (void);
+static void check_thread (void);
 static void initialize_environment (emacs_env *, struct emacs_env_private *);
 static void finalize_environment (emacs_env *);
 static void finalize_environment_unwind (void *);
@@ -181,7 +182,7 @@ static emacs_value const module_nil = 0;
 
    1. The first argument should always be a pointer to emacs_env.
 
-   2. Each function should first call check_main_thread.  Note that
+   2. Each function should first call check_thread.  Note that
       this function is a no-op unless Emacs was built with
       --enable-checking.
 
@@ -215,7 +216,7 @@ static emacs_value const module_nil = 0;
 
 #define MODULE_FUNCTION_BEGIN_NO_CATCH(error_retval)                    \
   do {                                                                  \
-    check_main_thread ();                                               \
+    check_thread ();                                                    \
     if (module_non_local_exit_check (env) != emacs_funcall_exit_return) \
       return error_retval;                                              \
   } while (false)
@@ -241,8 +242,9 @@ CHECK_USER_PTR (Lisp_Object obj)
 static emacs_env *
 module_get_environment (struct emacs_runtime *ert)
 {
-  check_main_thread ();
-  return &ert->private_members->pub;
+  emacs_env *env = &ert->private_members->pub;
+  check_thread ();
+  return env;
 }
 
 /* To make global refs (GC-protected global values) keep a hash that
@@ -303,21 +305,21 @@ module_free_global_ref (emacs_env *env, emacs_value ref)
 static enum emacs_funcall_exit
 module_non_local_exit_check (emacs_env *env)
 {
-  check_main_thread ();
+  check_thread ();
   return env->private_members->pending_non_local_exit;
 }
 
 static void
 module_non_local_exit_clear (emacs_env *env)
 {
-  check_main_thread ();
+  check_thread ();
   env->private_members->pending_non_local_exit = emacs_funcall_exit_return;
 }
 
 static enum emacs_funcall_exit
 module_non_local_exit_get (emacs_env *env, emacs_value *sym, emacs_value *data)
 {
-  check_main_thread ();
+  check_thread ();
   struct emacs_env_private *p = env->private_members;
   if (p->pending_non_local_exit != emacs_funcall_exit_return)
     {
@@ -332,7 +334,7 @@ module_non_local_exit_get (emacs_env *env, emacs_value *sym, emacs_value *data)
 static void
 module_non_local_exit_signal (emacs_env *env, emacs_value sym, emacs_value data)
 {
-  check_main_thread ();
+  check_thread ();
   if (module_non_local_exit_check (env) == emacs_funcall_exit_return)
     module_non_local_exit_signal_1 (env, value_to_lisp (sym),
 				    value_to_lisp (data));
@@ -341,7 +343,7 @@ module_non_local_exit_signal (emacs_env *env, emacs_value sym, emacs_value data)
 static void
 module_non_local_exit_throw (emacs_env *env, emacs_value tag, emacs_value value)
 {
-  check_main_thread ();
+  check_thread ();
   if (module_non_local_exit_check (env) == emacs_funcall_exit_return)
     module_non_local_exit_throw_1 (env, value_to_lisp (tag),
 				   value_to_lisp (value));
@@ -724,12 +726,13 @@ module_function_arity (const struct Lisp_Module_Function *const function)
 /* Helper functions.  */
 
 static void
-check_main_thread (void)
+check_thread (void)
 {
+  eassert (current_thread != NULL);
 #ifdef HAVE_PTHREAD
-  eassert (pthread_equal (pthread_self (), main_thread_id));
+  eassert (pthread_equal (pthread_self (), current_thread->thread_id));
 #elif defined WINDOWSNT
-  eassert (GetCurrentThreadId () == dwMainThreadId);
+  eassert (GetCurrentThreadId () == current_thread->thread_id);
 #endif
 }
 
-- 
2.13.1