GNU bug report logs - #77985
31.0.50; infinite loop in `make-frame-names-alist`

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 77985 in the body.
You can then email your comments to 77985 AT debbugs.gnu.org in the normal way.

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

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

From: Al Haji-Ali <abdo.haji.ali <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Tue, 22 Apr 2025 12:01:38 +0100

I am running into an infinite loop in the function `make-frame-names-alist` which is called from `select-frame-by-name`.

The infinite loop happens when a frame with the property `no-other-frame` is the currently selected one. Starting with `emacs -Q`, the following code demonstrates the issue:

(with-selected-frame (make-frame '((no-other-frame . t)))
    (make-frame-names-alist))

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, martin rudalics <rudalics <at> gmx.at>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sat, 26 Apr 2025 16:15:09 +0300

> From: Al Haji-Ali <abdo.haji.ali <at> gmail.com>
> Date: Tue, 22 Apr 2025 12:01:38 +0100
> 
> 
> I am running into an infinite loop in the function `make-frame-names-alist` which is called from `select-frame-by-name`.
> 
> The infinite loop happens when a frame with the property `no-other-frame` is the currently selected one. Starting with `emacs -Q`, the following code demonstrates the issue:
> 
> (with-selected-frame (make-frame '((no-other-frame . t)))
>     (make-frame-names-alist))

Martin, any suggestions?

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

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>, Al Haji-Ali <abdo.haji.ali <at> gmail.com>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sat, 26 Apr 2025 18:34:16 +0200

>> I am running into an infinite loop in the function `make-frame-names-alist` which is called from `select-frame-by-name`.
>>
>> The infinite loop happens when a frame with the property `no-other-frame` is the currently selected one. Starting with `emacs -Q`, the following code demonstrates the issue:
>>
>> (with-selected-frame (make-frame '((no-other-frame . t)))
>>      (make-frame-names-alist))
>
> Martin, any suggestions?

The same problem occurs with

(let ((frame (make-frame)))
  (make-frame-invisible frame)
  (with-selected-frame frame
    (make-frame-names-alist)))

Both can be stopped via C-g here.

(progn
  (set-frame-parameter nil 'no-other-frame t)
  (delete-other-frames))

is worse.  C-g doesn't help here.

Inherently, these are siblings of Bug#15025.  Suggestions are:

- 'delete-other-frames' and ‘make-frame-names-alist’ signal an error
  when the selected frame has a 'no-other-frame t parameter or is
  invisible.  Simple.

- Have t for the MINIFRAME argument of 'next-frame' cycle through all
  frames including invisible and 'no-other-frame frames.  Simple but
  this would mean that 'make-frame-names-alist' would return the names
  of invisible frames too (it could filter them out afterwards).

- Have 'make-frame-names-alist' and 'delete-other-frames' call
  'frame-list' and iterate over it.  More robust but invasive.

Whatever we do, we probably should say that

    (while (not (eq frame current-frame))
	(setq frame (next-frame frame 0)))

can loop.

martin

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: abdo.haji.ali <at> gmail.com, 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sat, 26 Apr 2025 19:41:51 +0300

> Date: Sat, 26 Apr 2025 18:34:16 +0200
> Cc: 77985 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> gmx.at>
> 
>  >> I am running into an infinite loop in the function `make-frame-names-alist` which is called from `select-frame-by-name`.
>  >>
>  >> The infinite loop happens when a frame with the property `no-other-frame` is the currently selected one. Starting with `emacs -Q`, the following code demonstrates the issue:
>  >>
>  >> (with-selected-frame (make-frame '((no-other-frame . t)))
>  >>      (make-frame-names-alist))
>  >
>  > Martin, any suggestions?
> 
> The same problem occurs with
> 
> (let ((frame (make-frame)))
>    (make-frame-invisible frame)
>    (with-selected-frame frame
>      (make-frame-names-alist)))
> 
> Both can be stopped via C-g here.
> 
> (progn
>    (set-frame-parameter nil 'no-other-frame t)
>    (delete-other-frames))
> 
> is worse.  C-g doesn't help here.
> 
> Inherently, these are siblings of Bug#15025.  Suggestions are:
> 
> - 'delete-other-frames' and ‘make-frame-names-alist’ signal an error
>    when the selected frame has a 'no-other-frame t parameter or is
>    invisible.  Simple.
> 
> - Have t for the MINIFRAME argument of 'next-frame' cycle through all
>    frames including invisible and 'no-other-frame frames.  Simple but
>    this would mean that 'make-frame-names-alist' would return the names
>    of invisible frames too (it could filter them out afterwards).
> 
> - Have 'make-frame-names-alist' and 'delete-other-frames' call
>    'frame-list' and iterate over it.  More robust but invasive.

What about detecting when we made a full circle through all the
candidate frames, and stopping at that point?  Is that feasible?

> Whatever we do, we probably should say that
> 
>      (while (not (eq frame current-frame))
> 	(setq frame (next-frame frame 0)))
> 
> can loop.

Sure, but here the loop is explicit in the looping program, so it
could prevent the infloop.

Thanks.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: abdo.haji.ali <at> gmail.com, 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sat, 26 Apr 2025 20:51:42 +0200

> What about detecting when we made a full circle through all the
> candidate frames, and stopping at that point?  Is that feasible?

Hard to tell.  next_frame itself already fails to do that.

  while (passed < 2)
    FOR_EACH_FRAME (tail, f)
      {
	if (passed)
	  {
	    f = candidate_frame (f, frame, minibuf);
	    if (!NILP (f))
	      return f;
	  }
	if (EQ (frame, f))
	  passed++;
      }
  return frame;

If candidate_frame returns nil for FRAME, f is never set to FRAME and
passed is never incremented.  This could be fixed as

--- a/src/frame.c
+++ b/src/frame.c
@@ -2205,7 +2205,7 @@ candidate_frame (Lisp_Object candidate, Lisp_Object frame, Lisp_Object minibuf)
 static Lisp_Object
 next_frame (Lisp_Object frame, Lisp_Object minibuf)
 {
-  Lisp_Object f, tail;
+  Lisp_Object f, f1, tail;
   int passed = 0;

   eassume (CONSP (Vframe_list));
@@ -2215,9 +2215,9 @@ next_frame (Lisp_Object frame, Lisp_Object minibuf)
       {
 	if (passed)
 	  {
-	    f = candidate_frame (f, frame, minibuf);
-	    if (!NILP (f))
-	      return f;
+	    f1 = candidate_frame (f, frame, minibuf);
+	    if (!NILP (f1))
+	      return f1;
 	  }
 	if (EQ (frame, f))
 	  passed++;

But 'make-frame-names-alist', since it never sees frame set to
current-frame here,

    (while (not (eq frame current-frame))
      (progn
	(push (cons (frame-parameter frame 'name) frame) falist)
	(setq frame (next-frame frame 0))))

endlessly pushes the other frame to falist.

martin

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

From: Al Haji-Ali <abdo.haji.ali <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, martin rudalics <rudalics <at> gmx.at>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sun, 27 Apr 2025 08:40:27 +0100

On 26/04/2025, Eli Zaretskii wrote:
>>      (while (not (eq frame current-frame))
>> 	(setq frame (next-frame frame 0)))
>> 
>> can loop.
>
> Sure, but here the loop is explicit in the looping program, so it
> could prevent the infloop.

Indeed, I thought a loop like this would work without changing the documented behaviour as far as I can see:

,----
| (let ((current-frame (selected-frame))
|       prev-frame
|       frame)
|   (while (and
|           (not (eq frame current-frame))
|           (not (eq frame prev-frame)))
|     (setq
|      prev-frame frame
|      frame (next-frame frame 0))))
`----


On 26/04/2025, martin rudalics wrote:
> - Have t for the MINIFRAME argument of 'next-frame' cycle through all
>    frames including invisible and 'no-other-frame frames.  Simple but
>    this would mean that 'make-frame-names-alist' would return the names
>    of invisible frames too (it could filter them out afterwards).

In my emacs, `make-frame-names-alist` passes 0 to next-frame, but `delete-other-frames` already passes t. 

-- Al

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

From: martin rudalics <rudalics <at> gmx.at>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sun, 27 Apr 2025 10:22:55 +0200

[Message part 1 (text/plain, inline)]

>> Sure, but here the loop is explicit in the looping program, so it
>> could prevent the infloop.
>
> Indeed, I thought a loop like this would work without changing the documented behaviour as far as I can see:
>
> ,----
> | (let ((current-frame (selected-frame))
> |       prev-frame
> |       frame)
> |   (while (and
> |           (not (eq frame current-frame))
> |           (not (eq frame prev-frame)))
> |     (setq
> |      prev-frame frame
> |      frame (next-frame frame 0))))
> `----

Whatever we do here we won't fix the principal problem with next_frame
namely that

(progn
  (set-frame-parameter nil 'no-other-frame t)
  (next-frame nil 0))

can loop forever.  I attach a patch that should fix the problems with
'make-frame-names-alist' and 'delete-other-frames' using 'frame-list' as
base.

martin

[frame-list-1.diff (text/x-patch, attachment)]

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

From: martin rudalics <rudalics <at> gmx.at>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sun, 27 Apr 2025 11:18:23 +0200

[Message part 1 (text/plain, inline)]

> Whatever we do here we won't fix the principal problem with next_frame
> namely that
>
> (progn
>    (set-frame-parameter nil 'no-other-frame t)
>    (next-frame nil 0))
>
> can loop forever.

The attached patch includes my earlier proposal for fixing this and also
reverts the return value of 'make-frame-names-alist'.  Please try it.

Thanks, martin

[frame-list-1.diff (text/x-patch, attachment)]

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

From: Al Haji-Ali <abdo.haji.ali <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Mon, 28 Apr 2025 12:04:09 +0100

On 27/04/2025, martin rudalics wrote:

> The attached patch includes my earlier proposal for fixing this and also
> reverts the return value of 'make-frame-names-alist'.  Please try it.

This patch passed all my, admittedly limited, test cases.

In the patch, `make-frame-names-alist` returns only frames on the same display. I don't know if this is a behaviour change or to make it consistent with `next-frame` (the doc of `next-frame` doesn't mention displays -- might worth clarifying there if this is the behaviour).

Another minor/unimportant point (feel free to ignore) is that the patch also allows `make-frame-names-alist` to accept a FRAME argument. I think it would be more natural to return frames on the same display as the passed FRAME rather than the selected one.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Tue, 29 Apr 2025 10:37:13 +0200

[Message part 1 (text/plain, inline)]

> This patch passed all my, admittedly limited, test cases.

Thanks for checking.

> In the patch, `make-frame-names-alist` returns only frames on the same
> display. I don't know if this is a behaviour change or to make it
> consistent with `next-frame` (the doc of `next-frame` doesn't mention
> displays -- might worth clarifying there if this is the behaviour).

Thanks for spotting this.  'make-frame-names-alist' didn't have a
doc-string.  It only contained the comment

  ;; Only consider the frames on the same display.

so I thought that was the intended behavior.  In fact it wasn't so I
changed that now.

> Another minor/unimportant point (feel free to ignore) is that the
> patch also allows `make-frame-names-alist` to accept a FRAME
> argument. I think it would be more natural to return frames on the
> same display as the passed FRAME rather than the selected one.

I added the "(&optional frame)" because with a few exceptions most
frame-related functions have it so coders don't have to think twice when
calling any of these functions.

I attach a new version which also implements 'frame-list-1' in a
different way.  It's illustrative to look into how 'frame-list' and
'frame-list-1' relate.  With the following

(defun make-frames (number)
  (let ((i 0))
    (set-frame-parameter nil 'name (number-to-string (setq i (1+ i))))
    (while (< i number)
      (set-frame-parameter
       (make-frame) 'name (number-to-string (setq i (1+ i)))))))

make four frames via

(make-frames 4)

with titles "1" to "4".  Now when evaluating

(frame-list)
(frame-list-1)

the result for the latter will change when you switch to another frame
while the former remains unaltered.  I have no idea if it's that what we
want for 'make-frame-names-alist' but 'frame-list-1' should preserve the
old behavior.

Note here that we always list frames in the reverse order of their
creation.  This means that with respect to the selected frame,
'next-frame' conceptually returns the frame created previously and
'previous-frame' the one created next (unless these functions wrap
around, obviously) since both of these are based on the underlying
Vframe_list which is reproduced by 'frame-list'.

martin

[frame-list-1.diff (text/x-patch, attachment)]

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

From: Al Haji-Ali <abdo.haji.ali <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Tue, 29 Apr 2025 22:03:12 +0100

On 29/04/2025, martin rudalics wrote:
> I added the "(&optional frame)" because with a few exceptions most
> frame-related functions have it so coders don't have to think twice when
> calling any of these functions.

Thanks for the changes! I am incorporating them in my emacs config since I keep running into the infinite loop :)

Am I right in saying that `frame-list-1` is only currently being used in `make-frame-names-alist`, or did you mean to use it in other places as well?
If that's the only place, I think this function is only being used by `select-frame-by-name`. Both of these functions do not stipulate an order in there docs, so I am not sure a particular ordering is required?

Also a suggestion: if the ordering is indeed required, is there a reason for defining a new function instead of changing `frame-list` to take an argument (where the list starts), e.g., when passing nil the order is not modified.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Wed, 30 Apr 2025 11:47:10 +0200

> Am I right in saying that `frame-list-1` is only currently being used
> in `make-frame-names-alist`, or did you mean to use it in other places
> as well?

It could serve as template or substitute for anyone who uses
'next-frame' to get a list of frames and runs into an infinite loop.
Earlier Emacs versions will be still around for quite some time.

> If that's the only place, I think this function is only
> being used by `select-frame-by-name`. Both of these functions do not
> stipulate an order in there docs, so I am not sure a particular
> ordering is required?

IIUC 'select-frame-by-name' expects the frames created before the
selected frame be proposed first.  With plain 'frame-list' it would
always propose the most recently created frame - which would often be
the selected frame.  But I never use 'select-frame-by-name'.  Maybe you
can confirm that claim via say

(defun make-frame-names-alist (&optional frame)
  "Return alist of frame names and frames starting with FRAME.
Only visible or iconified frames are listed.  The optional argument
FRAME must specify a live frame and defaults to the selected frame."
  (let ((frames (frame-list))
	alist)
    (dolist (frame frames)
      (when (frame-visible-p frame)
	(push (cons (frame-parameter frame 'name) frame) alist)))
    (nreverse alist)))

> Also a suggestion: if the ordering is indeed required, is there a
> reason for defining a new function instead of changing `frame-list` to
> take an argument (where the list starts), e.g., when passing nil the
> order is not modified.

I contemplated that and found out that 'frame-list' may have to be
redesigned anyway to omit tooltip frames (and eventually also menubar
frames) on ttys and I'd like to resolve that issue before adding an
optional argument.

martin

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

From: martin rudalics <rudalics <at> gmx.at>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sun, 4 May 2025 12:02:59 +0200

[Message part 1 (text/plain, inline)]

> Thanks for the changes! I am incorporating them in my emacs config
> since I keep running into the infinite loop :)

I now made a number of additional changes:

- I rewrote next_frame in the style of prev_frame so it has to scan the
  frame list once only.

- `make-frame-names-alist' now lists only frames on the same terminal as
  FRAME and skips frames whose 'no-other-frame' parameter is non-nil.

- I updated some doc-strings.

Please try with the attached diff.  If you don't find any problems, I'll
install it in a couple of days.

Thanks, martin

[frame-list-1.diff (text/x-patch, attachment)]

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

From: Al Haji-Ali <abdo.haji.ali <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985 <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Tue, 06 May 2025 08:54:23 +0100

On 04/05/2025, martin rudalics wrote:

> I now made a number of additional changes:
>
> - I rewrote next_frame in the style of prev_frame so it has to scan the
>    frame list once only.
>
> - `make-frame-names-alist' now lists only frames on the same terminal as
>    FRAME and skips frames whose 'no-other-frame' parameter is non-nil.
>
> - I updated some doc-strings.
>
> Please try with the attached diff.  If you don't find any problems, I'll
> install it in a couple of days.
>

This patch also works for me (again with my limited tests).
I don't see any issues with the implementation/additional documentation.

Thanks!

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

From: martin rudalics <rudalics <at> gmx.at>
To: Al Haji-Ali <abdo.haji.ali <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 77985-done <at> debbugs.gnu.org
Subject: Re: bug#77985: 31.0.50; infinite loop in `make-frame-names-alist`
Date: Sat, 24 May 2025 11:42:02 +0200

> This patch also works for me (again with my limited tests).
> I don't see any issues with the implementation/additional documentation.

Marking as done.

Thanks, martin

Previous Next

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