From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#16051: 24.3.50; Emacs hang - resize frame manually Date: Mon, 23 Dec 2013 18:57:49 +0200 Message-ID: <83zjnr7bea.fsf@gnu.org> References: <3eea48d4-9267-45fa-84c8-3eb9c9290558@default> <52B59B44.9060307@poczta.onet.pl> <83a9fu9r1j.fsf@gnu.org> <52B5B7EA.2080809@poczta.onet.pl> <837gay9nx2.fsf@gnu.org> <52B5CC35.10404@gmx.at> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1387817891 10150 80.91.229.3 (23 Dec 2013 16:58:11 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 23 Dec 2013 16:58:11 +0000 (UTC) Cc: jarekczek@poczta.onet.pl, 16051@debbugs.gnu.org To: martin rudalics Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Dec 23 17:58:16 2013 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Vv8pb-0004nF-VL for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Dec 2013 17:58:16 +0100 Original-Received: from localhost ([::1]:34498 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vv8pb-0000Ow-Mh for geb-bug-gnu-emacs@m.gmane.org; Mon, 23 Dec 2013 11:58:15 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55591) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vv8pU-0000OG-0r for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 11:58:12 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vv8pP-0000TX-AV for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 11:58:07 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:50374) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vv8pP-0000T9-73 for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 11:58:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1Vv8pO-0006L4-ET for bug-gnu-emacs@gnu.org; Mon, 23 Dec 2013 11:58:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 23 Dec 2013 16:58:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 16051 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 16051-submit@debbugs.gnu.org id=B16051.138781788124359 (code B ref 16051); Mon, 23 Dec 2013 16:58:02 +0000 Original-Received: (at 16051) by debbugs.gnu.org; 23 Dec 2013 16:58:01 +0000 Original-Received: from localhost ([127.0.0.1]:36160 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Vv8pM-0006Km-R5 for submit@debbugs.gnu.org; Mon, 23 Dec 2013 11:58:01 -0500 Original-Received: from mtaout23.012.net.il ([80.179.55.175]:42059) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Vv8pJ-0006KV-Sr for 16051@debbugs.gnu.org; Mon, 23 Dec 2013 11:57:59 -0500 Original-Received: from conversion-daemon.a-mtaout23.012.net.il by a-mtaout23.012.net.il (HyperSendmail v2007.08) id <0MY900700R13X600@a-mtaout23.012.net.il> for 16051@debbugs.gnu.org; Mon, 23 Dec 2013 18:57:56 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout23.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MY9007TUR4JWP10@a-mtaout23.012.net.il>; Mon, 23 Dec 2013 18:57:56 +0200 (IST) In-reply-to: <52B5CC35.10404@gmx.at> X-012-Sender: halo1@inter.net.il X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.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.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:82447 Archived-At: > Date: Sat, 21 Dec 2013 18:13:25 +0100 > From: martin rudalics > CC: Jarek Czekalski , 16051@debbugs.gnu.org > > > I don't think it's important, certainly not extremely important. You > > can hang Emacs with as little as '(while t)'. Users who do > > unreasonable things should expect trouble, because Emacs gives them a > > lot of rope to hang themselves. > > If I do drag _very_ fast as described by the OP, Emacs reliably crashes > here as: > > > #0 terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at emacs.c:351 > #1 0x0116400a in die (msg=0x14763e0 "row >= 0 && row < matrix->nrows", file=0x147618c "dispnew.c", line=1369) at alloc.c:6742 > #2 0x01004ed3 in matrix_row (matrix=0x36e2800, row=7) at dispnew.c:1369 > #3 0x01040eaa in hscroll_window_tree (window=...) at xdisp.c:12610 > #4 0x01041398 in hscroll_windows (window=...) at xdisp.c:12737 > #5 0x010437bd in redisplay_internal () at xdisp.c:13631 > #6 0x01044126 in redisplay_preserve_echo_area (from_where=11) at xdisp.c:13856 It turns out this bug has 3 separate parts. 2 of them are very clear and uncontroversial, so I simply fixed them; see revision 115718 on the trunk. After that, there are no more assertion violations like above, and no hangs, and it is much harder to cause a redisplay loop. Also, the only redisplay loops I can trigger are immediately interrupted by C-g. But since such loops can still be triggered, there's another part to this riddle. Here, the problem and the solution seem clear to me, but since the code is quite deliberately doing what it does, I'd like to ask Martin about the underlying logic. I'm talking about this part of redisplay_tool_bar: if (change_height_p) { int new_lines = ((new_height + FRAME_LINE_HEIGHT (f) - 1) / FRAME_LINE_HEIGHT (f)); XSETFRAME (frame, f); Fmodify_frame_parameters (frame, list1 (Fcons (Qtool_bar_lines, make_number (new_lines)))); /* Always do that now. */ clear_glyph_matrix (w->desired_matrix); f->n_tool_bar_rows = nrows; f->fonts_changed = 1; return 1; } Why does it make sense to "always do that"? Suppose new_lines is exactly equal to the current number of canonical lines used by the tool-bar window (it can happen even if pixel sizes are different, due to integer truncation). Or suppose you are asking for N lines, but don't get it because the frame is too small. Why set the fonts_changed flag in those cases? That flag causes redisplay to give up, abandon the current glyph matrices, and retry anew. What happens with those endless cycles is precisely that: redisplay_tool_bar sets the fonts_changed flag, which causes redisplay_internal to retry, which again calls redisplay_tool_bar, which again sets the flag, etc., ad nauseam. If there is some reasoning behind this "always do that" logic, please describe it. Otherwise, I propose the patch below, which cures the problem altogether for me; if you agree, I will install it. As an aside, I stared for a long time at this part of w32fns.c:x_set_tool_bar_lines (which is part of the issue, because it is called by modify-frame-parameters, when the tool-bar-lines parameter is changed), and I still don't get why it does what it does: root_height = WINDOW_PIXEL_HEIGHT (XWINDOW (FRAME_ROOT_WINDOW (f))); if (root_height - delta < unit) { delta = root_height - unit; nlines = (root_height / unit) + min (1, (root_height % unit)); } First, delta is recomputed, but the result is never used. More importantly, you assign to nlines a value that is the size of the root window _plus_ one line? Did you mean minus instead? Finally, the corresponding xfns.c implementation still works in lines, not in pixels, as w32fns.c did before your pixelwise changes. Is this disparity on purpose or an oversight? Here's the patch I propose to solve the last part of this bug. It abandons the "do it always" method, and only changes the tool-bar-lines parameter and sets the fonts_changed flag if the required height of the tool bar differs from the current height by at least 1 canonical line, and if it is smaller than the maximum number of lines any window can get on this frame. --- src/xdisp.c~1 2013-12-23 18:20:09.678832900 +0200 +++ src/xdisp.c 2013-12-23 18:50:56.993834000 +0200 @@ -12289,18 +12289,24 @@ redisplay_tool_bar (struct frame *f) if (change_height_p) { + int old_lines = WINDOW_TOTAL_LINES (w); int new_lines = ((new_height + FRAME_LINE_HEIGHT (f) - 1) / FRAME_LINE_HEIGHT (f)); + int max_lines = + WINDOW_TOTAL_LINES (XWINDOW (FRAME_ROOT_WINDOW (f))) - 1; - XSETFRAME (frame, f); - Fmodify_frame_parameters (frame, - list1 (Fcons (Qtool_bar_lines, - make_number (new_lines)))); - /* Always do that now. */ - clear_glyph_matrix (w->desired_matrix); - f->n_tool_bar_rows = nrows; - f->fonts_changed = 1; - return 1; + if (new_lines <= max_lines + && eabs (new_lines - old_lines) >= 1) + { + XSETFRAME (frame, f); + Fmodify_frame_parameters (frame, + list1 (Fcons (Qtool_bar_lines, + make_number (new_lines)))); + clear_glyph_matrix (w->desired_matrix); + f->n_tool_bar_rows = nrows; + f->fonts_changed = 1; + return 1; + } } } }