[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Support threads in modules
From: |
Eli Zaretskii |
Subject: |
Re: [PATCH] Support threads in modules |
Date: |
Wed, 26 Apr 2017 08:23:27 +0300 |
> 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?
- [PATCH] Support threads in modules, Philipp Stephani, 2017/04/22
- Re: [PATCH] Support threads in modules, Eli Zaretskii, 2017/04/22
- Re: [PATCH] Support threads in modules, Philipp Stephani, 2017/04/22
- Re: [PATCH] Support threads in modules, Eli Zaretskii, 2017/04/22
- Re: [PATCH] Support threads in modules, Philipp Stephani, 2017/04/22
- Re: [PATCH] Support threads in modules, Eli Zaretskii, 2017/04/22
- Re: [PATCH] Support threads in modules, John Wiegley, 2017/04/23
- Re: [PATCH] Support threads in modules, Stefan Monnier, 2017/04/23
- Re: [PATCH] Support threads in modules,
Eli Zaretskii <=
- Re: [PATCH] Support threads in modules, Philipp Stephani, 2017/04/23
- Re: [PATCH] Support threads in modules, Philipp Stephani, 2017/04/23
- Re: [PATCH] Support threads in modules, Eli Zaretskii, 2017/04/26