[PATCH] Support threads in modules

[PATCH] Support threads in modules

[Philipp Stephani]

Rather than checking for the main thread, store the owning thread in
the module structures and check for it.

* emacs-module.c (check_thread): New function.
(MODULE_FUNCTION_BEGIN, 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.
(initialize_environment): Initialize thread.
---
 src/emacs-module.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/emacs-module.c b/src/emacs-module.c
index 1b445dcc3b..22a0ed44f7 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -29,6 +29,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "dynlib.h"
 #include "coding.h"
 #include "syssignal.h"
+#include "thread.h"
 
 #include <intprops.h>
 #include <verify.h>
@@ -86,6 +87,9 @@ struct emacs_env_private
      storage is always available for them, even in an out-of-memory
      situation.  */
   Lisp_Object non_local_exit_symbol, non_local_exit_data;
+
+  /* Thread that owns this environment object. */
+  struct thread_state *thread;
 };
 
 /* The private parts of an `emacs_runtime' object contain the initial
@@ -106,7 +110,7 @@ static Lisp_Object module_format_fun_env (const 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 (emacs_env *);
 static void finalize_environment (struct emacs_env_private *);
 static void initialize_environment (emacs_env *, struct emacs_env_private *priv);
 static void module_handle_signal (emacs_env *, Lisp_Object);
@@ -206,7 +210,7 @@ struct module_fun_env
 
    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.
 
@@ -238,7 +242,7 @@ struct module_fun_env
    should be a sentinel value.  */
 
 #define MODULE_FUNCTION_BEGIN(error_retval)                             \
-  check_main_thread ();                                                 \
+  check_thread (env);                                                   \
   if (module_non_local_exit_check (env) != emacs_funcall_exit_return)   \
     return error_retval;                                                \
   MODULE_HANDLE_NONLOCAL_EXIT (error_retval)
@@ -256,8 +260,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 (env);
+  return env;
 }
 
 /* To make global refs (GC-protected global values) keep a hash that
@@ -318,21 +323,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 (env);
   return env->private_members->pending_non_local_exit;
 }
 
 static void
 module_non_local_exit_clear (emacs_env *env)
 {
-  check_main_thread ();
+  check_thread (env);
   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 (env);
   struct emacs_env_private *p = env->private_members;
   if (p->pending_non_local_exit != emacs_funcall_exit_return)
     {
@@ -347,7 +352,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 (env);
   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));
@@ -356,7 +361,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 (env);
   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));
@@ -448,7 +453,7 @@ module_type_of (emacs_env *env, emacs_value value)
 static bool
 module_is_not_nil (emacs_env *env, emacs_value value)
 {
-  check_main_thread ();
+  check_thread (env);
   if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
     return false;
   return ! NILP (value_to_lisp (value));
@@ -457,7 +462,7 @@ module_is_not_nil (emacs_env *env, emacs_value value)
 static bool
 module_eq (emacs_env *env, emacs_value a, emacs_value b)
 {
-  check_main_thread ();
+  check_thread (env);
   if (module_non_local_exit_check (env) != emacs_funcall_exit_return)
     return false;
   return EQ (value_to_lisp (a), value_to_lisp (b));
@@ -743,12 +748,16 @@ usage: (module-call ENVOBJ &rest ARGLIST)   */)
 /* Helper functions.  */
 
 static void
-check_main_thread (void)
+check_thread (emacs_env *env)
 {
+  struct thread_state *thread = env->private_members->thread;
+  eassert (thread != NULL);
+  eassert (current_thread != NULL);
+  eassert (current_thread == thread);
 #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
 }
 
