Re: bug#60144: 30.0.50; PGTK Emacs crashes after signal

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 19 Dec 2022 09:56:40 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > But AFAIK it is _always_ safe for unblock_input to signal.  When do
> > you think it isn't?
> 
> In almost all of the helper functions where it is called:
> tear_down_x_back_buffer, set_up_x_back_buffer, et cetera.

I don't think I understand why.  Let's take tear_down_x_back_buffer,
for example: why isn't it safe to call unblock_input from it?
tear_down_x_back_buffer is called:

  . from x_set_inhibit_double_buffering, which is a frame parameter
    handler, so it is only called from the Lisp thread
  . from x_free_frame_resources, which AFAIU is only called from the
    Lisp thread, when we delete a frame

So what is the problem here? what am I missing?

> Otherwise, why does it not call maybe_quit after reading pending events?
> If it's safe to signal from unblock_input, then there should be no point
> in delaying quitting until the next time maybe_quit is called?

Sorry, I don't see the relevance.  Even if we don't immediately call
maybe_quit, it doesn't mean we couldn't if we wanted or needed to.  It
isn't evidence that unblock_input cannot signal or is more generally
unsafe to call.

> > ??? 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.
> 
> It can run from unblock_input.

But unblock_input is only called from the Lisp thread, at least in the
w32 build.  And it should only be called from the Lisp thread.  It
cannot be safely called from any other thread.

> Basically any code where it is unsafe to
> signal (and there is a lot) calling unblock_input is put at risk.  Think
> of all the callers of xmalloc without input blocked.

Why would that be the problem?  Did you read the commentary around
line 710 in alloc.c?

> > You cannot request that.  note_mouse_highlight examines properties,
> > and that can always signal, because properties are Lisp creatures and
> > can invoke Lisp.
> 
> Could you please explain how?

Which part is unclear?

> > 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.
> 
> Non-local exits from handle_one_xevent are simply not safe.  That
> function is not only called from XTread_socket, it is also called from
> x_dispatch_event, which is called from various toolkit menus, including
> inside the menu widget's own event loop.

These calls will all have to go, then.  Maybe we should use some
simplified, stripped down version of handle_one_xevent, which doesn't
invoke processing that can access Lisp data and the global state.  Why
would a toolkit menu widget need to call note_mouse_highlight, which
is _our_ function that implements _our_ features, about which GTK (or
any other toolkit) knows absolutely nothing??

The fact that we call handle_one_xevent from places other than the
read_socket_hook is IMO a grave mistake, and we should strive to fix
as many of such mistakes as possible.  Because making these calls safe
is impossible, and by allowing them, perhaps even increasing their
numbers as time goes by, we make Emacs more and more fragile on the
platforms where we do that.

> > 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.
> 
> How would working with the mouse highlight work inside a popup menu,
> then?  Keyboard events are not being read inside popup_get_selection.

If it's impossible to do safely, it will not work.  It already doesn't
work in popup menus on w32, for similar reasons.  That's a minor
inconvenience to users, and a much smaller catastrophe than making
these unsafe calls.

> > AFAIU, these two events will both be queued, and will both be
> > processed, so there will be not "suck mouse highlight".
> 
> No, clear_mouse_face is absolutely safe to call synchronously, from the
> likes of x_mouse_leave and the XI_Leave handlers.
> 
> In fact, it can be called from places where it is impossible to handle
> keyboard events: popup dialogs and menus.

I don't see how this is related, sorry.  You asked:

> 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.

And I responded saying that there should be no "mouse stuck" because
both XI_Motion and XI_Leave will be queued and processed in the order
they arrived.  What does this have to do with the safety of calling
clear_mouse_face, or lack thereof?

I'm saying that there's no need to process these events immediately,
when the event calls into the Lisp machine.  We can, and in many cases
already do, queue the event and process it a bit later, and in a
different, safer, context.

> > 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.
> 
> Just that I'm not confident in that.  I can't explain why but the mouse
> highlight code is already tricky now, and something tells me moving it
> to keyboard.c will make it even trickier.

But it's already "in keyboard.c"!  See the part that begins with

  /* Try generating a mouse motion event.  */
  else if (some_mouse_moved ())

which ends with

      if (!NILP (x) && NILP (obj))
        obj = make_lispy_movement (f, bar_window, part, x, y, t);

Then we process these "lispy movements" as input.

> And besides, note_mouse_highlight should not signal, regardless of
> whether or not doing so is safe.

Not normally, no.  But if some Lisp program sets bad text properties
or overlays, it could signal.  We can never assume in Emacs that some
Lisp machine code never signals.