GNU bug report logs - #31312
Segmentation fault with doom-emacs, NeoTree and Zoom

Previous Next

Package: emacs;

Reported by: Andrea Cardaci <cyrus.and <at> gmail.com>

Date: Sun, 29 Apr 2018 17:01:02 UTC

Severity: normal

Tags: confirmed, fixed

Fixed in version 26.1

Done: Noam Postavsky <npostavs <at> gmail.com>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Sun, 29 Apr 2018 17:01:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Andrea Cardaci <cyrus.and <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 29 Apr 2018 17:01:02 GMT) Full text and rfc822 format available.

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

From: Andrea Cardaci <cyrus.and <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Sun, 29 Apr 2018 12:56:47 +0200
[Message part 1 (text/plain, inline)]
Hello,

Someone filed a bug for my repo (https://github.com/cyrus-and/zoom/issues/20)
but I'm pretty sure it's a bug in Emacs. Unfortunately the steps to
reproduce it involve setting up `doom-emacs` which is quite cumbersome.

You can find all the details at the above link but in short using
`doom-emacs` with Zoom and NeoTree crashes Emacs with a segmentation fault.

I apologize for the low quality of this bug report, but I think that's
better than nothing.


Andrea
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Tue, 01 May 2018 13:27:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Andrea Cardaci <cyrus.and <at> gmail.com>
Cc: 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Tue, 01 May 2018 09:25:59 -0400
tags 31312 + confirmed
found 31312 26.1
quit

Andrea Cardaci <cyrus.and <at> gmail.com> writes:

> Someone filed a bug for my repo (https://github.com/cyrus-and/zoom/issues/20)
> but I'm pretty sure it's a bug in Emacs. Unfortunately the steps to
> reproduce it involve setting up `doom-emacs` which is quite cumbersome.
>
> You can find all the details at the above link but in short using
> `doom-emacs` with Zoom and NeoTree crashes Emacs with a segmentation fault.

Reproduced with emacs-26, seems there is a window without a buffer.

../../src/buffer.h:914: Emacs fatal error: assertion failed: BUFFERP (a)

Thread 1 "emacs" hit Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
    at ../../src/emacs.c:364
364	  signal (sig, SIG_DFL);
(gdb) bt
#0  terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../src/emacs.c:364
#1  0x0000000000614655 in die (msg=0x74ecf9 "BUFFERP (a)", file=0x74ece6 "../../src/buffer.h", 
    line=914) at ../../src/alloc.c:7423
#2  0x000000000057a1a8 in XBUFFER (a=XIL(0)) at ../../src/buffer.h:914
#3  0x0000000000464716 in reconsider_clip_changes (w=0x60537f0) at ../../src/xdisp.c:13727
#4  0x000000000046514d in redisplay_internal () at ../../src/xdisp.c:13939
#5  0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
#6  0x00000000005851ae in read_char (commandflag=1, map=XIL(0x5cf7503), prev_event=XIL(0), 
    used_mouse_menu=0x7fffffffe41f, end_time=0x0) at ../../src/keyboard.c:2480
#7  0x00000000005957c5 in read_key_sequence (keybuf=0x7fffffffe570, bufsize=30, prompt=XIL(0), 
    dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, 
    prevent_redisplay=false) at ../../src/keyboard.c:9147
#8  0x0000000000581d55 in command_loop_1 () at ../../src/keyboard.c:1368
#9  0x0000000000638529 in internal_condition_case (bfun=0x581909 <command_loop_1>, 
    handlers=XIL(0x5250), hfun=0x580f4c <cmd_error>) at ../../src/eval.c:1332
#10 0x0000000000581524 in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1110
#11 0x0000000000637a32 in internal_catch (tag=XIL(0xc6f0), func=0x5814f7 <command_loop_2>, arg=XIL(0))
    at ../../src/eval.c:1097
#12 0x00000000005814c2 in command_loop () at ../../src/keyboard.c:1089
#13 0x0000000000580a36 in recursive_edit_1 () at ../../src/keyboard.c:695
#14 0x0000000000580c2b in Frecursive_edit () at ../../src/keyboard.c:766
#15 0x000000000057e7c6 in main (argc=1, argv=0x7fffffffea38) at ../../src/emacs.c:1713
(gdb) frame 3
#3  0x0000000000464716 in reconsider_clip_changes (w=0x60537f0) at ../../src/xdisp.c:13727
13727	  struct buffer *b = XBUFFER (w->contents);
(gdb) p w->contents
$1 = XIL(0)





Added tag(s) confirmed. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Tue, 01 May 2018 13:27:02 GMT) Full text and rfc822 format available.

