GNU bug report logs - #79306
Broken follow-mode

Previous Next

To reply to this bug, email your comments to 79306 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Broken follow-mode
Date: Mon, 25 Aug 2025 09:30:10 +0300

Some recent change in master broke display for follow-mode:

0. emacs -Q
1. visit a file in fundamental-mode, e.g.
   C-x C-f etc/COPYING RET
2. split horizontally
   C-x 3
3. M-x follow-mode RET
4. scroll down a few screens
5. type 'M-<' (beginning-of-buffer)

It displays the message "Back to top level",
but doesn't move to the beginning of the buffer.

Typing 'C-l' updates the screen, and reveals
that 'M-<' actually moved to the beginning of the buffer,
but didn't update the screen.

Message #8 received at 79306 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79306 <at> debbugs.gnu.org
Subject: Re: bug#79306: Broken follow-mode
Date: Mon, 25 Aug 2025 15:54:17 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Mon, 25 Aug 2025 09:30:10 +0300
> 
> Some recent change in master broke display for follow-mode:
> 
> 0. emacs -Q
> 1. visit a file in fundamental-mode, e.g.
>    C-x C-f etc/COPYING RET
> 2. split horizontally
>    C-x 3
> 3. M-x follow-mode RET
> 4. scroll down a few screens
> 5. type 'M-<' (beginning-of-buffer)
> 
> It displays the message "Back to top level",
> but doesn't move to the beginning of the buffer.

Looks like the trap set by Stefan a month ago finally caught its first
victim: follow-mode calls redisplay recursively, from
pre-redisplay-function:

  Thread 1 hit Breakpoint 4, redisplay_internal () at xdisp.c:17122
  17122     struct window *w = XWINDOW (selected_window);
  (gdb) bt
  #0  redisplay_internal () at xdisp.c:17122
  #1  0x0104197e in redisplay_preserve_echo_area (from_where=2) at xdisp.c:18040
  #2  0x00fe51f4 in Fredisplay (force=XIL(0)) at dispnew.c:7004
  #3  0x0123da19 in funcall_subr (subr=0x13ee240 <Sredisplay>, numargs=0,
      args=0xa019120) at eval.c:3242
  #4  0x012aa6a8 in exec_byte_code (fun=XIL(0xa0000000103368c0),
      args_template=514, nargs=2, args=0xa019128) at bytecode.c:805
  #5  0x0123e30c in funcall_lambda (fun=XIL(0xa000000010336cd0), nargs=1,
      arg_vector=0xa019050) at eval.c:3333
  #6  0x0123d3d2 in funcall_general (fun=XIL(0xa000000010336cd0), numargs=1,
      args=0xa019050) at eval.c:3125
  #7  0x0123d6fb in Ffuncall (nargs=2, args=0xa019048) at eval.c:3174
  #8  0x0123c472 in Fapply (nargs=2, args=0xa019048) at eval.c:2788
  #9  0x0123dea4 in funcall_subr (subr=0x13fc600 <Sapply>, numargs=2,
      args=0xa019048) at eval.c:3265
  #10 0x012aa6a8 in exec_byte_code (fun=XIL(0xa00000001033a090),
      args_template=128, nargs=1, args=0xbfdcd0) at bytecode.c:805
  #11 0x0123e30c in funcall_lambda (fun=XIL(0xa00000001033a090), nargs=1,
      arg_vector=0xbfdcd0) at eval.c:3333
  #12 0x0123d3d2 in funcall_general (fun=XIL(0xa00000001033a090), numargs=1,
      args=0xbfdcd0) at eval.c:3125
  #13 0x0123d6fb in Ffuncall (nargs=2, args=0xbfdcc8) at eval.c:3174
  #14 0x012386f0 in internal_condition_case_n (bfun=0x123d5af <Ffuncall>,
      nargs=2, args=0xbfdcc8, handlers=XIL(0x30),
      hfun=0x10151d3 <dsafe_eval_handler>) at eval.c:1770
  #15 0x01015307 in dsafe__call (inhibit_quit=true, f=0x123d5af <Ffuncall>,
      nargs=2, args=0xbfdcc8) at xdisp.c:3102
  #16 0x0103711d in prepare_menu_bars () at xdisp.c:14054
  #17 0x0103f95e in redisplay_internal () at xdisp.c:17283
  #18 0x0103e65d in redisplay () at xdisp.c:16844
  #19 0x011456ef in read_char (commandflag=1, map=XIL(0xc00000001711a630),
      prev_event=XIL(0), used_mouse_menu=0xbff31f, end_time=0x0)
      at keyboard.c:2672
  #20 0x0115ee45 in read_key_sequence (keybuf=0xbff5f8, prompt=XIL(0),
      dont_downcase_last=false, can_return_switch_frame=true,
      fix_current_buffer=true, prevent_redisplay=false,
      disable_text_conversion_p=false) at keyboard.c:11146
  #21 0x011412ca in command_loop_1 () at keyboard.c:1424
  #22 0x01238416 in internal_condition_case (bfun=0x1140c68 <command_loop_1>,
      handlers=XIL(0x90), hfun=0x113fcc1 <cmd_error>) at eval.c:1690
  #23 0x011406cd in command_loop_2 (handlers=XIL(0x90)) at keyboard.c:1163
  #24 0x012375a2 in internal_catch (tag=XIL(0x12a80),
      func=0x1140696 <command_loop_2>, arg=XIL(0x90)) at eval.c:1370
  #25 0x01140638 in command_loop () at keyboard.c:1141
  #26 0x0113f721 in recursive_edit_1 () at keyboard.c:749
  #27 0x0113f9bf in Frecursive_edit () at keyboard.c:832
  #28 0x0113a9aa in main (argc=2, argv=0xf02638) at emacs.c:2629

  Lisp Backtrace:
  "redisplay" (0xa019120)
  "follow-adjust-window" (0xa0190b8)
  "follow-post-command-hook" (0xa019080)
  "follow-pre-redisplay-function" (0xa019050)
  "apply" (0xa019048)
  0x1033a090 PVEC_CLOSURE
  "redisplay_internal (C function)" (0x0)

