[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: |
Sun, 11 Jun 2017 18:01:05 +0300 |
> From: Philipp Stephani <address@hidden>
> Date: Sat, 10 Jun 2017 19:58:40 +0000
> Cc: address@hidden, address@hidden
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.