bug Marked as found in versions 26.1. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Tue, 01 May 2018 13:27:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Tue, 01 May 2018 14:34:02 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Noam Postavsky <npostavs <at> gmail.com>, Andrea Cardaci <cyrus.and <at> gmail.com>
Cc: 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Tue, 01 May 2018 16:33:07 +0200
> Reproduced with emacs-26, seems there is a window without a buffer.
>
> ../../src/buffer.h:914: Emacs fatal error: assertion failed: BUFFERP (a)
>
> Thread 1 "emacs" hit Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
>      at ../../src/emacs.c:364
> 364	  signal (sig, SIG_DFL);
> (gdb) bt
> #0  terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../src/emacs.c:364
> #1  0x0000000000614655 in die (msg=0x74ecf9 "BUFFERP (a)", file=0x74ece6 "../../src/buffer.h",
>      line=914) at ../../src/alloc.c:7423
> #2  0x000000000057a1a8 in XBUFFER (a=XIL(0)) at ../../src/buffer.h:914
> #3  0x0000000000464716 in reconsider_clip_changes (w=0x60537f0) at ../../src/xdisp.c:13727
> #4  0x000000000046514d in redisplay_internal () at ../../src/xdisp.c:13939
> #5  0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
> #6  0x00000000005851ae in read_char (commandflag=1, map=XIL(0x5cf7503), prev_event=XIL(0),
>      used_mouse_menu=0x7fffffffe41f, end_time=0x0) at ../../src/keyboard.c:2480
> #7  0x00000000005957c5 in read_key_sequence (keybuf=0x7fffffffe570, bufsize=30, prompt=XIL(0),
>      dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true,
>      prevent_redisplay=false) at ../../src/keyboard.c:9147
> #8  0x0000000000581d55 in command_loop_1 () at ../../src/keyboard.c:1368
> #9  0x0000000000638529 in internal_condition_case (bfun=0x581909 <command_loop_1>,
>      handlers=XIL(0x5250), hfun=0x580f4c <cmd_error>) at ../../src/eval.c:1332
> #10 0x0000000000581524 in command_loop_2 (ignore=XIL(0)) at ../../src/keyboard.c:1110
> #11 0x0000000000637a32 in internal_catch (tag=XIL(0xc6f0), func=0x5814f7 <command_loop_2>, arg=XIL(0))
>      at ../../src/eval.c:1097
> #12 0x00000000005814c2 in command_loop () at ../../src/keyboard.c:1089
> #13 0x0000000000580a36 in recursive_edit_1 () at ../../src/keyboard.c:695
> #14 0x0000000000580c2b in Frecursive_edit () at ../../src/keyboard.c:766
> #15 0x000000000057e7c6 in main (argc=1, argv=0x7fffffffea38) at ../../src/emacs.c:1713
> (gdb) frame 3

Is w in reconsider_clip_changes the same as XWINDOW (selected_window)
when it fails?  If so, could you instrument select_window_1 (or use a
watchpoint) to check whether selected_window = window ever assigns a
window with XWINDOW (window)->contents equal Qnil?

Thanks, martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Tue, 01 May 2018 23:11:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Tue, 01 May 2018 19:10:09 -0400
martin rudalics <rudalics <at> gmx.at> writes:

> Is w in reconsider_clip_changes the same as XWINDOW (selected_window)
> when it fails?

Nope.  In fact, doing

    break 13939 if (w != XWINDOW(selected_window))
    
triggers only just before the bad reconsider_clip_changes call.

#0  redisplay_internal () at ../../src/xdisp.c:13939
#1  0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
#2  0x00000000005851ae in read_char (commandflag=1, map=XIL(0x5e99db3), prev_event=XIL(0), 
    used_mouse_menu=0x7fffffffe41f, end_time=0x0) at ../../src/keyboard.c:2480
#3  0x00000000005957c5 in read_key_sequence (keybuf=0x7fffffffe570, bufsize=30, prompt=XIL(0), 
    dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, 
    prevent_redisplay=false) at ../../src/keyboard.c:9147
#4  0x0000000000581d55 in command_loop_1 () at ../../src/keyboard.c:1368
(More stack frames follow...)
(gdb) p w
$16 = (struct window *) 0x47d22f0
(gdb) p sw
$17 = (struct window *) 0x47d22f0
(gdb) p XWINDOW(selected_window)
$18 = (struct window *) 0x61109d0
(gdb) p w->contents
$19 = XIL(0)
(gdb) p XWINDOW(selected_window)->contents
$20 = XIL(0x493e565)
(gdb) xpr
Lisp_Vectorlike
PVEC_BUFFER
$21 = (struct buffer *) 0x493e560
(unsigned char *) 0x4a34568 " *NeoTree*"




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 06:17:01 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 08:16:26 +0200
>> Is w in reconsider_clip_changes the same as XWINDOW (selected_window)
>> when it fails?
>
> Nope.  In fact, doing
>
>      break 13939 if (w != XWINDOW(selected_window))
>
> triggers only just before the bad reconsider_clip_changes call.
>
> #0  redisplay_internal () at ../../src/xdisp.c:13939
> #1  0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
> #2  0x00000000005851ae in read_char (commandflag=1, map=XIL(0x5e99db3), prev_event=XIL(0),
>      used_mouse_menu=0x7fffffffe41f, end_time=0x0) at ../../src/keyboard.c:2480
> #3  0x00000000005957c5 in read_key_sequence (keybuf=0x7fffffffe570, bufsize=30, prompt=XIL(0),
>      dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true,
>      prevent_redisplay=false) at ../../src/keyboard.c:9147
> #4  0x0000000000581d55 in command_loop_1 () at ../../src/keyboard.c:1368
> (More stack frames follow...)
> (gdb) p w
> $16 = (struct window *) 0x47d22f0
> (gdb) p sw
> $17 = (struct window *) 0x47d22f0
> (gdb) p XWINDOW(selected_window)
> $18 = (struct window *) 0x61109d0
> (gdb) p w->contents
> $19 = XIL(0)
> (gdb) p XWINDOW(selected_window)->contents
> $20 = XIL(0x493e565)
> (gdb) xpr
> Lisp_Vectorlike
> PVEC_BUFFER
> $21 = (struct buffer *) 0x493e560
> (unsigned char *) 0x4a34568 " *NeoTree*"

I'm probably too silly to understand this.  IIUC we are here

  /* do_pending_window_change could change the selected_window due to
     frame resizing which makes the selected window too small.  */
  if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
    sw = w;

  /* Clear frames marked as garbaged.  */
  clear_garbaged_frames ();

  /* Build menubar and tool-bar items.  */
  if (NILP (Vmemory_full))
    prepare_menu_bars ();

  reconsider_clip_changes (w);