Then we catch this and throw to top-level:

      int redisplay_counter_before = redisplay_counter;
      /* Use Qt to ensure debugger does not run,
	 to reduce the risk of wanting to redisplay.  */
      val = internal_condition_case_n (f, nargs, args, Qt,
				       dsafe_eval_handler);
      if (redisplay_counter_before != redisplay_counter)
        /* A nested redisplay happened, abort this one!  */
        /* FIXME: Rather than jump all the way to `top-level`
           we should exit only the current redisplay.  */
        Ftop_level ();  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Is it allowed to perform redisplay from pre-redisplay-function?  If
so, we need to refine the above test, or maybe disable it when we call
pre-redisplay-function.

Message #11 received at 79306 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79306 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#79306: Broken follow-mode
Date: Thu, 04 Sep 2025 00:58:34 -0400

> Is it allowed to perform redisplay from pre-redisplay-function?

Yeah, I think `pre-redisplay-function` is still early enough that it
happens before we start messing with the state, so it's safe.  I guess
that means it can be called with a normal `call` rather than
`safe_call`.

> If so, we need to refine the above test, or maybe disable it when we
> call pre-redisplay-function.

Agreed.  Maybe with something like the patch below?


        Stefan


diff --git a/src/xdisp.c b/src/xdisp.c
index 89561d750b6..e486eb177c1 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -14059,7 +14059,8 @@ prepare_menu_bars (void)
 		windows = Fcons (this, windows);
 	    }
 	}
-      dsafe_calln (true, Vpre_redisplay_function, windows);
+      /* FIXME: Bind 'inhibit_quit'.  */
+      calln (true, Vpre_redisplay_function, windows);
     }
 
   /* Update all frame titles based on their buffer names, etc.  We do

Message #14 received at 79306 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79306 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#79306: Broken follow-mode
Date: Thu, 04 Sep 2025 09:15:51 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Juri Linkov <juri <at> linkov.net>,  79306 <at> debbugs.gnu.org
> Date: Thu, 04 Sep 2025 00:58:34 -0400
> 
> > Is it allowed to perform redisplay from pre-redisplay-function?
> 
> Yeah, I think `pre-redisplay-function` is still early enough that it
> happens before we start messing with the state, so it's safe.  I guess
> that means it can be called with a normal `call` rather than
> `safe_call`.
> 
> > If so, we need to refine the above test, or maybe disable it when we
> > call pre-redisplay-function.
> 
> Agreed.  Maybe with something like the patch below?

I guess so (but we need a comment there explaining the reason for not
calling dsafe_call).  And why didn't you bind inhibit_quit?

One other thing we should perhaps consider: before you introduced the
redisplay_counter thing, redisplay_internal would in this case return
immediately at the beginning:

  /* I don't think this happens but let's be paranoid.  In particular,
     this was observed happening when Emacs shuts down due to losing X
     connection, in which case accessing SELECTED_FRAME and the frame
     structure is likely to barf.  */
  if (redisplaying_p)
    return;

In case of follow-mode, this means the call to 'redisplay' in
follow-adjust-window would previously do nothing.

So the change you propose will avoid the top-level throw, but it might
produce behavior in follow-mode that is different from what we had in
Emacs 30.  I don't use follow-mode, so I don't know what that could
cause, but maybe someone who does could run with the patch and see if
anything unexpected happens?

