Re: [PATCH] Support threads in modules

[Eli Zaretskii]

> From: Stefan Monnier <address@hidden>
> 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?