and the only ways this could cause w != XWINDOW(selected_window)
before the last call is either !WINDOWP (selected_window) or that
clear_garbaged_frames or prepare_menu_bars change selected_window.
Can you find the responsible?

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 11:54:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org, martin rudalics <rudalics <at> gmx.at>,
 Noam Postavsky <npostavs <at> gmail.com>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 14:52:55 +0300
On May 2, 2018 9:16:26 AM GMT+03:00, martin rudalics <rudalics <at> gmx.at> wrote:
> >> Is w in reconsider_clip_changes the same as XWINDOW
> (selected_window)
>  >> when it fails?
>  >
>  > Nope.  In fact, doing
>  >
>  >      break 13939 if (w != XWINDOW(selected_window))
>  >
>  > triggers only just before the bad reconsider_clip_changes call.
>  >
>  > #0  redisplay_internal () at ../../src/xdisp.c:13939
>  > #1  0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
> > #2  0x00000000005851ae in read_char (commandflag=1,
> map=XIL(0x5e99db3), prev_event=XIL(0),
> >      used_mouse_menu=0x7fffffffe41f, end_time=0x0) at
> ../../src/keyboard.c:2480
> > #3  0x00000000005957c5 in read_key_sequence (keybuf=0x7fffffffe570,
> bufsize=30, prompt=XIL(0),
> >      dont_downcase_last=false, can_return_switch_frame=true,
> fix_current_buffer=true,
>  >      prevent_redisplay=false) at ../../src/keyboard.c:9147
> > #4  0x0000000000581d55 in command_loop_1 () at
> ../../src/keyboard.c:1368
>  > (More stack frames follow...)
>  > (gdb) p w
>  > $16 = (struct window *) 0x47d22f0
>  > (gdb) p sw
>  > $17 = (struct window *) 0x47d22f0
>  > (gdb) p XWINDOW(selected_window)
>  > $18 = (struct window *) 0x61109d0
>  > (gdb) p w->contents
>  > $19 = XIL(0)
>  > (gdb) p XWINDOW(selected_window)->contents
>  > $20 = XIL(0x493e565)
>  > (gdb) xpr
>  > Lisp_Vectorlike
>  > PVEC_BUFFER
>  > $21 = (struct buffer *) 0x493e560
>  > (unsigned char *) 0x4a34568 " *NeoTree*"
> 
> I'm probably too silly to understand this.  IIUC we are here
> 
>    /* do_pending_window_change could change the selected_window due to
>       frame resizing which makes the selected window too small.  */
> if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) !=
> sw)
>      sw = w;
> 
>    /* Clear frames marked as garbaged.  */
>    clear_garbaged_frames ();
> 
>    /* Build menubar and tool-bar items.  */
>    if (NILP (Vmemory_full))
>      prepare_menu_bars ();
> 
>    reconsider_clip_changes (w);
> 
> and the only ways this could cause w != XWINDOW(selected_window)
> before the last call is either !WINDOWP (selected_window) or that
> clear_garbaged_frames or prepare_menu_bars change selected_window.
> Can you find the responsible?
> 
> martin

Those are exactly the questions I'd like to have answers for.

Btw, I tried to run the recipe in "emacs -Q", and it didn't crash, so I guess
something in doom-land is a necessary piece of this puzzle.

Also, please take a look at zoom.el: it hooks window-size-change-functions
and resizes the windows in that hook(!).  And prepare_menu_bars runs
window-size-change-functions, which resets the frame's
window-configuration-changed flag, assuming that no one in their right
mind will change the configuration from a hook that notifies about
changes in configuration.

After all this madness, it's small wonder that we end up with a dead
window in W.

Of course, we need to protect us against such calamities in any case...





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 11:54:04 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 12:22:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 08:21:15 -0400
[Message part 1 (text/plain, inline)]
martin rudalics <rudalics <at> gmx.at> writes:

> and the only ways this could cause w != XWINDOW(selected_window)
> before the last call is either !WINDOWP (selected_window) or that
> clear_garbaged_frames or prepare_menu_bars change selected_window.
> Can you find the responsible?

Yes, found it, seems to be a combination of zoom
window-size-change-functions called via prepare_menu_bars and doom
advice on balance-windows.  I got 12 changes of selected_window.  First
hit summarized inline, the rest of the backtraces are attached below.

#0  select_window_1 (window=XIL(0x1601c35), inhibit_point_swap=false) at ../../src/window.c:562
#1  0x00000000004aff15 in select_window (window=XIL(0x1601c35), norecord=XIL(0xc090), inhibit_point_swap=false) at ../../src/window.c:520
#2  0x00000000004b0183 in Fselect_window (window=XIL(0x1601c35), norecord=XIL(0xc090)) at ../../src/window.c:590
#3  0x00000000004beaa7 in Fdelete_window_internal (window=XIL(0x1696c35)) at ../../src/window.c:4667

#79 0x0000000000441b1a in safe_call1 (fn=XIL(0x4a9e6d0), arg=XIL(0x1600c35)) at ../../src/xdisp.c:2644
#80 0x00000000004bb27a in run_window_size_change_functions (frame=XIL(0x1600c35)) at ../../src/window.c:3457
#81 0x0000000000460409 in prepare_menu_bars () at ../../src/xdisp.c:12065
#82 0x0000000000465141 in redisplay_internal () at ../../src/xdisp.c:13937
#83 0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518

Lisp Backtrace:
[...]
"delete-window" (0xffff9ab0)
[...]
"doom/popup-close" (0xffffa068)
[...]
"doom*popups-save" (0xffffaa00)
[...]
"ad-Advice-balance-windows" (0xffffb530)
"apply" (0xffffb528)
"balance-windows" (0xffffb960)
"let" (0xffffbcc0)
"zoom--update" (0xffffbe10)
[...]
"zoom--handler" (0xffffc928)
"redisplay_internal (C function)" (0x0)