Message #17 received at 79306 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79306 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#79306: Broken follow-mode
Date: Thu, 04 Sep 2025 18:41:21 -0400

> I guess so (but we need a comment there explaining the reason for not
> calling dsafe_call).  And why didn't you bind inhibit_quit?

I just wanted to express the intent.

> One other thing we should perhaps consider: before you introduced the
> redisplay_counter thing, redisplay_internal would in this case return
> immediately at the beginning:
>
>   /* I don't think this happens but let's be paranoid.  In particular,
>      this was observed happening when Emacs shuts down due to losing X
>      connection, in which case accessing SELECTED_FRAME and the frame
>      structure is likely to barf.  */
>   if (redisplaying_p)
>     return;

Oh, very good point.
So maybe we just need the last two hunks below and the not the first.


        Stefan


diff --git a/src/xdisp.c b/src/xdisp.c
index 89561d750b6..763d9902486 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -14059,7 +14059,15 @@ prepare_menu_bars (void)
 		windows = Fcons (this, windows);
 	    }
 	}
-      dsafe_calln (true, Vpre_redisplay_function, windows);
+      {
+	/* We used to use 'dsafe_call' here, but actually we have not touched
+	   the redisplay state yet, so it's still safe to exit non-locally.  */
+	specpdl_ref count = SPECPDL_INDEX ();
+	specbind (Qinhibit_quit, Qt);
+	specbind (Qinhibit_redisplay, Qt);
+	calln (Vpre_redisplay_function, windows);
+	unbind_to (count, val);
+      }
     }
 
   /* Update all frame titles based on their buffer names, etc.  We do
@@ -17137,8 +17145,6 @@ redisplay_internal (void)
   bool polling_stopped_here = false;
   Lisp_Object tail, frame;
 
-  redisplay_counter++;
-
   /* Set a limit to the number of retries we perform due to horizontal
      scrolling, this avoids getting stuck in an uninterruptible
      infinite loop (Bug #24633).  */
@@ -17197,6 +17203,8 @@ redisplay_internal (void)
     return;
 #endif
 
+  redisplay_counter++;
+
   /* Record a function that clears redisplaying_p
      when we leave this function.  */
   specpdl_ref count = SPECPDL_INDEX ();

Message #20 received at 79306 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 79306 <at> debbugs.gnu.org
Subject: Re: bug#79306: Broken follow-mode
Date: Fri, 05 Sep 2025 09:20:07 +0300

> So maybe we just need the last two hunks below and the not the first.

The first patch produced compilation warnings and crashed,
the first hunk of this patch even doesn't compile.
But with the last two hunks follow-mode works correctly as before.

Message #23 received at 79306 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79306 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#79306: Broken follow-mode
Date: Fri, 05 Sep 2025 09:45:30 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: juri <at> linkov.net,  79306 <at> debbugs.gnu.org
> Date: Thu, 04 Sep 2025 18:41:21 -0400
> 
> > I guess so (but we need a comment there explaining the reason for not
> > calling dsafe_call).  And why didn't you bind inhibit_quit?
> 
> I just wanted to express the intent.
> 
> > One other thing we should perhaps consider: before you introduced the
> > redisplay_counter thing, redisplay_internal would in this case return
> > immediately at the beginning:
> >
> >   /* I don't think this happens but let's be paranoid.  In particular,
> >      this was observed happening when Emacs shuts down due to losing X
> >      connection, in which case accessing SELECTED_FRAME and the frame
> >      structure is likely to barf.  */
> >   if (redisplaying_p)
> >     return;
> 
> Oh, very good point.
> So maybe we just need the last two hunks below and the not the first.

Sounds good, thanks.  Though if we do this, wouldn't it render the
entire machinery of redisplay_counter effectively disabled, since
redisplaying_p is set right after we increment redisplay_counter?

Juri, can you try those two hunks and see if there are any problems
left in follow-mode?

Message #28 received at 79306-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 79306-done <at> debbugs.gnu.org
Subject: Re: bug#79306: Broken follow-mode
Date: Fri, 05 Sep 2025 15:12:36 -0400

Eli said:
> Sounds good, thanks.  Though if we do this, wouldn't it render the
> entire machinery of redisplay_counter effectively disabled, since
> redisplaying_p is set right after we increment redisplay_counter?

No, `redisplaying_p` is reset to false in a few different cases.

[ With the `redisplay_counter` mechanism, we could actually be more
  liberal w.r.t allowing to reset `redisplaying_p` to false (e.g. we
  could probably expose it to ELisp).  ]

Juri said:
> But with the last two hunks follow-mode works correctly as before.

Thanks, pushed to `master`, closing.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.