xnrealloc: How and when to properly use it.

xnrealloc: How and when to properly use it.

[Keith David Bershatsky]

[-- Attachment #1: Type: text/plain, Size: 4879 bytes --]

I am working on feature requests 22873 (multiple fake cursors) and 17684 (crosshairs that track the cursor position).

On a W32 build of Emacs (built with MinGW_32 and ezwinports downloaded a few years ago), my usage of xnrealloc (more often than not) causes Emacs to crash when _not_ running under gdb.  When running under gdb, however, everything seems to work well.  I do not experience any problems on the NS port of Emacs.  I have not done sufficient testing to know whether an X11 build of Emacs suffers from this issue.

I looked around for a core dump named emacs_backtrace.txt, but did not find it anywhere on the hard drive of the Windows XP SP-3 machine.

I tried following the instructions of the message in the quit dialog window that told me to launch gdb -p [Emacs PID], but I did not find anything that looked particularly helpful to my untrained eye.

At this point in time, I am assuming the following:  (1) I should not be using xnrealloc; or, (2) I am using xnrealloc incorrectly?

I envision a few scenarios where the window cache of fake cursors needs to be modified:

A.  If the window got deleted, then I need to loop through the live windows and free the memory allocation for the dead window cache of fake cursors.

B.  If the window still exists and the fake cursors feature is turned off, then do one of two things:  (1) free the memory for the cache entirely; or, (2) reduce the size of the cache to a size of just one.

C.  When the fake cursors feature is active, increase the size of the cache as needed.  When the cache needs to be reset, only reset the counter and leave the cache as-is (it will contain outdated information / garbage).  As far as I can tell, this is working well and needs no changes that I am aware of.

Attached is a patch.diff that applies to the master branch as of 04/08/2019 (a038df77de7b1aa2d73a6478493b8838b59e4982).  It is the latest version of the fake cursors features (containing several bug fixes), which has not yet been posted to the bug trackers for 22873 / 16784.

Below are simplified versions of the functions responsible to populate the cache and reset the cache.

static void
mc_populate_cache (struct window *w, enum type_of_cache cache_type)
{
  switch (cache_type)
    {
      case NO_CACHE:
        {
          return;
        }
      case MC_CACHE:
        {
          if (BUFFERP (w->contents)
              && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
            {
              int max_elts = 25000;
              /* Example assumes there is one (1) new elt each window update. */
              ++w->mc_elts.used;
              /* Maybe increase the size of the array. */
              if (w->mc_elts.allocated < w->mc_elts.used
                  && w->mc_elts.used < max_elts)
                {
                  int old_alloc = w->mc_elts.allocated;
                  int new_elts = w->mc_elts.used - w->mc_elts.allocated;
                  w->mc_elts.caches = xpalloc (w->mc_elts.caches, &w->mc_elts.allocated,
                                                new_elts, INT_MAX, sizeof *w->mc_elts.caches);
                  memset (w->mc_elts.caches + old_alloc, 0,
                           (w->mc_elts.allocated - old_alloc) * sizeof *w->mc_elts.caches);
                }
              int nth = w->mc_elts.used - 1;
              for (int elt = 0; elt < w->mc_elts.used; ++elt)
                {
                  if (elt == nth)
                    {
                      w->mc_elts.caches[nth].x = elt;
                      double red = elt; 
                      w->mc_elts.caches[elt].foreground.red = red;
                      fprintf (stderr, "w->mc_elts.used (%d) | w->mc_elts.allocated (%d)\n",
                                        w->mc_elts.used, w->mc_elts.allocated);
                    }
                }
            }
          break;
        }
      case CH_CACHE:
        {
          break;
        }
      case FC_CACHE:
        {
          break;
        }
    }
}

static void
mc_reset_cache (struct window *w, enum type_of_cache cache_type)
{
  switch (cache_type)
    {
      case NO_CACHE:
        {
          return;
        }
      case MC_CACHE:
        {
          if (BUFFERP (w->contents)
              && NILP (BVAR (XBUFFER (w->contents), crosshairs))
              && w->mc_elts.used > 1)
            {
              /* Decrease the size of the array to a bare minimum. */
              xnrealloc (w->mc_elts.caches, 1, sizeof *w->mc_elts.caches);
              w->mc_elts.used = 0;
              w->mc_elts.allocated = 1;
            }
            else if (BUFFERP (w->contents)
                     && !NILP (BVAR (XBUFFER (w->contents), crosshairs)))
              w->mc_elts.used = 0;
          break;
        }
      case CH_CACHE:
        {
          break;
        }
      case FC_CACHE:
        {
          break;
        }
    }
}


[-- Attachment #2: 2019_04_27__12_14_48_598.diff --]
[-- Type: application/diff, Size: 500424 bytes --]

Re: xnrealloc: How and when to properly use it.

[Paul Eggert]

Keith David Bershatsky wrote:
> On a W32 build of Emacs (built with MinGW_32 and ezwinports downloaded a few years ago), my usage of xnrealloc (more often than not) causes Emacs to crash when_not_  running under gdb.

Try the GDB command 'set disable-randomization off'. Of course this makes 
debugging more ... interesting.

+              /* Decrease the size of the array to a bare minimum. */
+              xnrealloc (w->mc_elts.caches, 1, sizeof *w->mc_elts.caches);

That can't be right; at the very least you need to assign the returned value of 
xnrealloc back to w->mc_elts.caches.

Re: xnrealloc: How and when to properly use it.

[Keith David Bershatsky]

Thank you, Paul, for reviewing and responding to this particular thread.

Based upon your suggestion, I set w->mc_elts.caches = xnrealloc ... and have thus far been unable to crash Emacs -- i.e., your suggestion appears to have resolved the issue.  I'll do some additional testing tomorrow and report back.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-27-2019 18:51:51] <27 Apr 2019 18:51:51 -0700>
> From: Paul Eggert <eggert@cs.ucla.edu>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Subject: Re: xnrealloc: How and when to properly use it.
> 
> Keith David Bershatsky wrote:
> > On a W32 build of Emacs (built with MinGW_32 and ezwinports downloaded a few years ago), my usage of xnrealloc (more often than not) causes Emacs to crash when_not_  running under gdb.
> 
> Try the GDB command 'set disable-randomization off'. Of course this makes
> debugging more ... interesting.
> 
> +              /* Decrease the size of the array to a bare minimum. */
> +              xnrealloc (w->mc_elts.caches, 1, sizeof *w->mc_elts.caches);
> 
> That can't be right; at the very least you need to assign the returned value of xnrealloc back to w->mc_elts.caches.

Re: xnrealloc: How and when to properly use it.

[Keith David Bershatsky]

I did some additional testing today and confirmed that assigning the returned value of xnrealloc back to w->mc_elts.caches has resolved the issue.

Thank you for the help -- greatly appreciated!

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [04-27-2019 18:51:51] <27 Apr 2019 18:51:51 -0700>
> From: Paul Eggert <eggert@cs.ucla.edu>
> To: Keith David Bershatsky <esq@lawlist.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Subject: Re: xnrealloc: How and when to properly use it.
> 
> Keith David Bershatsky wrote:
> > On a W32 build of Emacs (built with MinGW_32 and ezwinports downloaded a few years ago), my usage of xnrealloc (more often than not) causes Emacs to crash when_not_  running under gdb.
> 
> Try the GDB command 'set disable-randomization off'. Of course this makes
> debugging more ... interesting.
> 
> +              /* Decrease the size of the array to a bare minimum. */
> +              xnrealloc (w->mc_elts.caches, 1, sizeof *w->mc_elts.caches);
> 
> That can't be right; at the very least you need to assign the returned value of xnrealloc back to w->mc_elts.caches.