bug#60144: 30.0.50; PGTK Emacs crashes after signal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: karl@karlotness.com,  60144@debbugs.gnu.org
> Date: Sun, 18 Dec 2022 21:45:38 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> That code has problems signalling errors, unless it is okay for
> >> unblock_input to signal.
> >
> > I don't understand this part.  Why and how is unblock_input part of
> > the picture?
> 
> Because unblock_input can call process_pending_signals, and in doing so
> handle_async_input, which calls gobble_input (and thus the
> read_socket_hook.)  As a result, it is not safe for any read_socket_hook
> to signal as long as it is not ok for unblock_input to signal as well.

But AFAIK it is _always_ safe for unblock_input to signal.  When do
you think it isn't?

> > So in the X/GTK build we have the same problem as with PGTK?  If so,
> > why not change that as well, to work as I described, i.e. enqueue
> > events to our own event queue, which we will then read and process in
> > safe context?
> >
> > AFAIU, w32 already works like that.  Does it not?
> 
> It doesn't, see how w32_note_mouse_movement is called from
> w32_read_socket.

??? w32_read_socket runs in the Lisp (a.k.a. "main") thread.  So it is
safe for any code it calls it to signal errors and do anything else
it's safe to do for the Lisp machine.

> > Yes, understood.  But it just tells me that we need to change the
> > architecture so that the events delivered by the window-system are not
> > processed in callbacks we install to be called by the window-system,
> > they should be processed in our own safe context.
> 
> The problem is note_mouse_highlight is simply not supposed to signal.

You cannot request that.  note_mouse_highlight examines properties,
and that can always signal, because properties are Lisp creatures and
can invoke Lisp.

> It is a function called directly while handling async input as far back
> as Emacs 19, much like expose_frame.  (IIRC back then there was a
> slightly different implementation in each of the *term.c files.)

We did a lot of dangerous stuff in Emacs 19, including (oh horror!)
reading input and doing redisplay inside signal handlers.  We
gradually removed all those unsafe parts, and nowadays we only do the
minimum in such contexts.  If unsafe processing of input is still with
us, we should move to safer techniques.  That this unsafe code exists
for such a long time is therefore not a justification for it to
continue existing.

Also, I think this unsafe processing of events only happens with
GTK/PGTK; other X configurations call XTread_socket and
handle_one_xevent from keyboard.c, in a "safe" context.

> Moving note_mouse_highlight out of handle_one_xevent would lead to other
> bugs, since mouse movement must be processed in order wrt to other X
> events.

I didn't say we shouldn't process mouse movements.  I said that this
processing should be limited to generating an Emacs input event and
queuing it, will all the pertinent information for further processing.
For example, note_mouse_highlight does just three things:

  . redisplays portion of the window in a special face
  . changes the way the cursor is drawn
  . shows help-echo

All of that can be done given an input event read by terminals
read_socket_hook inside keyboard.c, provided that the information
about the mouse move is stored in the input event.  There's
absolutely no reason to produce the above 3 effects right where we are
fed the raw X or GTK event from the window-system or the toolkit.

> For example, if an XI_Motion event arrives and is queued, and
> then a subsequent XI_Leave event arrives before that event has a chance
> to be processed ``in our own safe context'', note_mouse_highlight will
> be called after the mouse has left the frame, leading to stuck mouse
> highlight.

AFAIU, these two events will both be queued, and will both be
processed, so there will be not "suck mouse highlight".

So I still don't understand what is it that I'm missing that makes you
say this safe processing of window-system events is impossible.

Anyway, this bug report is not the proper place to discuss this.
Please start a discussion on emacs-devel, and let's pick this up
there.