(defun zoom--on ()
  [...]
  (add-hook 'window-size-change-functions #'zoom--handler)

(defun zoom--handler (&optional window-or-frame norecord)
  [...]
    (with-selected-window
      [...]
      (zoom--update))))

(defun zoom--update ()
  "Update the window layout in the current frame."
  (let (;; temporarily disables this mode during resize to avoid infinite
        ;; recursion (probably not needed thanks to the following)
        (zoom-mode nil)
        ;; temporarily disable all (even external) hooks about window
        ;; configuration changes to try to avoid potential flickering since
        ;; `balance-windows' calls them
        (window-configuration-change-hook nil)
        ;; make sure that other windows are resized nicely after resizing the
        ;; selected one
        (window-combination-resize t)
        ;; make sure that the exact same amount of pixels is assigned to all the
        ;; siblings
        (window-resize-pixelwise t))
    ;; start from a balanced layout anyway
    (balance-windows)
    ;; check if the selected window is not ignored
    (unless (zoom--window-ignored-p)
      (zoom--resize)
      (zoom--fix-scroll))))

;; doom/core/core-popups.el:
  (advice-add #'balance-windows :around #'doom*popups-save)

(defun doom*popups-save (orig-fn &rest args)
  "Sets aside all popups before executing the original function, usually to
prevent the popup(s) from messing up the UI (or vice versa)."
  (save-popups! (apply orig-fn args)))

(defmacro save-popups! (&rest body)
  "Sets aside all popups before executing the original function, usually to
prevent the popup(s) from messing up the UI (or vice versa)."
  `(let ((in-popup-p (doom-popup-p))
         (popups (doom-popup-windows))
         (doom-popup-remember-history t)
         (doom-popup-inhibit-autokill t))
     (when popups
       (mapc #'doom/popup-close popups))
     (unwind-protect
         (progn ,@body)
       (when popups
         (let ((origin (selected-window)))
           (doom/popup-restore)
           (unless in-popup-p
             (select-window origin)))))))

[bug-31312.gdbinit.diff (text/plain, attachment)]
[selwindow.log.gz (application/gzip, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 12:46:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org, Noam Postavsky <npostavs <at> gmail.com>,
 martin rudalics <rudalics <at> gmx.at>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 15:45:31 +0300
On May 2, 2018 3:21:15 PM GMT+03:00, Noam Postavsky <npostavs <at> gmail.com> wrote:
> martin rudalics <rudalics <at> gmx.at> writes:
> 
> > and the only ways this could cause w != XWINDOW(selected_window)
> > before the last call is either !WINDOWP (selected_window) or that
> > clear_garbaged_frames or prepare_menu_bars change selected_window.
> > Can you find the responsible?
> 
> Yes, found it, seems to be a combination of zoom
> window-size-change-functions called via prepare_menu_bars and doom
> advice on balance-windows.  I got 12 changes of selected_window. 
> First
> hit summarized inline, the rest of the backtraces are attached below.
> 
> #0  select_window_1 (window=XIL(0x1601c35), inhibit_point_swap=false)
> at ../../src/window.c:562
> #1  0x00000000004aff15 in select_window (window=XIL(0x1601c35),
> norecord=XIL(0xc090), inhibit_point_swap=false) at
> ../../src/window.c:520
> #2  0x00000000004b0183 in Fselect_window (window=XIL(0x1601c35),
> norecord=XIL(0xc090)) at ../../src/window.c:590
> #3  0x00000000004beaa7 in Fdelete_window_internal
> (window=XIL(0x1696c35)) at ../../src/window.c:4667
> 
> #79 0x0000000000441b1a in safe_call1 (fn=XIL(0x4a9e6d0),
> arg=XIL(0x1600c35)) at ../../src/xdisp.c:2644
> #80 0x00000000004bb27a in run_window_size_change_functions
> (frame=XIL(0x1600c35)) at ../../src/window.c:3457
> #81 0x0000000000460409 in prepare_menu_bars () at
> ../../src/xdisp.c:12065
> #82 0x0000000000465141 in redisplay_internal () at
> ../../src/xdisp.c:13937
> #83 0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
> 
> Lisp Backtrace:
> [...]
> "delete-window" (0xffff9ab0)
> [...]
> "doom/popup-close" (0xffffa068)
> [...]
> "doom*popups-save" (0xffffaa00)
> [...]
> "ad-Advice-balance-windows" (0xffffb530)
> "apply" (0xffffb528)
> "balance-windows" (0xffffb960)
> "let" (0xffffbcc0)
> "zoom--update" (0xffffbe10)
> [...]
> "zoom--handler" (0xffffc928)
> "redisplay_internal (C function)" (0x0)
> 
> (defun zoom--on ()
>   [...]
>   (add-hook 'window-size-change-functions #'zoom--handler)
> 
> (defun zoom--handler (&optional window-or-frame norecord)
>   [...]
>     (with-selected-window
>       [...]
>       (zoom--update))))
> 
> (defun zoom--update ()
>   "Update the window layout in the current frame."
> (let (;; temporarily disables this mode during resize to avoid
> infinite
>         ;; recursion (probably not needed thanks to the following)
>         (zoom-mode nil)
>         ;; temporarily disable all (even external) hooks about window
>    ;; configuration changes to try to avoid potential flickering since
>         ;; `balance-windows' calls them
>         (window-configuration-change-hook nil)
>  ;; make sure that other windows are resized nicely after resizing the
>         ;; selected one
>         (window-combination-resize t)
> ;; make sure that the exact same amount of pixels is assigned to all
> the
>         ;; siblings
>         (window-resize-pixelwise t))
>     ;; start from a balanced layout anyway
>     (balance-windows)
>     ;; check if the selected window is not ignored
>     (unless (zoom--window-ignored-p)
>       (zoom--resize)
>       (zoom--fix-scroll))))
> 
> ;; doom/core/core-popups.el:
>   (advice-add #'balance-windows :around #'doom*popups-save)
> 
> (defun doom*popups-save (orig-fn &rest args)
> "Sets aside all popups before executing the original function, usually
> to
> prevent the popup(s) from messing up the UI (or vice versa)."
>   (save-popups! (apply orig-fn args)))
> 
> (defmacro save-popups! (&rest body)
> "Sets aside all popups before executing the original function, usually
> to
> prevent the popup(s) from messing up the UI (or vice versa)."
>   `(let ((in-popup-p (doom-popup-p))
>          (popups (doom-popup-windows))
>          (doom-popup-remember-history t)
>          (doom-popup-inhibit-autokill t))
>      (when popups
>        (mapc #'doom/popup-close popups))
>      (unwind-protect
>          (progn ,@body)
>        (when popups
>          (let ((origin (selected-window)))
>            (doom/popup-restore)
>            (unless in-popup-p
>              (select-window origin)))))))

So we need the same defense after prepare_menu_bars as we
have after do_pending_changes, I think.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 12:46:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:28:01 GMT) Full text and rfc822 format available.

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

From: Andrea Cardaci <cyrus.and <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, bug-gnu-emacs <at> gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 2 May 2018 15:27:12 +0200
[Message part 1 (text/plain, inline)]
> Also, please take a look at zoom.el: it hooks window-size-change-functions
> and resizes the windows in that hook(!).

> assuming that no one in their right
> mind will change the configuration from a hook that notifies about
> changes in configuration

Yeah, I'm that guy...

Any advice on this part? I needed to force the size of windows and hooking
`select-window` (via `advice-add`) is apparently not enough to catch all
the cases.

Is there a better way to achieve what I want?

On 2 May 2018 at 14:45, Eli Zaretskii <eliz <at> gnu.org> wrote:

> On May 2, 2018 3:21:15 PM GMT+03:00, Noam Postavsky <npostavs <at> gmail.com>
> wrote:
> > martin rudalics <rudalics <at> gmx.at> writes:
> >
> > > and the only ways this could cause w != XWINDOW(selected_window)
> > > before the last call is either !WINDOWP (selected_window) or that
> > > clear_garbaged_frames or prepare_menu_bars change selected_window.
> > > Can you find the responsible?
> >
> > Yes, found it, seems to be a combination of zoom
> > window-size-change-functions called via prepare_menu_bars and doom
> > advice on balance-windows.  I got 12 changes of selected_window.
> > First
> > hit summarized inline, the rest of the backtraces are attached below.
> >
> > #0  select_window_1 (window=XIL(0x1601c35), inhibit_point_swap=false)
> > at ../../src/window.c:562
> > #1  0x00000000004aff15 in select_window (window=XIL(0x1601c35),
> > norecord=XIL(0xc090), inhibit_point_swap=false) at
> > ../../src/window.c:520
> > #2  0x00000000004b0183 in Fselect_window (window=XIL(0x1601c35),
> > norecord=XIL(0xc090)) at ../../src/window.c:590
> > #3  0x00000000004beaa7 in Fdelete_window_internal
> > (window=XIL(0x1696c35)) at ../../src/window.c:4667
> >
> > #79 0x0000000000441b1a in safe_call1 (fn=XIL(0x4a9e6d0),
> > arg=XIL(0x1600c35)) at ../../src/xdisp.c:2644
> > #80 0x00000000004bb27a in run_window_size_change_functions
> > (frame=XIL(0x1600c35)) at ../../src/window.c:3457
> > #81 0x0000000000460409 in prepare_menu_bars () at
> > ../../src/xdisp.c:12065
> > #82 0x0000000000465141 in redisplay_internal () at
> > ../../src/xdisp.c:13937
> > #83 0x0000000000463c16 in redisplay () at ../../src/xdisp.c:13518
> >
> > Lisp Backtrace:
> > [...]
> > "delete-window" (0xffff9ab0)
> > [...]
> > "doom/popup-close" (0xffffa068)
> > [...]
> > "doom*popups-save" (0xffffaa00)
> > [...]
> > "ad-Advice-balance-windows" (0xffffb530)
> > "apply" (0xffffb528)
> > "balance-windows" (0xffffb960)
> > "let" (0xffffbcc0)
> > "zoom--update" (0xffffbe10)
> > [...]
> > "zoom--handler" (0xffffc928)
> > "redisplay_internal (C function)" (0x0)
> >
> > (defun zoom--on ()
> >   [...]
> >   (add-hook 'window-size-change-functions #'zoom--handler)
> >
> > (defun zoom--handler (&optional window-or-frame norecord)
> >   [...]
> >     (with-selected-window
> >       [...]
> >       (zoom--update))))
> >
> > (defun zoom--update ()
> >   "Update the window layout in the current frame."
> > (let (;; temporarily disables this mode during resize to avoid
> > infinite
> >         ;; recursion (probably not needed thanks to the following)
> >         (zoom-mode nil)
> >         ;; temporarily disable all (even external) hooks about window
> >    ;; configuration changes to try to avoid potential flickering since
> >         ;; `balance-windows' calls them
> >         (window-configuration-change-hook nil)
> >  ;; make sure that other windows are resized nicely after resizing the
> >         ;; selected one
> >         (window-combination-resize t)
> > ;; make sure that the exact same amount of pixels is assigned to all
> > the
> >         ;; siblings
> >         (window-resize-pixelwise t))
> >     ;; start from a balanced layout anyway
> >     (balance-windows)
> >     ;; check if the selected window is not ignored
> >     (unless (zoom--window-ignored-p)
> >       (zoom--resize)
> >       (zoom--fix-scroll))))
> >
> > ;; doom/core/core-popups.el:
> >   (advice-add #'balance-windows :around #'doom*popups-save)
> >
> > (defun doom*popups-save (orig-fn &rest args)
> > "Sets aside all popups before executing the original function, usually
> > to
> > prevent the popup(s) from messing up the UI (or vice versa)."
> >   (save-popups! (apply orig-fn args)))
> >
> > (defmacro save-popups! (&rest body)
> > "Sets aside all popups before executing the original function, usually
> > to
> > prevent the popup(s) from messing up the UI (or vice versa)."
> >   `(let ((in-popup-p (doom-popup-p))
> >          (popups (doom-popup-windows))
> >          (doom-popup-remember-history t)
> >          (doom-popup-inhibit-autokill t))
> >      (when popups
> >        (mapc #'doom/popup-close popups))
> >      (unwind-protect
> >          (progn ,@body)
> >        (when popups
> >          (let ((origin (selected-window)))
> >            (doom/popup-restore)
> >            (unless in-popup-p
> >              (select-window origin)))))))
>
> So we need the same defense after prepare_menu_bars as we
> have after do_pending_changes, I think.
>
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:28:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:43:01 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 15:42:29 +0200
> Yes, found it, seems to be a combination of zoom
> window-size-change-functions called via prepare_menu_bars and doom
> advice on balance-windows.  I got 12 changes of selected_window.  First
> hit summarized inline, the rest of the backtraces are attached below.

Magnificent, many thanks.

A first interpretation is as follows (please correct me if you think
I'm wrong): The first hit stems from a call of 'delete-window' where
'frame-first-window' apparently finds a bad window.  The remainders
but the last one come from running 'window-configuration-change-hook'
where the saved selected window after running the local part of the
hook is dead.  The last one comes from 'window-size-change-functions'
where a call of 'select-window' apparently succeeds passing us a bad
window.

The 'window-configuration-change-hook' instances are design failures -
we simply have to let the selected window alone when the saved one has
been deleted.  The other two deserve some investigation.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:44:01 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>, bug-gnu-emacs <at> gnu.org, 
 Noam Postavsky <npostavs <at> gmail.com>
Cc: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 15:42:58 +0200
> So we need the same defense after prepare_menu_bars as we
> have after do_pending_changes, I think.

If you mean something like

  if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
    sw = w;

I'm afraid that this would fail since selected_window has no buffer
any more (or may have even been recycled already).  Or am I missing
something?

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:44:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:48:02 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Andrea Cardaci <cyrus.and <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: bug-gnu-emacs <at> gnu.org, Noam Postavsky <npostavs <at> gmail.com>,
 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 15:47:32 +0200
> Any advice on this part? I needed to force the size of windows and hooking
> `select-window` (via `advice-add`) is apparently not enough to catch all
> the cases.

It might not help in the case at hand, but instead of advising
'select-window' (which won't do the job anyway when a window is
selected directly via select_window) please add your function to
'buffer-list-update-hook'.  Hooking 'select-window' _is_ bad idea,
always.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 13:49:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 15:05:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: cyrus.and <at> gmail.com, npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 18:03:45 +0300
> Date: Wed, 02 May 2018 15:42:58 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: 31312 <at> debbugs.gnu.org, Andrea Cardaci <cyrus.and <at> gmail.com>
> 
>  > So we need the same defense after prepare_menu_bars as we
>  > have after do_pending_changes, I think.
> 
> If you mean something like
> 
>    if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
>      sw = w;
> 
> I'm afraid that this would fail since selected_window has no buffer
> any more (or may have even been recycled already).

Is that a fact?  I might be mistaken, but my take on what Noam found
was that the selected window is OK, it's just that the window held in
W is dead (i.e. it was deleted inside the tempest that happened in
run_window_size_change_functions called by prepare_menu_bars).  So my
suggestion is to update W with the new selected window.

In general, if the selected window has no buffer, we are in deep
trouble, and not just in redisplay.  So I very much hope this is not
what happens here.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 15:07:02 GMT) Full text and rfc822 format available.

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

From: Andrea Cardaci <cyrus.and <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 31312 <at> debbugs.gnu.org, bug-gnu-emacs <at> gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 2 May 2018 17:06:11 +0200
[Message part 1 (text/plain, inline)]
> It might not help in the case at hand, but instead of advising
> 'select-window' (which won't do the job anyway when a window is
> selected directly via select_window) please add your function to
> 'buffer-list-update-hook'.  Hooking 'select-window' _is_ bad idea,
> always.

I will try that, even though I'm pretty sure that I already tried this
approach in the past and switched back to advising `select-window` but I
cannot recall the reason.

On 2 May 2018 at 15:47, martin rudalics <rudalics <at> gmx.at> wrote:

> > Any advice on this part? I needed to force the size of windows and
> hooking
> > `select-window` (via `advice-add`) is apparently not enough to catch all
> > the cases.
>
> It might not help in the case at hand, but instead of advising
> 'select-window' (which won't do the job anyway when a window is
> selected directly via select_window) please add your function to
> 'buffer-list-update-hook'.  Hooking 'select-window' _is_ bad idea,
> always.
>
> martin
>
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 15:07:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 15:07:03 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrea Cardaci <cyrus.and <at> gmail.com>
Cc: rudalics <at> gmx.at, npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 18:06:47 +0300
> From: Andrea Cardaci <cyrus.and <at> gmail.com>
> Date: Wed, 2 May 2018 15:27:12 +0200
> Cc: bug-gnu-emacs <at> gnu.org, Noam Postavsky <npostavs <at> gmail.com>, 
> 	martin rudalics <rudalics <at> gmx.at>, 31312 <at> debbugs.gnu.org
> 
> Any advice on this part? I needed to force the size of windows and hooking `select-window` (via `advice-add`)
> is apparently not enough to catch all the cases.
> 
> Is there a better way to achieve what I want?

If what Martin suggested somehow doesn't fit the bill, and no other
ideas are brought up, then I prefer to discuss new features or
infrastructure to satisfy similar needs, rather than force
applications to use such fragile techniques.  E.g., we could provide
some customizable controls in the code that resizes windows when
needed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 15:15:01 GMT) Full text and rfc822 format available.

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

From: Andrea Cardaci <cyrus.and <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, Noam Postavsky <npostavs <at> gmail.com>,
 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 2 May 2018 17:14:04 +0200
[Message part 1 (text/plain, inline)]
> If what Martin suggested somehow doesn't fit the bill

It doesn't, because it deals with advising `select-window` while the main
issue here is that I resize windows in the `window-size-change-functions`
hook (albeit using a guard to avoid infinite recursion) and I can't come up
any workarounds for that.

On 2 May 2018 at 17:06, Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Andrea Cardaci <cyrus.and <at> gmail.com>
> > Date: Wed, 2 May 2018 15:27:12 +0200
> > Cc: bug-gnu-emacs <at> gnu.org, Noam Postavsky <npostavs <at> gmail.com>,
> >       martin rudalics <rudalics <at> gmx.at>, 31312 <at> debbugs.gnu.org
> >
> > Any advice on this part? I needed to force the size of windows and
> hooking `select-window` (via `advice-add`)
> > is apparently not enough to catch all the cases.
> >
> > Is there a better way to achieve what I want?
>
> If what Martin suggested somehow doesn't fit the bill, and no other
> ideas are brought up, then I prefer to discuss new features or
> infrastructure to satisfy similar needs, rather than force
> applications to use such fragile techniques.  E.g., we could provide
> some customizable controls in the code that resizes windows when
> needed.
>
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 15:31:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrea Cardaci <cyrus.and <at> gmail.com>
Cc: rudalics <at> gmx.at, npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 18:30:04 +0300
> From: Andrea Cardaci <cyrus.and <at> gmail.com>
> Date: Wed, 2 May 2018 17:14:04 +0200
> Cc: Noam Postavsky <npostavs <at> gmail.com>, martin rudalics <rudalics <at> gmx.at>, 31312 <at> debbugs.gnu.org
> 
> > If what Martin suggested somehow doesn't fit the bill
> 
> It doesn't, because it deals with advising `select-window` while the main issue here is that I resize windows in
> the `window-size-change-functions` hook (albeit using a guard to avoid infinite recursion) and I can't come up
> any workarounds for that.

In that case, could you explain in more detail what zoom.el attempts
to do, and what does it need from Emacs core to be able to do that?

(It's probably better to start a new discussion on emacs-devel, not
here.)

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 18:44:01 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: cyrus.and <at> gmail.com, npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 20:43:30 +0200
>> If you mean something like
>>
>>     if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
>>       sw = w;
>>
>> I'm afraid that this would fail since selected_window has no buffer
>> any more (or may have even been recycled already).
>
> Is that a fact?  I might be mistaken, but my take on what Noam found
> was that the selected window is OK, it's just that the window held in
> W is dead (i.e. it was deleted inside the tempest that happened in
> run_window_size_change_functions called by prepare_menu_bars).  So my
> suggestion is to update W with the new selected window.

Then I misinterpreted Noam's results.  Anyway, a simple way to
reproduce the bug is

(defun foo (frame)
  (delete-window (selected-window)))

(add-hook 'window-size-change-functions 'foo)

and do C-x 2.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Wed, 02 May 2018 19:03:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: cyrus.and <at> gmail.com, npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 22:02:32 +0300
> Date: Wed, 02 May 2018 20:43:30 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org, cyrus.and <at> gmail.com
> 
> (defun foo (frame)
>    (delete-window (selected-window)))
> 
> (add-hook 'window-size-change-functions 'foo)
> 
> and do C-x 2.

Right, and as you can see, selected_window is still a valid leaf
window when we abort in this case.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Thu, 03 May 2018 00:05:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: eliz <at> gnu.org, 31312 <at> debbugs.gnu.org, cyrus.and <at> gmail.com
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Wed, 02 May 2018 20:04:28 -0400
martin rudalics <rudalics <at> gmx.at> writes:

>> So we need the same defense after prepare_menu_bars as we
>> have after do_pending_changes, I think.
>
> If you mean something like
>
>   if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
>     sw = w;
>
> I'm afraid that this would fail since selected_window has no buffer
> any more (or may have even been recycled already).  Or am I missing
> something?

It seems to work (I don't know enough about the code to explain why).  I
applied this patch:

--- i/src/xdisp.c
+++ w/src/xdisp.c
@@ -13936,6 +13936,11 @@ redisplay_internal (void)
   if (NILP (Vmemory_full))
     prepare_menu_bars ();
 
+  /* prepare_menu_bars may call lisp hooks and hence change the
+     selected_window.  */
+  if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
+    sw = w;
+
   reconsider_clip_changes (w);
 
   /* In most cases selected window displays current buffer.  */

And then following the original recipe does not segfault.  There is a
Lisp error, but I think that's already a bug in zoom and/or doom.

Debugger entered--Lisp error: (wrong-type-argument window-live-p #<window 39>)
  set-window-dedicated-p(#<window 39> t)
  neo-window--init(#<window 39> #<buffer  *NeoTree*>)
  neo-global--create-window()
  neo-global--get-window(t)
  neo-global--open-dir("/home/npostavs/src/doom/")
  neo-global--open()
  neotree-show()
  funcall-interactively(neotree-show)
  call-interactively(neotree-show record nil)
  command-execute(neotree-show record)
  #f(compiled-function (cmd) #<bytecode 0x1289c19>)("neotree-show")
  ivy-call()
  ivy-read("M-x " ("toggle-debug-on-error" "zoom-mode" [...]
  counsel-M-x()
  funcall-interactively(counsel-M-x)
  call-interactively(counsel-M-x nil nil)
  command-execute(counsel-M-x)





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Thu, 03 May 2018 02:34:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: rudalics <at> gmx.at, 31312 <at> debbugs.gnu.org, cyrus.and <at> gmail.com
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Thu, 03 May 2018 05:32:44 +0300
> From: Noam Postavsky <npostavs <at> gmail.com>
> Cc: eliz <at> gnu.org,  31312 <at> debbugs.gnu.org,  cyrus.and <at> gmail.com
> Date: Wed, 02 May 2018 20:04:28 -0400
> 
> martin rudalics <rudalics <at> gmx.at> writes:
> 
> >> So we need the same defense after prepare_menu_bars as we
> >> have after do_pending_changes, I think.
> >
> > If you mean something like
> >
> >   if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
> >     sw = w;
> >
> > I'm afraid that this would fail since selected_window has no buffer
> > any more (or may have even been recycled already).  Or am I missing
> > something?
> 
> It seems to work (I don't know enough about the code to explain why).  I
> applied this patch:
> 
> --- i/src/xdisp.c
> +++ w/src/xdisp.c
> @@ -13936,6 +13936,11 @@ redisplay_internal (void)
>    if (NILP (Vmemory_full))
>      prepare_menu_bars ();
>  
> +  /* prepare_menu_bars may call lisp hooks and hence change the
> +     selected_window.  */
> +  if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
> +    sw = w;
> +
>    reconsider_clip_changes (w);
>  
>    /* In most cases selected window displays current buffer.  */
> 
> And then following the original recipe does not segfault.  There is a
> Lisp error, but I think that's already a bug in zoom and/or doom.

Right.

Please push to the emacs-26 branch, and thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Thu, 03 May 2018 07:12:01 GMT) Full text and rfc822 format available.

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

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: cyrus.and <at> gmail.com, npostavs <at> gmail.com, 31312 <at> debbugs.gnu.org
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Thu, 03 May 2018 09:11:11 +0200
> Right, and as you can see, selected_window is still a valid leaf
> window when we abort in this case.

Yes.  I clearly misinterpreted Noam's log.

martin




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Thu, 03 May 2018 12:22:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 31312 <at> debbugs.gnu.org, cyrus.and <at> gmail.com
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Thu, 03 May 2018 08:21:34 -0400
Eli Zaretskii <eliz <at> gnu.org> writes:

> Please push to the emacs-26 branch, and thanks.

Wouldn't it make sense to remove the previous check, like this?

--- i/src/xdisp.c
+++ w/src/xdisp.c
@@ -13924,11 +13924,6 @@ redisplay_internal (void)
   /* Notice any pending interrupt request to change frame size.  */
   do_pending_window_change (true);
 
-  /* do_pending_window_change could change the selected_window due to
-     frame resizing which makes the selected window too small.  */
-  if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
-    sw = w;
-
   /* Clear frames marked as garbaged.  */
   clear_garbaged_frames ();
 
@@ -13936,6 +13931,13 @@ redisplay_internal (void)
   if (NILP (Vmemory_full))
     prepare_menu_bars ();
 
+  /* do_pending_window_change could change the selected_window due to
+     frame resizing which makes the selected window too small.
+     prepare_menu_bars may call lisp hooks and hence also change the
+     selected_window.  */
+  if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw)
+    sw = w;
+
   reconsider_clip_changes (w);
 
   /* In most cases selected window displays current buffer.  */







Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Thu, 03 May 2018 17:55:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: rudalics <at> gmx.at, 31312 <at> debbugs.gnu.org, cyrus.and <at> gmail.com
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Thu, 03 May 2018 20:54:12 +0300
> From: Noam Postavsky <npostavs <at> gmail.com>
> Cc: rudalics <at> gmx.at,  31312 <at> debbugs.gnu.org,  cyrus.and <at> gmail.com
> Date: Thu, 03 May 2018 08:21:34 -0400
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Please push to the emacs-26 branch, and thanks.
> 
> Wouldn't it make sense to remove the previous check, like this?

That's a tad more risky, but I guess we can afford that.  So please
do.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#31312; Package emacs. (Fri, 04 May 2018 01:20:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 31312 <at> debbugs.gnu.org, cyrus.and <at> gmail.com
Subject: Re: bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom
Date: Thu, 03 May 2018 21:18:49 -0400
tags 31312 fixed
close 31312 26.1
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Cc: rudalics <at> gmx.at,  31312 <at> debbugs.gnu.org,  cyrus.and <at> gmail.com
>> Date: Thu, 03 May 2018 08:21:34 -0400
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > Please push to the emacs-26 branch, and thanks.
>> 
>> Wouldn't it make sense to remove the previous check, like this?

> That's a tad more risky, but I guess we can afford that.  So please
> do.

Done.

[1: b90ce66d32]: 2018-05-03 20:58:06 -0400
  Handle selected_window change in prepare_menu_bars (Bug#31312)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b90ce66d32ede14a9191008096e596f6dfb9a48b>





Added tag(s) fixed. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Fri, 04 May 2018 01:20:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 26.1, send any further explanations to 31312 <at> debbugs.gnu.org and Andrea Cardaci <cyrus.and <at> gmail.com> Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Fri, 04 May 2018 01:20:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 01 Jun 2018 11:24:03 GMT) Full text and rfc822 format available.

This bug report was last modified 7 years and 71 days ago.

Previous Next


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