Test whether buffer/window is ready for start_display, etc.

Test whether buffer/window is ready for start_display, etc.

[Keith David Bershatsky]

In developing implementation of my own feature requests to draw crosshairs (#17684) using multiple fake cursors (#22873), I have come across situations when Emacs is starting up and restoring buffers (custom version of desktop.el) such that several internal functions cause Emacs to crash -- presumably because the combination of buffer and window are not ready yet.  I am presently using the following five (5) tests to see whether window/buffer are ready for things like `start_display`, `move_it_to`, etc.  Is there a better test to see whether the buffer/window combination is ready for me to get to work?

  if (w != XWINDOW (selected_window))
    return;

  if (!WINDOW_VALID_P (selected_window))
    return;

  Lisp_Object buf = w->contents;
  CHECK_BUFFER (buf);
  struct buffer *b = XBUFFER (buf);

/* eassert (CHARPOS (pos) >= BEGV && CHARPOS (pos) <= ZV);
   eassert (charpos == BYTE_TO_CHAR (bytepos));
   eassert (BUF_BEG_BYTE (b) <= bytepos && bytepos <= BUF_Z_BYTE (b));
*/

  ptrdiff_t charpos = marker_position (w->start);
  ptrdiff_t bytepos = marker_byte_position (w->start);

  bool barf_crash_one = (charpos >= BEGV && charpos <= ZV) ? false : true;
  if (barf_crash_one)
    return;

  bool barf_crash_two = (charpos == BYTE_TO_CHAR (bytepos)) ? false : true;
  if (barf_crash_two)
    return;

  bool barf_crash_three = (BUF_BEG_BYTE (b) <= bytepos && bytepos <= BUF_Z_BYTE (b)) ? false : true;
  if (barf_crash_three)
    return;

Thanks,

Keith

Re: Test whether buffer/window is ready for start_display, etc.

[Eli Zaretskii]

> Date: Wed, 27 Sep 2017 19:21:17 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> In developing implementation of my own feature requests to draw crosshairs (#17684) using multiple fake cursors (#22873), I have come across situations when Emacs is starting up and restoring buffers (custom version of desktop.el) such that several internal functions cause Emacs to crash -- presumably because the combination of buffer and window are not ready yet.  I am presently using the following five (5) tests to see whether window/buffer are ready for things like `start_display`, `move_it_to`, etc.  Is there a better test to see whether the buffer/window combination is ready for me to get to work?

The answer is "always".  The crashes you see must be due to some other
factors, and you haven't told enough to guess what those factors might
be.  Showing a couple of backtraces from those crashes might fill in
those blanks.

>   if (w != XWINDOW (selected_window))
>     return;

This doesn't feel relevant.  You can always redisplay a window that is
not the selected window, provided that you take care to switch to its
buffer (redisplay does that already).

>   if (!WINDOW_VALID_P (selected_window))
>     return;

How did you get into this situation?  It shouldn't happen during
redisplay, AFAIR.

>   ptrdiff_t charpos = marker_position (w->start);
>   ptrdiff_t bytepos = marker_byte_position (w->start);
> 
>   bool barf_crash_one = (charpos >= BEGV && charpos <= ZV) ? false : true;
>   if (barf_crash_one)
>     return;

If window's start point is invalid, it means the window needs to be
redisplayed.  Again, I'm not sure I understand how do you get into
this situation in the middle of redisplay.

>   bool barf_crash_two = (charpos == BYTE_TO_CHAR (bytepos)) ? false : true;
>   if (barf_crash_two)
>     return;

Likewise.  Markers are updated when a buffer is modified, so this
should never happen.

>   bool barf_crash_three = (BUF_BEG_BYTE (b) <= bytepos && bytepos <= BUF_Z_BYTE (b)) ? false : true;
>   if (barf_crash_three)
>     return;

Since you already verified the same condition about the character
position, and you already verified that the character position and the
byte position are consistent, this condition is redundant.

Re: Test whether buffer/window is ready for start_display, etc.

[Keith David Bershatsky]

Thank you, Eli, for looking into this particular thread.

Crosshairs and multiple fake cursors are drawn/erased when the following functions are called, each depending upon the flavor of Emacs (NS, X11, W32).  This approach is based upon the existing method in which the real cursor is drawn.

* ns_update_window_end (nsterm.m)

* x_update_window_end (w32term.c)

* x_update_window_end (xterm.c)

Normally, redisplay gets to the above-functions when a window needs updating.  I want crosshairs and/or fake cursors to be drawn/erased even when there is no need to update the window for any other reason.  I throw the boolean switches for crosshairs and/or fake cursors a few different ways:  toggle window.h switches with a function called from Lisp (sometimes attached to an idle timer, sometimes not); or, a keyboard press.  The crosshairs and/or fake cursors also erase/draw if they were previously drawn on the window and the cache with their prior coordinates is still populated.

When no window updating is required, redisplay_window is *not* called.  The most common way for this to happen is an idle timer function being called.  In order to make sure that *_update_window_end gets called when redisplay_window is *not* involved, I put in a condition preventing redisplay_internal from using "goto end_of_redisplay".  In other words, if my boolean switches are "true", then do *not* "goto end_of_redisplay".

When loading buffers with a custom desktop restore feature while Emacs is starting up, it is possible that this would normally be a "goto end_of_redisplay" condition that I have preempted.  However, I would need to do more testing to see whether redisplay_window was called on the window where the crash occurred.

The conditions of (w != XWINDOW (selected_window)) and (!WINDOW_VALID_P (selected_window)) were "wild guesses" on my part in an effort to help avoid potential future crashes.

The conditions to see whether eassert would cause a crash in that situation (and immediately return if that would be the case) were implemented by me after three separate crashes and gdb told me said eassert conditions were the reasons:

(CHARPOS (pos) >= BEGV && CHARPOS (pos) <= ZV);

(charpos == BYTE_TO_CHAR (bytepos));

(BUF_BEG_BYTE (b) <= bytepos && bytepos <= BUF_Z_BYTE (b))

I have commented out the last check, which you indicated was redundant.  I am happy to do more work on troubleshooting, which would entail removing the conditions that have prevented crashes and then get the backtraces -- perhaps that would tell me whether redisplay_window got bypassed such that preemption of "goto end_of_redisplay" is the problem.

Re: Test whether buffer/window is ready for start_display, etc.

[Eli Zaretskii]

> Date:  Wed, 04 Oct 2017 19:35:13 -0700
> From:  Keith David Bershatsky <esq@lawlist.com>
> Cc:  emacs-devel@gnu.org
> 
> Crosshairs and multiple fake cursors are drawn/erased when the following functions are called, each depending upon the flavor of Emacs (NS, X11, W32).  This approach is based upon the existing method in which the real cursor is drawn.
> 
> * ns_update_window_end (nsterm.m)
> 
> * x_update_window_end (w32term.c)
> 
> * x_update_window_end (xterm.c)

This design is very problematic.  The "normal" cursor is drawn in
update_window functions because it only depends on the location of
point, so it doesn't need to be redrawn if the window needs no update.
But your fake cursors are different, AFAIU, and require redrawing even
when the window didn't change.  So your idea (AFAIU) of following the
footsteps of normal cursor drawing is not a good idea.

The only safe way of displaying such stuff is by making
redisplay_window aware of the changes in the window, by, for example,
moving some overlay to where the fake cursor is drawn.

With your current design, as I understand it, you will have many
problems down the line, because it simply doesn't fit into how the
display engine works.  So I think you should redesign your
implementation, to have the fake cursors updated as part of the normal
window update code in redisplay_window.