bug#31312: Segmentation fault with doom-emacs, NeoTree and Zoom

[Andrea Cardaci <cyrus.and@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4990 bytes --]

> 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@gnu.org> wrote:

> On May 2, 2018 3:21:15 PM GMT+03:00, Noam Postavsky <npostavs@gmail.com>
> wrote:
> > martin rudalics <rudalics@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.
>

[-- Attachment #2: Type: text/html, Size: 7019 bytes --]