From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani
> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 10 Jun 2017 19:58:40 +0000
> Cc: emacs-dev= el@gnu.org, phst@g= oogle.com
There's no point in arguing further, as you've already submitted a<= br> patch that I agree with.=C2=A0 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.=C2=A0 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 a= ccess
> > Lisp objects, please describe such a use case, so we all are on t= he
> > 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.<= br>
Right.=C2=A0 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= 39;t be easserts.=C2=A0
> > Once again: there _are_ legitimate situations in Emacs when for a=
> > short time current_thread is NULL.=C2=A0 If you assume that these=
> > situations don't happen, your code will sooner or later crash= and
> > burn.=C2=A0 I'm saying this because I once thought current_th= read should be
> > non-NULL at all times, and my code which assumed that did crash.<= br> > >
>
> I've reviewed the threads code, and I'm quite sure that curren= t_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 mod= ule).
> That's exactly one of the undefined behavior situations that the a= ssertions
> are meant to prevent.
Right.=C2=A0 And that's why dereferencing a potentially NULL pointer is= IMO
something we should avoid doing.Yes, b= ut in general we can't avoid that. It's a module bug, but we can= 9;t protect against all module bugs. The assertions are there for module de= velopers to catch these bugs before releasing a module.=C2=A0<= div>
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<= br> session.=C2=A0 But that might be hard to accomplish.=C2=A0 In any case, not= e
that eassert compiles to nothing in the production build.Yes, that's why in the other thread I've impleme= nted the module assertions, which are enabled on the command line and relia= bly crash Emacs.Other ways of error reporting without crashing a= re unfortunately not possible without major changes to the module API.=C2= =A0