@@ -902,6 +911,7 @@ static void
 initialize_environment (emacs_env *env, struct emacs_env_private *priv)
 {
   priv->pending_non_local_exit = emacs_funcall_exit_return;
+  priv->thread = current_thread;
   env->size = sizeof *env;
   env->private_members = priv;
   env->make_global_ref = module_make_global_ref;
-- 
2.12.2

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 22 Apr 2017 17:24:44 +0200
> Cc: Philipp Stephani <phst@google.com>
> 
> Rather than checking for the main thread, store the owning thread in
> the module structures and check for it.

Can you explain the purpose of these changes and the motivation?  A
module shouldn't be restricted to be used by a single thread, should
it?

Thanks.

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 22. Apr. 2017 um 21:09 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 22 Apr 2017 17:24:44 +0200
> > Cc: Philipp Stephani <phst@google.com>
> >
> > Rather than checking for the main thread, store the owning thread in
> > the module structures and check for it.
>
> Can you explain the purpose of these changes and the motivation?  A
> module shouldn't be restricted to be used by a single thread, should
> it?
>
>
No, but the thread used to create an environment object, the current Emacs
thread, and the current OS thread all have to match. Right now this isn't
checked; the current code checks only for the main thread, which isn't
correct any more now that there can be more than one interpreter thread.

[-- Attachment #2: Type: text/html, Size: 1231 bytes --]

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 22 Apr 2017 19:21:35 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
>  Can you explain the purpose of these changes and the motivation? A
>  module shouldn't be restricted to be used by a single thread, should
>  it?
> 
> No, but the thread used to create an environment object, the current Emacs thread, and the current OS thread
> all have to match. Right now this isn't checked; the current code checks only for the main thread, which isn't
> correct any more now that there can be more than one interpreter thread. 

I agree that checking for the main thread is not TRT, but why not
allow any thread of those in all_threads?  Why do we care that the env
pointer was created by the same thread as the one using it?  We should
only care that the invoking thread is one of the Emacs application
threads, no?

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 22. Apr. 2017 um 21:49 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 22 Apr 2017 19:21:35 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
> >
> >  Can you explain the purpose of these changes and the motivation? A
> >  module shouldn't be restricted to be used by a single thread, should
> >  it?
> >
> > No, but the thread used to create an environment object, the current
> Emacs thread, and the current OS thread
> > all have to match. Right now this isn't checked; the current code checks
> only for the main thread, which isn't
> > correct any more now that there can be more than one interpreter thread.
>
> I agree that checking for the main thread is not TRT, but why not
> allow any thread of those in all_threads?  Why do we care that the env
> pointer was created by the same thread as the one using it?  We should
> only care that the invoking thread is one of the Emacs application
> threads, no?
>

- Using objects across threads requires careful synchronization. It's not
impossible, but a large burden on module authors.
- The more restrictive the module API is, the more freedom we have in the
implementation.
- The current check is easy to implement and understand. A check for
multiple threads would require another hash table with thread IDs etc.
- The Emacs module API is modelled after JNI, which has the same
restriction (
http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html).
We should stick to the JNI semantics wherever possible, because JNI is a
well-tested and robust technology.

[-- Attachment #2: Type: text/html, Size: 2261 bytes --]

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 22 Apr 2017 20:05:19 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
>  I agree that checking for the main thread is not TRT, but why not
>  allow any thread of those in all_threads? Why do we care that the env
>  pointer was created by the same thread as the one using it? We should
>  only care that the invoking thread is one of the Emacs application
>  threads, no?
> 
> - Using objects across threads requires careful synchronization.

Not sure what synchronization you have in mind.  There's only one
running thread at any given time, and a thread cannot be preempted
while it runs Lisp, so synchronization seems unnecessary, as a thread
cannot modify a Lisp object that another thread is modifying.

> - The more restrictive the module API is, the more freedom we have in the implementation.

I don't think I understand this.  From my POV, restricting modules to
be called only from one thread is too restrictive, and I see no reason
for that.

> - The current check is easy to implement and understand. A check for multiple threads would require another
> hash table with thread IDs etc.

Why do you need a has table?  There's a linked list in all_threads
which holds all the known application threads, we just need a loop
over them.

> - The Emacs module API is modelled after JNI, which has the same restriction
> (http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html).

Themodule API is modelled after JNI, but the threads model is a far
cry from Java threads.  So I don't think this argument is relevant,
unless you can show specific problems with our implementation of
threads.

Modules were added to Emacs to allow easy interfaces to external
libraries.  It makes very little sense to me to arbitrarily restrict
the use of such external libraries in only some threads.  IMO, it's a
grave limitation.

Re: [PATCH] Support threads in modules

[John Wiegley]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> I don't think I understand this. From my POV, restricting modules to be
EZ> called only from one thread is too restrictive, and I see no reason for
EZ> that.

I see Eli's point here; I'm wondering Philipp, did you run into a particular
problem your patch is trying to solve, or are you trying to preempt future
problems?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [PATCH] Support threads in modules

[Stefan Monnier]

EZ> I don't think I understand this. From my POV, restricting modules to be
EZ> called only from one thread is too restrictive, and I see no reason for
EZ> that.
> I see Eli's point here; I'm wondering Philipp, did you run into a particular
> problem your patch is trying to solve, or are you trying to preempt future
> problems?

IIUC this doesn't restrict a module to be used with only one thread.
It just makes sure that the module can only call back Elisp from the
same thread that called it (and "called it" doesn't mean here just "some
time in the past" but "somewhere up the stack").

This makes sense since an "emacs_env" can only be used during the
current call (it's stack-allocated).


        Stefan

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 3449 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 22. Apr. 2017 um 22:13 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 22 Apr 2017 20:05:19 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
> >
> >  I agree that checking for the main thread is not TRT, but why not
> >  allow any thread of those in all_threads? Why do we care that the env
> >  pointer was created by the same thread as the one using it? We should
> >  only care that the invoking thread is one of the Emacs application
> >  threads, no?
> >
> > - Using objects across threads requires careful synchronization.
>
> Not sure what synchronization you have in mind.  There's only one
> running thread at any given time, and a thread cannot be preempted
> while it runs Lisp, so synchronization seems unnecessary, as a thread
> cannot modify a Lisp object that another thread is modifying.
>

However, that's just an implementation detail, and the mapping between
Emacs and OS threads is unspecified (for good reason). To be
forward-compatible, either we have to formally document this and then never
ever change the implementation, or document that module authors can't rely
on it and have to perform synchronization themselves.


>
> > - The more restrictive the module API is, the more freedom we have in
> the implementation.
>
> I don't think I understand this.  From my POV, restricting modules to
> be called only from one thread is too restrictive, and I see no reason
> for that.
>

I meant this as a general statement: if an API is more restrictive (i.e.
has stronger preconditions), its implementation can be more flexible
because it has to deal with fewer corner cases.
Specifically, the modules API is definitely not intended to restrict
modules to be single-threaded. It only restricts under which circumstances
environments can be used: the lifetime is restricted to the storage
duration of the environment pointer, and this patch adds an additional
restriction that environments are thread-affine.


>
> > - The current check is easy to implement and understand. A check for
> multiple threads would require another
> > hash table with thread IDs etc.
>
> Why do you need a has table?  There's a linked list in all_threads
> which holds all the known application threads, we just need a loop
> over them.
>

I think I misunderstood what would be needed to implement this check; in
fact, it's already implemented in this patch:
  assert (current_thread->thread_id == pthread_self());
Note that it's not enough to check whether the current OS thread matches
*some* Emacs thread, it has to be *the current* Emacs thread, otherwise the
interpreter state is inconsistent.


>
> > - The Emacs module API is modelled after JNI, which has the same
> restriction
> > (
> http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html
> ).
>
> Themodule API is modelled after JNI, but the threads model is a far
> cry from Java threads.  So I don't think this argument is relevant,
> unless you can show specific problems with our implementation of
> threads.
>

It's generally not possible to show specific problems because there is not
enough experience with the combination of multithreading and modules. Also,
thread synchronization problems (data races) typically lead to subtle
undefined behavior, which isn't easily observed.
By contrast, there's lots of experience with JNI, and it makes sense to
base a new design on an existing design.

[-- Attachment #2: Type: text/html, Size: 4702 bytes --]

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

John Wiegley <jwiegley@gmail.com> schrieb am So., 23. Apr. 2017 um
08:14 Uhr:

> >>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
>
> EZ> I don't think I understand this. From my POV, restricting modules to be
> EZ> called only from one thread is too restrictive, and I see no reason for
> EZ> that.
>
> I see Eli's point here; I'm wondering Philipp, did you run into a
> particular
> problem your patch is trying to solve, or are you trying to preempt future
> problems?
>
>
The particular problems I'm trying to solve are:
- With multithreading, the current thread check (that only checks for the
main thread) is clearly incorrect and I wanted to fix it.
- I finally wanted to start writing some documentation about the module
API. For that, I need to know both its desired and actual behavior,
including preconditions, guarantees, and restrictions.

[-- Attachment #2: Type: text/html, Size: 1279 bytes --]

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sun, 23 Apr 2017 08:40:31 -0400
> 
> EZ> I don't think I understand this. From my POV, restricting modules to be
> EZ> called only from one thread is too restrictive, and I see no reason for
> EZ> that.
> > I see Eli's point here; I'm wondering Philipp, did you run into a particular
> > problem your patch is trying to solve, or are you trying to preempt future
> > problems?
> 
> IIUC this doesn't restrict a module to be used with only one thread.
> It just makes sure that the module can only call back Elisp from the
> same thread that called it (and "called it" doesn't mean here just "some
> time in the past" but "somewhere up the stack").

Isn't that guaranteed?  IOW, what would be a scenario where in the
situation you described we will be in a different thread?

Perhaps we should make a step back and ask ourselves what is the
purpose of this check.  AFAIU, its original purpose was to make sure
module functions are called only from the Lisp thread, not from some
thread started by some linked-in library.

Nowadays, we can have more than one Lisp thread, but the analogous
check still makes sense, IMO.  So how about this much simpler
implementation:

  static void
  check_thread (void)
  {
  #ifdef THREADS_ENABLED
    if (current_thread)
      {
  # if defined (HAVE_PTHREAD)
        eassert (pthread_equal (pthread_self (), current_thread->thread_id));
  # elif defined (WINDOWSNT)
        eassert (GetCurrentThreadId () == current_thread->thread_id);
  # endif
      }
  #endif
  }

This fixes a few minor problems with the original proposal:

 . it works in an Emacs built without threads
 . it does NOT require current_thread to be non-NULL, as this can
   legitimately happen during short periods of time (see thread.c), and
 . it avoids recording some arbitrary thread in the env struct

The main idea of the check is still there, and will do its job.

Comments?

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 23 Apr 2017 15:52:36 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
>  > - Using objects across threads requires careful synchronization.
> 
>  Not sure what synchronization you have in mind. There's only one
>  running thread at any given time, and a thread cannot be preempted
>  while it runs Lisp, so synchronization seems unnecessary, as a thread
>  cannot modify a Lisp object that another thread is modifying.
> 
> However, that's just an implementation detail, and the mapping between Emacs and OS threads is
> unspecified (for good reason).

It isn't unspecified: the ELisp manual explicitly documents that only
one Lisp thread can run at any given time.  If we ever change that, it
will be a very large job, and the result will be a very different
Emacs.

> To be forward-compatible, either we have to formally document this and then
> never ever change the implementation, or document that module authors can't rely on it and have to perform
> synchronization themselves.

Since it is documented already, module authors can rely on that.

> I meant this as a general statement: if an API is more restrictive (i.e. has stronger preconditions), its
> implementation can be more flexible because it has to deal with fewer corner cases.

I don't think it's right for us to favor our flexibility over that of
the Lisp programmers who will want to use these features.

> Specifically, the modules API is definitely not intended to restrict modules to be single-threaded. It only
> restricts under which circumstances environments can be used: the lifetime is restricted to the storage
> duration of the environment pointer, and this patch adds an additional restriction that environments are
> thread-affine.

I think we only care that module functions run in the context of some
Lisp thread, and we don't care which Lisp thread is that.  So I
proposed a simplified implementation of the test, which I hope you
will agree with.

> Note that it's not enough to check whether the current OS thread matches *some* Emacs thread, it has to be
> *the current* Emacs thread, otherwise the interpreter state is inconsistent.

Given the documented implementation and restrictions on Lisp threads,
the condition you want to enforce is guaranteed, as long as the
current thread is some Lisp thread.

>  > - The Emacs module API is modelled after JNI, which has the same restriction
>  > (http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html).
> 
>  Themodule API is modelled after JNI, but the threads model is a far
>  cry from Java threads. So I don't think this argument is relevant,
>  unless you can show specific problems with our implementation of
>  threads.
> 
> It's generally not possible to show specific problems because there is not enough experience with the
> combination of multithreading and modules. Also, thread synchronization problems (data races) typically lead
> to subtle undefined behavior, which isn't easily observed.
> By contrast, there's lots of experience with JNI, and it makes sense to base a new design on an existing
> design.

Once again, I think that the JNI experience related to threads is not
relevant for Emacs, due to a very restrictive form of threading we
allow.

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 5142 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 26. Apr. 2017 um 07:32 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sun, 23 Apr 2017 15:52:36 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
> >
> >  > - Using objects across threads requires careful synchronization.
> >
> >  Not sure what synchronization you have in mind. There's only one
> >  running thread at any given time, and a thread cannot be preempted
> >  while it runs Lisp, so synchronization seems unnecessary, as a thread
> >  cannot modify a Lisp object that another thread is modifying.
> >
> > However, that's just an implementation detail, and the mapping between
> Emacs and OS threads is
> > unspecified (for good reason).
>
> It isn't unspecified: the ELisp manual explicitly documents that only
> one Lisp thread can run at any given time.


That is true for a *Lisp* thread, not for an *OS* thread. On the OS thread
level we might theoretically have multiple threads per Lisp thread, or miss
some synchronization barriers, etc.


>
> > To be forward-compatible, either we have to formally document this and
> then
> > never ever change the implementation, or document that module authors
> can't rely on it and have to perform
> > synchronization themselves.
>
> Since it is documented already, module authors can rely on that.
>

It's not documented because it can't be documented without talking about
the behavior of the C module API, and that is itself not documented yet. If
we choose to guarantee that unsynchronized accesses to module functions
don't introduce data races, then we need to say that explicitly in the
module documentation, and stick to it. (I.e. once that decision has been
made, there's no going back.)


>
> > I meant this as a general statement: if an API is more restrictive (i.e.
> has stronger preconditions), its
> > implementation can be more flexible because it has to deal with fewer
> corner cases.
>
> I don't think it's right for us to favor our flexibility over that of
> the Lisp programmers who will want to use these features.
>

That's a matter for debate. Wide contracts, such as guaranteeing some
behavior, are undoubtedly great for clients, but on the other hand are not
as future-proof as narrow contracts, because it often turns out that the
wide contract guarantees some behavior that is considered undesirable with
more experience (e.g. signed integer overflow in Java vs. C).


>
> > Specifically, the modules API is definitely not intended to restrict
> modules to be single-threaded. It only
> > restricts under which circumstances environments can be used: the
> lifetime is restricted to the storage
> > duration of the environment pointer, and this patch adds an additional
> restriction that environments are
> > thread-affine.
>
> I think we only care that module functions run in the context of some
> Lisp thread, and we don't care which Lisp thread is that.  So I
> proposed a simplified implementation of the test, which I hope you
> will agree with.
>

If we want to guarantee that environment functions can be called from
arbitrary threads without introducing data races, then the only assertion
that's necessary is whether (current_thread->thread_id ==
GetCurrentThreadId ()); without that undefined behavior would be almost
guaranteed.


>
> > Note that it's not enough to check whether the current OS thread matches
> *some* Emacs thread, it has to be
> > *the current* Emacs thread, otherwise the interpreter state is
> inconsistent.
>
> Given the documented implementation and restrictions on Lisp threads,
> the condition you want to enforce is guaranteed, as long as the
> current thread is some Lisp thread.
>

If only the current thread can be scheduled, then these conditions should
indeed be equivalent. However, checking whether the current thread is the
current Emacs thread is much simpler to check (a single equality test).


>
> >  > - The Emacs module API is modelled after JNI, which has the same
> restriction
> >  > (
> http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html
> ).
> >
> >  Themodule API is modelled after JNI, but the threads model is a far
> >  cry from Java threads. So I don't think this argument is relevant,
> >  unless you can show specific problems with our implementation of
> >  threads.
> >
> > It's generally not possible to show specific problems because there is
> not enough experience with the
> > combination of multithreading and modules. Also, thread synchronization
> problems (data races) typically lead
> > to subtle undefined behavior, which isn't easily observed.
> > By contrast, there's lots of experience with JNI, and it makes sense to
> base a new design on an existing
> > design.
>
> Once again, I think that the JNI experience related to threads is not
> relevant for Emacs, due to a very restrictive form of threading we
> allow.
>

As said, if we make the guarantee that calling environment functions cannot
introduce data races, then yes, the thread affinity restriction is not
necessary. I'd just want to make sure that everyone understands the
advantages and disadvantages of both approaches.

[-- Attachment #2: Type: text/html, Size: 6814 bytes --]

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 10 Jun 2017 11:38:32 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
> Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 26. Apr. 2017 um 07:32 Uhr:

[It's very hard to have a discussion with 1.5 months between messages.]

> > >  > - Using objects across threads requires careful synchronization.
> > >
> > >  Not sure what synchronization you have in mind. There's only one
> > >  running thread at any given time, and a thread cannot be preempted
> > >  while it runs Lisp, so synchronization seems unnecessary, as a thread
> > >  cannot modify a Lisp object that another thread is modifying.
> > >
> > > However, that's just an implementation detail, and the mapping between
> > Emacs and OS threads is
> > > unspecified (for good reason).
> >
> > It isn't unspecified: the ELisp manual explicitly documents that only
> > one Lisp thread can run at any given time.
> 
> That is true for a *Lisp* thread, not for an *OS* thread. On the OS thread
> level we might theoretically have multiple threads per Lisp thread, or miss
> some synchronization barriers, etc.

You were AFAIU talking about accesses to Lisp objects, and these are
only possible via Lisp, which can only be run in a single thread at a
time.  So if the non-Lisp threads of any kind can enter this picture,
in a way that module functions will be run by those threads and access
Lisp objects, please describe such a use case, so we all are on the
same page regarding the situations we are considering.

> > > To be forward-compatible, either we have to formally document this and
> > then
> > > never ever change the implementation, or document that module authors
> > can't rely on it and have to perform
> > > synchronization themselves.
> >
> > Since it is documented already, module authors can rely on that.
> 
> It's not documented because it can't be documented without talking about
> the behavior of the C module API, and that is itself not documented yet.

If what you mean is that we don't say explicitly in the doc that only
one Lisp thread can be active at any given time, we can add that
statement right now.  Would that be enough to settle this particular
argument.

> If
> we choose to guarantee that unsynchronized accesses to module functions
> don't introduce data races, then we need to say that explicitly in the
> module documentation, and stick to it. (I.e. once that decision has been
> made, there's no going back.)

I think that ship sailed long ago: the current implementation assumes
implicitly and explicitly that at most only one Lisp thread runs at
any given time.  If we will ever want to have an implementation that
violates this constraint, we will have to completely rewrite
everything that is related to threads, including emacs-modules.c.

> > > I meant this as a general statement: if an API is more restrictive (i.e.
> > has stronger preconditions), its
> > > implementation can be more flexible because it has to deal with fewer
> > corner cases.
> >
> > I don't think it's right for us to favor our flexibility over that of
> > the Lisp programmers who will want to use these features.
> 
> That's a matter for debate.

I'm saying that as long as this is debatable, we shouldn't make life
for Lisp programmers so much harder based on assumptions that are not
universally approved by the project as a whole.  My opinion is that
the Lisp programmers' flexibility should be preferable to that of
developers.

> > I think we only care that module functions run in the context of some
> > Lisp thread, and we don't care which Lisp thread is that.  So I
> > proposed a simplified implementation of the test, which I hope you
> > will agree with.
> 
> If we want to guarantee that environment functions can be called from
> arbitrary threads without introducing data races, then the only assertion
> that's necessary is whether (current_thread->thread_id ==
> GetCurrentThreadId ()); without that undefined behavior would be almost
> guaranteed.

Sorry, I don't understand what you are saying here.  Especially since
your proposed condition is almost the one I proposed in

  http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00720.html

Does this mean that you agree with my reasoning and the code I
proposed there?

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.

> > > Note that it's not enough to check whether the current OS thread matches
> > *some* Emacs thread, it has to be
> > > *the current* Emacs thread, otherwise the interpreter state is
> > inconsistent.
> >
> > Given the documented implementation and restrictions on Lisp threads,
> > the condition you want to enforce is guaranteed, as long as the
> > current thread is some Lisp thread.
> 
> If only the current thread can be scheduled, then these conditions should
> indeed be equivalent.

Given the current design, this is actually a tautology: the current
thread is _by_definition_ the one that is currently scheduled.

> However, checking whether the current thread is the
> current Emacs thread is much simpler to check (a single equality test).

No, it isn't; see above for an important caveat.

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 6646 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 10. Juni 2017 um 14:38 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 10 Jun 2017 11:38:32 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
> >
> > Eli Zaretskii <eliz@gnu.org> schrieb am Mi., 26. Apr. 2017 um 07:32 Uhr:
>
> [It's very hard to have a discussion with 1.5 months between messages.]
>

Yes, sorry for the huge delay, I wanted to follow up here for a while :-/


>
> > > >  > - Using objects across threads requires careful synchronization.
> > > >
> > > >  Not sure what synchronization you have in mind. There's only one
> > > >  running thread at any given time, and a thread cannot be preempted
> > > >  while it runs Lisp, so synchronization seems unnecessary, as a
> thread
> > > >  cannot modify a Lisp object that another thread is modifying.
> > > >
> > > > However, that's just an implementation detail, and the mapping
> between
> > > Emacs and OS threads is
> > > > unspecified (for good reason).
> > >
> > > It isn't unspecified: the ELisp manual explicitly documents that only
> > > one Lisp thread can run at any given time.
> >
> > That is true for a *Lisp* thread, not for an *OS* thread. On the OS
> thread
> > level we might theoretically have multiple threads per Lisp thread, or
> miss
> > some synchronization barriers, etc.
>
> You were AFAIU talking about accesses to Lisp objects, and these are
> only possible via Lisp, which can only be run in a single thread at a
> time.  So if the non-Lisp threads of any kind can enter this picture,
> in a way that module functions will be run by those threads and access
> Lisp objects, please describe such a use case, so we all are on the
> same page regarding the situations we are considering.
>

Such a use case is what these assertions should guard against. Calling
environment functions from non-Lisp threads is undefined behavior, and
these assertions are meant to protect module developers against them.


>
> > > > To be forward-compatible, either we have to formally document this
> and
> > > then
> > > > never ever change the implementation, or document that module authors
> > > can't rely on it and have to perform
> > > > synchronization themselves.
> > >
> > > Since it is documented already, module authors can rely on that.
> >
> > It's not documented because it can't be documented without talking about
> > the behavior of the C module API, and that is itself not documented yet.
>
> If what you mean is that we don't say explicitly in the doc that only
> one Lisp thread can be active at any given time, we can add that
> statement right now.  Would that be enough to settle this particular
> argument.
>

There is no actual argument :-)
It's just a matter of how much documentation we need. I think the
guaranteed synchronization should be documented in the to-be-written
modules manual as well, if we choose to make the guarantee.


>
> > If
> > we choose to guarantee that unsynchronized accesses to module functions
> > don't introduce data races, then we need to say that explicitly in the
> > module documentation, and stick to it. (I.e. once that decision has been
> > made, there's no going back.)
>
> I think that ship sailed long ago: the current implementation assumes
> implicitly and explicitly that at most only one Lisp thread runs at
> any given time.  If we will ever want to have an implementation that
> violates this constraint, we will have to completely rewrite
> everything that is related to threads, including emacs-modules.c.
>

Yes, but if we do decide to rewrite, then the new code might need
additional synchronization if we guarantee thread safety which would
otherwise not be required.


>
> > > > I meant this as a general statement: if an API is more restrictive
> (i.e.
> > > has stronger preconditions), its
> > > > implementation can be more flexible because it has to deal with fewer
> > > corner cases.
> > >
> > > I don't think it's right for us to favor our flexibility over that of
> > > the Lisp programmers who will want to use these features.
> >
> > That's a matter for debate.
>
> I'm saying that as long as this is debatable, we shouldn't make life
> for Lisp programmers so much harder based on assumptions that are not
> universally approved by the project as a whole.  My opinion is that
> the Lisp programmers' flexibility should be preferable to that of
> developers.
>

The two options are:
1. Guarantee thread safety of module objects.
2. Don't guarantee thread safety of module objects.
You clearly advocate for (1), which is fine. What I'm trying to point out
is that if we picked (2) now, we can easily switch to (1) later, but not
the other way round. Once we decide to do (1), there's no way to go to (2),
even if in some future implementation (2) might become simpler or more
efficient. That's the tradeoff.


>
> > > I think we only care that module functions run in the context of some
> > > Lisp thread, and we don't care which Lisp thread is that.  So I
> > > proposed a simplified implementation of the test, which I hope you
> > > will agree with.
> >
> > If we want to guarantee that environment functions can be called from
> > arbitrary threads without introducing data races, then the only assertion
> > that's necessary is whether (current_thread->thread_id ==
> > GetCurrentThreadId ()); without that undefined behavior would be almost
> > guaranteed.
>
> Sorry, I don't understand what you are saying here.  Especially since
> your proposed condition is almost the one I proposed in
>
>   http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00720.html
>
> Does this mean that you agree with my reasoning and the code I
> proposed there?
>

Yes, almost, if we pick option (1) now.


>
> 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.

[-- Attachment #2: Type: text/html, Size: 8954 bytes --]

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- 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

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 11 Jun 2017 13:25:54 +0000
> Cc: emacs-devel@gnu.org, phst@google.com
> 
> Here's a new patch, which implements option (1). 

Thanks, this LGTM.

Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 10 Jun 2017 19:58:40 +0000
> Cc: emacs-devel@gnu.org, phst@google.com

There's no point in arguing further, as you've already submitted a
patch that I agree with.  But just for the record, a couple more
comments to clarify the issue.

> > You were AFAIU talking about accesses to Lisp objects, and these are
> > only possible via Lisp, which can only be run in a single thread at a
> > time.  So if the non-Lisp threads of any kind can enter this picture,
> > in a way that module functions will be run by those threads and access
> > Lisp objects, please describe such a use case, so we all are on the
> > same page regarding the situations we are considering.
> 
> Such a use case is what these assertions should guard against. Calling
> environment functions from non-Lisp threads is undefined behavior, and
> these assertions are meant to protect module developers against them.

Right.  But these assertions should IMO not by themselves invoke
undefined behavior, which dereferencing a NULL pointer does.

> > 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.

Right.  And that's why dereferencing a potentially NULL pointer is IMO
something we should avoid doing.

I also think that we should try replacing eassert in this case with a
function that doesn't crash Emacs, only errors out of the offending
module, since a faulty module ideally shouldn't crash the entire Emacs
session.  But that might be hard to accomplish.  In any case, note
that eassert compiles to nothing in the production build.

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am So., 11. Juni 2017 um 16:54 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sun, 11 Jun 2017 13:25:54 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
> >
> > Here's a new patch, which implements option (1).
>
> Thanks, this LGTM.
>

Pushed as 8160c7d914.

[-- Attachment #2: Type: text/html, Size: 808 bytes --]

Re: [PATCH] Support threads in modules

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am So., 11. Juni 2017 um 17:01 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 10 Jun 2017 19:58:40 +0000
> > Cc: emacs-devel@gnu.org, phst@google.com
>
> There's no point in arguing further, as you've already submitted a
> patch that I agree with.  But just for the record, a couple more
> comments to clarify the issue.
>

Thanks. Please note there's no real argument, just a matter of explicitly
making some decisions and documenting them.


>
> > > You were AFAIU talking about accesses to Lisp objects, and these are
> > > only possible via Lisp, which can only be run in a single thread at a
> > > time.  So if the non-Lisp threads of any kind can enter this picture,
> > > in a way that module functions will be run by those threads and access
> > > Lisp objects, please describe such a use case, so we all are on the
> > > same page regarding the situations we are considering.
> >
> > Such a use case is what these assertions should guard against. Calling
> > environment functions from non-Lisp threads is undefined behavior, and
> > these assertions are meant to protect module developers against them.
>
> Right.  But these assertions should IMO not by themselves invoke
> undefined behavior, which dereferencing a NULL pointer does.
>

Yes, as discussed in the other thread these shouldn't be easserts.


>
> > > 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.
>
> Right.  And that's why dereferencing a potentially NULL pointer is IMO
> something we should avoid doing.
>

Yes, but in general we can't avoid that. It's a module bug, but we can't
protect against all module bugs. The assertions are there for module
developers to catch these bugs before releasing a module.


>
> I also think that we should try replacing eassert in this case with a
> function that doesn't crash Emacs, only errors out of the offending
> module, since a faulty module ideally shouldn't crash the entire Emacs
> session.  But that might be hard to accomplish.  In any case, note
> that eassert compiles to nothing in the production build.
>

Yes, that's why in the other thread I've implemented the module assertions,
which are enabled on the command line and reliably crash Emacs.
Other ways of error reporting without crashing are unfortunately not
possible without major changes to the module API.

[-- Attachment #2: Type: text/html, Size: 4520 bytes --]