From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: martin rudalics Newsgroups: gmane.emacs.bugs Subject: bug#39977: 28.0.50; Unhelpful stack trace Date: Sat, 28 Mar 2020 19:38:24 +0100 Message-ID: <8f21c5ce-49a1-1865-6ea3-fe5120c1cfc5@gmx.at> References: <83blozckn2.fsf@gnu.org> <01305dbc-c69b-baf9-f0bf-1e5b8c04d970@gmx.at> <83y2s2bswl.fsf@gnu.org> <3ac189d0-5d05-fdf9-0922-0c464b1b17c3@gmx.at> <83k13lbgux.fsf@gnu.org> <83d09cb9gz.fsf@gnu.org> <69a74f9e-079b-a771-0213-f60ed0bf5720@gmx.at> <83y2rzf08m.fsf@gnu.org> <7a0b9999-6778-6235-fbc9-2a24b4e3bc53@gmx.at> <83tv2mg9j0.fsf@gnu.org> <9684641c-a59a-4ed6-a6b4-d3238f789050@gmx.at> <83sgi6g461.fsf@gnu.org> <90ac4084-4fbb-b5d7-6a7c-597d8f08e88a@gmx.at> <83imj1g1eu.fsf@gnu.org> <835zf1fobf.fsf@gnu.org> <4472dbf8-eec0-72eb-a4ad-c6b382d27f1f@gmx.at> <83zhcce7n2.fsf@gnu.org> <9a3df43d-4a83-bed0-9ad1-b59cf11b4c9c@gmx.at> <838sjtetmp.fsf@gnu.org> <88e06868-cfc2-7109-b5ea-71fea3aa897e@gmx.at> <83mu87b004.fsf@gnu.org> <83y2rk7uri.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------A3F4A933B0CBF1134683D960" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="94076"; mail-complaints-to="usenet@ciao.gmane.io" Cc: enometh@meer.net, 39977@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Mar 30 04:39:10 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jIkKU-000ONi-AR for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 30 Mar 2020 04:39:10 +0200 Original-Received: from localhost ([::1]:43808 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jIkKT-0008C2-4U for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 29 Mar 2020 22:39:09 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55138) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jIkHy-0003zQ-LI for bug-gnu-emacs@gnu.org; Sun, 29 Mar 2020 22:36:37 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jIkHw-0003lf-Bg for bug-gnu-emacs@gnu.org; Sun, 29 Mar 2020 22:36:34 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48656) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1jIkHw-0003lQ-51 for bug-gnu-emacs@gnu.org; Sun, 29 Mar 2020 22:36:32 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jIkHw-0004jl-0J for bug-gnu-emacs@gnu.org; Sun, 29 Mar 2020 22:36:32 -0400 X-Loop: help-debbugs@gnu.org Resent-From: martin rudalics Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 30 Mar 2020 02:36:31 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 39977 X-GNU-PR-Package: emacs Original-Received: via spool by 39977-submit@debbugs.gnu.org id=B39977.158553573217322 (code B ref 39977); Mon, 30 Mar 2020 02:36:31 +0000 Original-Received: (at 39977) by debbugs.gnu.org; 30 Mar 2020 02:35:32 +0000 Original-Received: from mout.gmx.net ([212.227.15.15]:51649) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jIGLr-0004VW-Nk for 39977@debbugs.gnu.org; Sat, 28 Mar 2020 14:38:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1585420705; bh=AcqcI4pJH+Z8Crkm7e4c0LQdre6RZUCEdjskHTlNoag=; h=X-UI-Sender-Class:Subject:To:Cc:References:From:Date:In-Reply-To; b=KjxL0GoPaIrhb6leuwFX15u8QaNsVnn92x78Yxn3D3eaBMB14HxyckXCKgDC5Y0N9 OvopEnVkofu0i9FIDb+xJRzxs02Tm/ZGMiWDMU85tBmO1HPselst7trNMd3oDiEX/h 1nAsS2RZxjAbLAYsqcf5HQJ6NEAFTHSPBtbbK5wY= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from [192.168.1.102] ([212.95.5.169]) by mail.gmx.com (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MN5eR-1izMSC21UD-00J0xh; Sat, 28 Mar 2020 19:38:25 +0100 In-Reply-To: <83y2rk7uri.fsf@gnu.org> Content-Language: en-US X-Provags-ID: V03:K1:SyOd37vHpe/YansDudWUB6/M56pce2DJ6e+9SrQ0lmsOI1+IVoY z/eO0UN+3is0iCyQmnptNU1wDQXmgGCr8gaENzx1fKDS0ytbRK4zVSysUZtW2Rp1742x4Cz 38ez6fiBW0W45jL2EeAXA+iXPEcifJDXC4V7p+X4UR4gRMhUONwIhf5Eyq3Kd+Dj4tnIco6 yEYFb5m/LgToQMJN/lakg== X-UI-Out-Filterresults: notjunk:1;V03:K0:JzjWQYTbmps=:YFbOafrDrcIaS4EQFa5MIn QUhIaOtW2SsOQLqSROALBDt2Sh8U+WE3BLi6uF7KDEcMRD0i/+GZM8GyvR11ayicSmEO3E8QY pyu3TZglqJWt6PhPyNUBjqcr85sdclsivN79zQJ9TwEp6jGeHJo+l54pbVLMZCYGXYtaLakF2 fSz8TbCGHO9j9t1HRawMoAh6p+7qAVS7y7PWMr25YPj4ODMl26rtuJnKBLih5+boQwB8voqWB 0pDsAmRtnL79QyPGDS3Qxk3oUcBFicTqI6V51ZfbiProBFK4ohJKllorviZ0RaGkSNXN/4IOf tGljjTkXHG3S+UW1z9uo6n2IEBNjYU9UuDpGQiLx+RAMs1o/JK51z2Yyj85ADEQcsLJI6CFB1 SpU3/xmXzZkrSWkYfLjhCYW2+8m1bMoit8jhKOIJHoxUCrGs3dEENOiWOpqIJfGd98cCEJTil K6B0vqVD/RjpWqEVp9YfjZFVPs9vbwRq3tUUy+zjP5obRWktSOTDbdr8FF1sgBqrcl2lwLJZQ qpwqqQrqy29IVtQB0XkxYQf5itUFIjSpfKbc3IQ1k3Pkw3B4CiUpM4B6Q7/BgO3ceb7BvJB3v fHZpVRsFhL8GgqtAGxIVhGsClpmbejmByiRlN3Tko0VC12hoFW0+3OBzTMd6UAynfJ+HW3AVE 6aArPRaVOLBL4V8Gjxvz6E+J745tnpd0kNCSZClypMSYEhN22Xq8SMuIfqT6J+SOlXMF3fxxN xcrb7ths9/pjkdj+m05U8ozvBlF7R5i0CojMsz41+fcndBcsR/a6HrAtvturhgu/jV2L/f8V X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-List-Received-Date: Sat, 28 Mar 2020 18:38:36 -0000 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:177850 Archived-At: This is a multi-part message in MIME format. --------------A3F4A933B0CBF1134683D960 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit >> >> (and (frame-live-p (selected-frame)) >> >> (window-live-p (selected-window)) >> >> (eq (frame-selected-window (selected-frame)) >> >> (selected-window))) >> > >> > I agree. But note that selected-frame could switch frames internally, >> > if the last selected frame is dead; as long as selected-frame also >> > adjusts the selected window, the above will still hold. >> >> Do you mean 'select-frame' instead of 'selected-frame'? > > No, I meant selected-frame. So you meant a 'selected-frame' based on the SELECTED_FRAME I proposed. The problem is that that macro won't protect the invariant if it was broken before, for example, by setting the selected window to a dead window as in Madhu's scenario. > Perhaps we should make unwind_format_mode_line less fragile, then. The problematic step happens already at the time we set it up. In a nutshell, Madhu's scenario goes as follows: In display_mode_lines we do Lisp_Object old_selected_window = selected_window; Lisp_Object old_selected_frame = selected_frame; ... display_mode_line (w, CURRENT_MODE_LINE_FACE_ID_3 (sel_w, sel_w, w), NILP (window_mode_line_format) ? BVAR (current_buffer, mode_line_format) : window_mode_line_format); ... XFRAME (new_frame)->selected_window = old_frame_selected_window; selected_frame = old_selected_frame; selected_window = old_selected_window; where display_mode_line deletes both, old_selected_frame and old_selected_window. So we end up with selected_frame and selected_window both referencing dead objects. The subsequent call of gui_consider_frame_title now does record_unwind_protect (unwind_format_mode_line, format_mode_line_unwind_data (f, current_buffer, selected_window, false)); where selected_window is already a dead window. Since in unwind_format_mode_line old_window is non-nil, it will call Fselect_window (old_window, Qt); which first chokes on CHECK_LIVE_WINDOW (window); which is the error reported when emacs does not crash and finally on sf = SELECTED_FRAME (); which crashes emacs due to the fact that selected_frame is dead. In either case, making the unwind_format_mode_line less fragile won't avoid any crash, it might just postpone it. >> And I have no idea yet why we need an extra unwind for restoring >> selected_frame and selected_window. Shouldn't these go hand in hand >> with what unwind_format_mode_line does? Does the one even know >> about the other? > > I don't think I understand what extra unwind are you talking about > here. Can you provide a more specific pointer to the relevant code? I meant the fact that we already do unwind_format_mode_line when formatting the mode line and that function could restore the selected window in a safe way. I'm far from proposing to use that approach when drawing the mode lines, though. Attached find a patch which should solve the more grave problems caused by a function deleting the previously selected frame or window. It intentionally does not change SELECTED_FRAME. Any abort there should be reserved to obscure bugs we have not been able to trace yet. Please read it with your usual care, it took me some time to convince myself that it selects its frame in a reasonable way. On master, I would then like to use restore_selected_window also for gui_consider_frame_title. The overhead caused by that is a great annoyance (especially when debugging frame switching code) and we could then hopefully get rid of the old_window stuff and the Bug#32777 fix as well. What any patch I provide here cannot do is to fix problems when the mode line code deletes the selected window right away. Code like (defvar window (split-window)) (defvar foo '(:eval (if (or (not (window-live-p window)) (eq window (frame-first-window))) (setq window (split-window)) (delete-window window)))) (setq-default mode-line-format foo) will continue to segfault unless you can cure that. I tried to fix it in the spirit of if (!FRAME_LIVE_P (it->f)) signal_error (":eval deleted the frame being displayed", elt); but that just caused emacs to hang. martin --------------A3F4A933B0CBF1134683D960 Content-Type: text/x-patch; name="restore_selected_window.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="restore_selected_window.diff" diff --git a/src/xdisp.c b/src/xdisp.c index a4de2698ca..e0273aba3f 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -12149,12 +12149,12 @@ unwind_format_mode_line (Lisp_Object vector) mode_line_string_face_prop = AREF (vector, 5); /* Select window before buffer, since it may change the buffer. */ - if (!NILP (old_window)) + if (WINDOW_LIVE_P (old_window)) { /* If the operation that we are unwinding had selected a window on a different frame, reset its frame-selected-window. For a text terminal, reset its top-frame if necessary. */ - if (!NILP (target_frame_window)) + if (WINDOW_LIVE_P (target_frame_window)) { Lisp_Object frame = WINDOW_FRAME (XWINDOW (target_frame_window)); @@ -12171,7 +12171,7 @@ unwind_format_mode_line (Lisp_Object vector) /* Restore point of target_frame_window's buffer (Bug#32777). But do this only after old_window has been reselected to avoid that the window point of target_frame_window moves. */ - if (!NILP (target_frame_window)) + if (WINDOW_LIVE_P (target_frame_window)) { Lisp_Object buffer = AREF (vector, 10); @@ -12594,18 +12598,58 @@ update_menu_bar (struct frame *f, bool save_match_data, bool hooks_run) #ifdef HAVE_WINDOW_SYSTEM -/* Select `frame' temporarily without running all the code in - do_switch_frame. - FIXME: Maybe do_switch_frame should be trimmed down similarly - when `norecord' is set. */ +/* Restore WINDOW as the selected window and its frame as the selected + frame. If WINDOW is dead but the selected frame is live, make the + latter's selected window the selected window. If both, WINDOW and + the selected frame, are dead, assign selected frame and window from + some arbitrary live frame. Abort if no such frame can be found. */ static void -fast_set_selected_frame (Lisp_Object frame) +restore_selected_window (Lisp_Object window) { - if (!EQ (selected_frame, frame)) + if (WINDOW_LIVE_P (window)) + /* If WINDOW is live, make it the selected window and set the + selected frame to its frame. */ { - selected_frame = frame; - selected_window = XFRAME (frame)->selected_window; + selected_window = window; + selected_frame = XWINDOW (window)->frame; } + else if (FRAMEP (selected_frame) && FRAME_LIVE_P (XFRAME (selected_frame))) + /* If WINDOW is dead but the selected frame is still live, make the + latter's selected window the selected one. */ + selected_window = FRAME_SELECTED_WINDOW (XFRAME (selected_frame)); + else + /* If WINDOW and the selected frame are dead, choose some live, + non-child and non-tooltip frame as the new selected frame and + make its selected window the selected window. */ + { + Lisp_Object tail; + Lisp_Object frame UNINIT; + + FOR_EACH_FRAME (tail, frame) + { + struct frame *f = XFRAME (frame); + + if (!FRAME_PARENT_FRAME (f) && !FRAME_TOOLTIP_P (f)) + { + selected_frame = frame; + selected_window = FRAME_SELECTED_WINDOW (f); + + return; + } + } + + /* Abort if we cannot find a live frame. */ + emacs_abort (); + } +} + +/* Restore WINDOW, if live, as its frame's selected window. */ +static void +restore_frame_selected_window (Lisp_Object window) +{ + if (WINDOW_LIVE_P (window)) + /* If WINDOW is live, make it its frame's selected window. */ + FRAME_SELECTED_WINDOW (XFRAME (XWINDOW (window)->frame)) = window; } #endif /* HAVE_WINDOW_SYSTEM */ @@ -12681,9 +12725,10 @@ update_tab_bar (struct frame *f, bool save_match_data) XFRAME (selected_frame)->selected_window)); #ifdef HAVE_WINDOW_SYSTEM Lisp_Object frame; - record_unwind_protect (fast_set_selected_frame, selected_frame); + record_unwind_protect (restore_selected_window, selected_window); XSETFRAME (frame, f); - fast_set_selected_frame (frame); + selected_frame = frame; + selected_window = FRAME_SELECTED_WINDOW (f); #endif /* Build desired tab-bar items from keymaps. */ @@ -13625,9 +13670,10 @@ update_tool_bar (struct frame *f, bool save_match_data) /* Since we only explicitly preserve selected_frame, check that selected_window would be redundant. */ XFRAME (selected_frame)->selected_window)); - record_unwind_protect (fast_set_selected_frame, selected_frame); + record_unwind_protect (restore_selected_window, selected_window); XSETFRAME (frame, f); - fast_set_selected_frame (frame); + selected_frame = frame; + selected_window = FRAME_SELECTED_WINDOW (f); /* Build desired tool-bar items from keymaps. */ new_tool_bar @@ -24939,11 +24985,14 @@ redisplay_mode_lines (Lisp_Object window, bool force) display_mode_lines (struct window *w) { Lisp_Object old_selected_window = selected_window; - Lisp_Object old_selected_frame = selected_frame; Lisp_Object new_frame = w->frame; - Lisp_Object old_frame_selected_window = XFRAME (new_frame)->selected_window; + ptrdiff_t count = SPECPDL_INDEX (); int n = 0; + record_unwind_protect (restore_selected_window, selected_window); + record_unwind_protect + (restore_frame_selected_window, XFRAME (new_frame)->selected_window); + if (window_wants_mode_line (w)) { Lisp_Object window; @@ -25009,9 +25058,8 @@ display_mode_lines (struct window *w) ++n; } - XFRAME (new_frame)->selected_window = old_frame_selected_window; - selected_frame = old_selected_frame; - selected_window = old_selected_window; + unbind_to (count, Qnil); + if (n > 0) w->must_be_updated_p = true; return n; --------------A3F4A933B0CBF1134683D960--