GNU bug report logs - #58343
29.0.50; ELisp code run in "inconsitent" selected-window state

Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.

Package: emacs; Reported by: Stefan Monnier <monnier@HIDDEN>; dated Fri, 7 Oct 2022 00:07:01 UTC; Maintainer for emacs is bug-gnu-emacs@HIDDEN.

Message received at 58343 <at> debbugs.gnu.org:


Received: (at 58343) by debbugs.gnu.org; 12 Oct 2022 18:08:57 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Wed Oct 12 14:08:56 2022
Received: from localhost ([127.0.0.1]:58400 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1oigA4-0004tt-Ep
	for submit <at> debbugs.gnu.org; Wed, 12 Oct 2022 14:08:56 -0400
Received: from mx3.muc.de ([193.149.48.5]:57745)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <acm@HIDDEN>) id 1oigA2-0004tf-Pg
 for 58343 <at> debbugs.gnu.org; Wed, 12 Oct 2022 14:08:55 -0400
Received: (qmail 48875 invoked by uid 3782); 12 Oct 2022 20:08:48 +0200
Received: from acm.muc.de (p4fe15af2.dip0.t-ipconnect.de [79.225.90.242])
 (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP;
 Wed, 12 Oct 2022 20:08:47 +0200
Received: (qmail 12215 invoked by uid 1000); 12 Oct 2022 18:08:47 -0000
Date: Wed, 12 Oct 2022 18:08:47 +0000
To: Stefan Monnier <monnier@HIDDEN>
Subject: Re: bug#58343: 29.0.50; ELisp code run in "inconsitent"
 selected-window state
Message-ID: <Y0cCr/2++Si4xgVl@ACM>
References: <jwvfsg0cxvw.fsf-monnier+emacs@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <jwvfsg0cxvw.fsf-monnier+emacs@HIDDEN>
X-Submission-Agent: TMDA/1.3.x (Ph3nix)
From: Alan Mackenzie <acm@HIDDEN>
X-Primary-Address: acm@HIDDEN
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 58343
Cc: 58343 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

Hello, Stefan.

On Thu, Oct 06, 2022 at 20:06:25 -0400, Stefan Monnier wrote:
> Package: Emacs
> Version: 29.0.50


> Hi Alan and friends,

> In commit dfa3e6f424b20fe27d9041b2ce7d69811df5d8cd, Alan added the
> following code to do_switch_frame:

>     diff --git a/src/frame.c b/src/frame.c
>     index ccac18d23c2..dc8045f41e6 100644
>     --- a/src/frame.c
>     +++ b/src/frame.c
>     @@ -1564,6 +1564,13 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
>        if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
>          last_nonminibuf_frame = XFRAME (selected_frame);
     
>     +  /* If the selected window in the target frame is its mini-window, we move
>     +     to a different window, the most recently used one, unless there is a
>     +     valid active minibuffer in the mini-window.  */
>     +  if (EQ (f->selected_window, f->minibuffer_window)
>     +      && NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
>     +    Fset_frame_selected_window (frame, call1 (Qget_mru_window, frame), Qnil);
>     +
>        Fselect_window (f->selected_window, norecord);
     
>        /* We want to make sure that the next event generates a frame-switch

> the problem with this is that it calls `Qget_mru_window` which is ELisp
> code, and that we're in the very middle of changing the selected frame,
> so we have already changed the `selected-frame` variable a few lines
> earlier, but the select-window has not yet been adjusted accordingly.

> Running ELisp code in a state where (selected-window) and
> (frame-selected-window) aren't equal is a recipe for problems.  I have
> already wasted many hours in the past tracking down bugs linked to this
> kind of situation (back when the mode-line was processed in such an
> inconsistent state, for example), and I really don't want to go there.

> So we should arrange to run this `get-mru-window` function at some other
> time, for example a few lines earlier before we set `selected-frame`.

> I don't understand this code nearly enough to know how to move the code,
> because it probably interacts with the other statements in non-trivial
> ways, so the patch below is just a naive suggestion (it seems to work
> here without triggering my many sprinkled assertions checking that
> `EQ (XFRAME (selected_frame)->selected_window, selected_window)`,
> but it's a far cry from a confirmation that it's right).
> Hopefully someone here is aware of some of the potential pitfalls.

One minor problem I see at the moment is that the call to
move_minibuffers_onto_frame wasn't moved together with the surrounding
code.  With its current calling convention, it needs to be called
_after_ selected_frame has been set, because it uses selected_frame.

Is there any reason this function call was left where it was?  Or could
it just be moved, too?  Otherwise, its calling convention will need to
be adapted.

Other than that, I haven't seen any problems as yet, though I admit I
haven't tried it out.

>         Stefan

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#58343; Package emacs. Full text available.

Message received at submit <at> debbugs.gnu.org:


Received: (at submit) by debbugs.gnu.org; 7 Oct 2022 00:06:46 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Thu Oct 06 20:06:46 2022
Received: from localhost ([127.0.0.1]:33852 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1ogat3-0001jN-JL
	for submit <at> debbugs.gnu.org; Thu, 06 Oct 2022 20:06:46 -0400
Received: from lists.gnu.org ([209.51.188.17]:53906)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <monnier@HIDDEN>) id 1ogasz-0001j6-25
 for submit <at> debbugs.gnu.org; Thu, 06 Oct 2022 20:06:43 -0400
Received: from eggs.gnu.org ([2001:470:142:3::10]:49216)
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <monnier@HIDDEN>)
 id 1ogasy-0000Ej-42
 for bug-gnu-emacs@HIDDEN; Thu, 06 Oct 2022 20:06:40 -0400
Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:31233)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <monnier@HIDDEN>)
 id 1ogasv-0002XQ-5N
 for bug-gnu-emacs@HIDDEN; Thu, 06 Oct 2022 20:06:39 -0400
Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1])
 by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 5D2C2100142;
 Thu,  6 Oct 2022 20:06:34 -0400 (EDT)
Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1])
 by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 55028100091;
 Thu,  6 Oct 2022 20:06:32 -0400 (EDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca;
 s=mail; t=1665101192;
 bh=qDjeLU8aXe44UFrY8YfWs1gp1wErj/+vR5ThiBiuoSw=;
 h=From:To:Subject:Date:From;
 b=H8/N28OgF/x5Zzl070q9rERvk8NTF/6Jt6fPlHD4Wa4k3iOJocN1oI1ol7SCHKqZw
 KX2xTi2G636HpUB5Fl2NzZ2ePFfRpw/tVlQ/Z7QAelxxTZdviXTMpgZfL6LgmdNAZG
 sjhNgqGsa0PbxIMevJ4EwsI8FONl0WGR653cyPsOZ994coz6DPH3jRe+7EqNF0yB8u
 yDZjqqZ6ttX1F2EqxCVs/dg3zbDA4axGQpDAyXOjzz0u/r6VgSMjWX454qsd/tXQTA
 WiDg1DPTYz/7TlLbXK3Wt4ffQgOv6tgtmLDo52b9cptbsaBjRxAmpUssZEOQMHbeON
 PLz5WIjiTBNOg==
Received: from milanesa (65-110-220-202.cpe.pppoe.ca [65.110.220.202])
 by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 20F8A120CE9;
 Thu,  6 Oct 2022 20:06:32 -0400 (EDT)
From: Stefan Monnier <monnier@HIDDEN>
To: bug-gnu-emacs@HIDDEN
Subject: 29.0.50; ELisp code run in "inconsitent" selected-window state
Message-ID: <jwvfsg0cxvw.fsf-monnier+emacs@HIDDEN>
X-Debbugs-Cc: Alan Mackenzie <acm@HIDDEN>
Date: Thu, 06 Oct 2022 20:06:25 -0400
MIME-Version: 1.0
Content-Type: text/plain
X-SPAM-INFO: Spam detection results:  0
 ALL_TRUSTED                -1 Passed through trusted hosts only via SMTP
 AWL -0.058 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DKIM_SIGNED               0.1 Message has a DKIM or DK signature,
 not necessarily valid
 DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature
 DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's
 domain
X-SPAM-LEVEL: 
Received-SPF: pass client-ip=132.204.25.50;
 envelope-from=monnier@HIDDEN; helo=mailscanner.iro.umontreal.ca
X-Spam_score_int: -42
X-Spam_score: -4.3
X-Spam_bar: ----
X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3,
 SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: -1.3 (-)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -2.3 (--)

Package: Emacs
Version: 29.0.50


Hi Alan and friends,

In commit dfa3e6f424b20fe27d9041b2ce7d69811df5d8cd, Alan added the
following code to do_switch_frame:

    diff --git a/src/frame.c b/src/frame.c
    index ccac18d23c2..dc8045f41e6 100644
    --- a/src/frame.c
    +++ b/src/frame.c
    @@ -1564,6 +1564,13 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
       if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
         last_nonminibuf_frame = XFRAME (selected_frame);
     
    +  /* If the selected window in the target frame is its mini-window, we move
    +     to a different window, the most recently used one, unless there is a
    +     valid active minibuffer in the mini-window.  */
    +  if (EQ (f->selected_window, f->minibuffer_window)
    +      && NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
    +    Fset_frame_selected_window (frame, call1 (Qget_mru_window, frame), Qnil);
    +
       Fselect_window (f->selected_window, norecord);
     
       /* We want to make sure that the next event generates a frame-switch

the problem with this is that it calls `Qget_mru_window` which is ELisp
code, and that we're in the very middle of changing the selected frame,
so we have already changed the `selected-frame` variable a few lines
earlier, but the select-window has not yet been adjusted accordingly.

Running ELisp code in a state where (selected-window) and
(frame-selected-window) aren't equal is a recipe for problems.  I have
already wasted many hours in the past tracking down bugs linked to this
kind of situation (back when the mode-line was processed in such an
inconsistent state, for example), and I really don't want to go there.

So we should arrange to run this `get-mru-window` function at some other
time, for example a few lines earlier before we set `selected-frame`.

I don't understand this code nearly enough to know how to move the code,
because it probably interacts with the other statements in non-trivial
ways, so the patch below is just a naive suggestion (it seems to work
here without triggering my many sprinkled assertions checking that
`EQ (XFRAME (selected_frame)->selected_window, selected_window)`,
but it's a far cry from a confirmation that it's right).
Hopefully someone here is aware of some of the potential pitfalls.


        Stefan


diff --git a/src/frame.c b/src/frame.c
index 91b9bec82c3..58b6ee50d23 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1503,18 +1503,8 @@ do_switch_frame (Lisp_Object frame, int for_deletion, Lisp_Object norecord)
 
   sf->select_mini_window_flag = MINI_WINDOW_P (XWINDOW (sf->selected_window));
 
-  selected_frame = frame;
-
   move_minibuffers_onto_frame (sf, for_deletion);
 
-  if (f->select_mini_window_flag
-      && !NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
-    f->selected_window = f->minibuffer_window;
-  f->select_mini_window_flag = false;
-
-  if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
-    last_nonminibuf_frame = XFRAME (selected_frame);
-
   /* If the selected window in the target frame is its mini-window, we move
      to a different window, the most recently used one, unless there is a
      valid active minibuffer in the mini-window.  */
@@ -1528,6 +1518,16 @@ do_switch_frame (Lisp_Object frame, int for_deletion, Lisp_Object norecord)
         Fset_frame_selected_window (frame, w, Qnil);
     }
 
+  selected_frame = frame;
+
+  if (f->select_mini_window_flag
+      && !NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
+    f->selected_window = f->minibuffer_window;
+  f->select_mini_window_flag = false;
+
+  if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
+    last_nonminibuf_frame = XFRAME (selected_frame);
+
   Fselect_window (f->selected_window, norecord);
 
   /* We want to make sure that the next event generates a frame-switch





Acknowledgement sent to Stefan Monnier <monnier@HIDDEN>:
New bug report received and forwarded. Copy sent to acm@HIDDEN, bug-gnu-emacs@HIDDEN. Full text available.
Report forwarded to acm@HIDDEN, bug-gnu-emacs@HIDDEN:
bug#58343; Package emacs. Full text available.
Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.
Last modified: Wed, 12 Oct 2022 18:15:02 UTC

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