all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
@ 2022-12-15 17:04 Juri Linkov
  2022-12-15 21:42 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2022-12-15 17:04 UTC (permalink / raw)
  To: 60096

0. emacs -Q
1. Evaluate:

(add-hook 'kill-buffer-hook
          (lambda ()
            (run-with-timer 0 nil (lambda () (tab-bar-select-tab 1)))))

2. Type:

C-x t 2
C-x k RET

Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
format_mode_line_unwind_data (target_frame=target_frame@entry=0x5555563c3310, obuf=<optimized out>, owin=XIL(0x55555602562d), save_proptrans=save_proptrans@entry=false) at xdisp.c:13223
13223	      current_buffer = b;
(gdb) bt
#0  format_mode_line_unwind_data (target_frame=target_frame@entry=0x5555563c3310, obuf=<optimized out>, owin=XIL(0x55555602562d), save_proptrans=save_proptrans@entry=false) at xdisp.c:13223
#1  0x0000555555600752 in gui_consider_frame_title (frame=XIL(0x5555563c3315)) at xdisp.c:13417
#2  0x0000555555605219 in prepare_menu_bars () at xdisp.c:13544
#3  redisplay_internal () at xdisp.c:16462
#4  0x00005555556ea03a in read_char (commandflag=1, map=XIL(0x555556770ea3), prev_event=XIL(0), used_mouse_menu=0x7fffffffddcb, end_time=0x0) at keyboard.c:2627
#5  0x00005555556ec658 in read_key_sequence (keybuf=<optimized out>, prompt=XIL(0), dont_downcase_last=<optimized out>, can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=<optimized out>) at keyboard.c:10074
#6  0x00005555556ee2c0 in command_loop_1 () at lisp.h:1171
#7  0x000055555576a1b7 in internal_condition_case (bfun=bfun@entry=0x5555556ee0e0 <command_loop_1>, handlers=handlers@entry=XIL(0x90), hfun=hfun@entry=0x5555556e11b0 <cmd_error>) at eval.c:1474
#8  0x00005555556d986a in command_loop_2 (handlers=handlers@entry=XIL(0x90)) at keyboard.c:1125
#9  0x000055555576a0f9 in internal_catch (tag=tag@entry=XIL(0xfa80), func=func@entry=0x5555556d9840 <command_loop_2>, arg=arg@entry=XIL(0x90)) at eval.c:1197
#10 0x00005555556d9806 in command_loop () at lisp.h:1171
#11 0x00005555556e0d08 in recursive_edit_1 () at keyboard.c:712
#12 0x00005555556e10b0 in Frecursive_edit () at keyboard.c:795
#13 0x00005555555b1a73 in main (argc=<optimized out>, argv=<optimized out>) at emacs.c:2529

Lisp Backtrace:
"redisplay_internal (C function)" (0x0)
(gdb) l
13218
13219	      /* If we select a window on another frame, make sure that that
13220		 selection does not leave its buffer's point modified when
13221		 unwinding (Bug#32777).  */
13222	      ASET (vector, 10, buffer);
13223	      current_buffer = b;
13224	      ASET (vector, 11, build_marker (current_buffer, PT, PT_BYTE));
13225	      current_buffer = cb;
13226	    }
13227
(gdb) p buffer
$3 = XIL(0)

This means that at this point the value of this is nil:

  XWINDOW (target_frame->selected_window)->contents;





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-15 17:04 bug#60096: 29.0.60; Crash in format_mode_line_unwind_data Juri Linkov
@ 2022-12-15 21:42 ` Eli Zaretskii
  2022-12-16  7:31   ` Juri Linkov
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-15 21:42 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 60096

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 15 Dec 2022 19:04:30 +0200
> 
> 0. emacs -Q
> 1. Evaluate:
> 
> (add-hook 'kill-buffer-hook
>           (lambda ()
>             (run-with-timer 0 nil (lambda () (tab-bar-select-tab 1)))))
> 
> 2. Type:
> 
> C-x t 2
> C-x k RET
> 
> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
> format_mode_line_unwind_data (target_frame=target_frame@entry=0x5555563c3310, obuf=<optimized out>, owin=XIL(0x55555602562d), save_proptrans=save_proptrans@entry=false) at xdisp.c:13223
> 13223	      current_buffer = b;

Thanks, should be fixed now on the release branch.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-15 21:42 ` Eli Zaretskii
@ 2022-12-16  7:31   ` Juri Linkov
  2022-12-16 11:46     ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2022-12-16  7:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60096-done

>> (add-hook 'kill-buffer-hook
>>           (lambda ()
>>             (run-with-timer 0 nil (lambda () (tab-bar-select-tab 1)))))
>> 
>> 2. Type:
>> 
>> C-x t 2
>> C-x k RET
>> 
>> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
>> format_mode_line_unwind_data (target_frame=target_frame@entry=0x5555563c3310, obuf=<optimized out>, owin=XIL(0x55555602562d), save_proptrans=save_proptrans@entry=false) at xdisp.c:13223
>> 13223	      current_buffer = b;
>
> Thanks, should be fixed now on the release branch.

I confirm that it's fixed, so closing.

PS: I wonder why now *scratch* shows keybindings inside
non-standard quotation marks: "C-x C-f"?  Why not 'C-x C-f'?
Or `C-x C-f'?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-16  7:31   ` Juri Linkov
@ 2022-12-16 11:46     ` Eli Zaretskii
  2022-12-16 14:39       ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-16 11:46 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 60096

> From: Juri Linkov <juri@linkov.net>
> Cc: 60096-done@debbugs.gnu.org
> Date: Fri, 16 Dec 2022 09:31:36 +0200
> 
> I confirm that it's fixed, so closing.
> 
> PS: I wonder why now *scratch* shows keybindings inside
> non-standard quotation marks: "C-x C-f"?  Why not 'C-x C-f'?
> Or `C-x C-f'?

I'm working on a better fix for this, which will revert back to how we
were showing the bindings originally.  Whether what we did originally
is the best way is a separate issue.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-16 11:46     ` Eli Zaretskii
@ 2022-12-16 14:39       ` Eli Zaretskii
  2022-12-16 15:03         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-16 14:39 UTC (permalink / raw)
  To: juri; +Cc: 60096

> Cc: 60096@debbugs.gnu.org
> Date: Fri, 16 Dec 2022 13:46:07 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Juri Linkov <juri@linkov.net>
> > Cc: 60096-done@debbugs.gnu.org
> > Date: Fri, 16 Dec 2022 09:31:36 +0200
> > 
> > I confirm that it's fixed, so closing.
> > 
> > PS: I wonder why now *scratch* shows keybindings inside
> > non-standard quotation marks: "C-x C-f"?  Why not 'C-x C-f'?
> > Or `C-x C-f'?
> 
> I'm working on a better fix for this, which will revert back to how we
> were showing the bindings originally.  Whether what we did originally
> is the best way is a separate issue.

Now done, please re-test.

Note that the *scratch* buffer is re-created in this scenario without
the blurb we put into it, and in Fundamental mode.  This is because
get-scratch-buffer-create, which is called to re-create it (because we
have no "other" buffer to switch to when the original *scratch* is
killed) signals an error when it deletes the temp buffer used by
substitute-command-keys.  This is what happens when safe C code which
can be called in situations where we literally pull ourselves by the
shoestrings, is replaced by less safe Lisp code.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-16 14:39       ` Eli Zaretskii
@ 2022-12-16 15:03         ` Eli Zaretskii
  2022-12-17  9:17           ` martin rudalics
  2022-12-17 17:23           ` Juri Linkov
  0 siblings, 2 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-16 15:03 UTC (permalink / raw)
  To: martin rudalics; +Cc: 60096, juri

> Cc: 60096@debbugs.gnu.org
> Date: Fri, 16 Dec 2022 16:39:57 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Note that the *scratch* buffer is re-created in this scenario without
> the blurb we put into it, and in Fundamental mode.

I've fixed at least the last part, so the major mode of the re-created
*scratch* is now as expected.

Martin, I'd appreciate your views on the changeset in commit
89f54e8157 and regarding the original problem to begin with.
Basically, set-window-configuration was called in a situation where it
has only one valid buffer to play with, which is tough for its logic
and the various other functions it invokes.  The result was a bunch of
bad windows with nil as their buffer.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-16 15:03         ` Eli Zaretskii
@ 2022-12-17  9:17           ` martin rudalics
  2022-12-17 10:00             ` Eli Zaretskii
  2022-12-17 17:23           ` Juri Linkov
  1 sibling, 1 reply; 17+ messages in thread
From: martin rudalics @ 2022-12-17  9:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60096, juri

 > Basically, set-window-configuration was called in a situation where it
 > has only one valid buffer to play with, which is tough for its logic
 > and the various other functions it invokes.  The result was a bunch of
 > bad windows with nil as their buffer.

If a live window has nil as its buffer, next redisplay will reliably
crash Emacs anyway.  So this

       /* We may have deleted windows above.  Then again, maybe we
	 haven't: the functions we call to maybe delete windows can
	 decide a window cannot be deleted.  Force recalculation of
	 Vwindow_list next time it is needed, to make sure stale
	 windows with no buffers don't escape into the wild, which
	 will cause crashes elsewhere.  */
       Vwindow_list = Qnil;

should not be needed.  Otherwise we'd have been in serious trouble ever
since Vwindow_list was added.  The earlier call

       delete_all_child_windows (FRAME_ROOT_WINDOW (f));

should have reliably reset Vwindow_list to nil.

martin





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17  9:17           ` martin rudalics
@ 2022-12-17 10:00             ` Eli Zaretskii
  2022-12-17 10:53               ` Eli Zaretskii
  2022-12-17 15:26               ` martin rudalics
  0 siblings, 2 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-17 10:00 UTC (permalink / raw)
  To: martin rudalics; +Cc: 60096, juri

> Date: Sat, 17 Dec 2022 10:17:00 +0100
> Cc: juri@linkov.net, 60096@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Basically, set-window-configuration was called in a situation where it
>  > has only one valid buffer to play with, which is tough for its logic
>  > and the various other functions it invokes.  The result was a bunch of
>  > bad windows with nil as their buffer.
> 
> If a live window has nil as its buffer, next redisplay will reliably
> crash Emacs anyway.  So this
> 
>        /* We may have deleted windows above.  Then again, maybe we
> 	 haven't: the functions we call to maybe delete windows can
> 	 decide a window cannot be deleted.  Force recalculation of
> 	 Vwindow_list next time it is needed, to make sure stale
> 	 windows with no buffers don't escape into the wild, which
> 	 will cause crashes elsewhere.  */
>        Vwindow_list = Qnil;
> 
> should not be needed.

It is needed in this case because it forces redisplay to recompute the
window list (in propagate_buffer_redisplay, if not earlier).  If the
above is not done, Vwindow_list will be reused, and the problematic
(bogus?) windows in it _will_ cause a crash.

> The earlier call
> 
>        delete_all_child_windows (FRAME_ROOT_WINDOW (f));
> 
> should have reliably reset Vwindow_list to nil.

AFAIR, the code which runs after that call recalculated Vwindow_list,
adding to it those windows with no buffer.  You should be able to see
that by running the simple recipe posted by Juri at the beginning of
this bug report.

What about the other parts of the changeset I installed -- do they
look okay to you? any comments?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 10:00             ` Eli Zaretskii
@ 2022-12-17 10:53               ` Eli Zaretskii
  2022-12-17 15:26               ` martin rudalics
  1 sibling, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-17 10:53 UTC (permalink / raw)
  To: rudalics; +Cc: 60096, juri

> Cc: 60096@debbugs.gnu.org, juri@linkov.net
> Date: Sat, 17 Dec 2022 12:00:47 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > The earlier call
> > 
> >        delete_all_child_windows (FRAME_ROOT_WINDOW (f));
> > 
> > should have reliably reset Vwindow_list to nil.
> 
> AFAIR, the code which runs after that call recalculated Vwindow_list,
> adding to it those windows with no buffer.

Specifically, here's what happens:

 . We call

	      wset_buffer (w, other_buffer_safely (Fcurrent_buffer ()));

 . other_buffer_safely cannot find a single buffer that satisfies the
   candidate_buffer condition, so it ends up recreating *scratch*
   (whose deletion caused this mess to begin with) by calling
   get-scratch-buffer-create in Lisp
 . get-scratch-buffer-create calls substitute-command-keys to produce
   the blurb we put in the comment at the beginning of *scratch*
 . substitute-command-keys uses a temporary buffer to format the
   message, and calls kill-buffer to delete that buffer when it's done
 . kill-buffer calls replace_buffer_in_windows, which calls
   replace-buffer-in-windows in Lisp
 . replace-buffer-in-windows calls window-list-1, which calls
   window_list, which fills Vwindow_list with windows that have no
   buffer:

    (gdb) pp Vwindow_list
    (#<window 8> #<window 4>)

 . one of these windows gets assigned a buffer, eventually, since it's
   a selected-window, but the other window stays without a buffer, and
   causes a crash in the following redisplay





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 10:00             ` Eli Zaretskii
  2022-12-17 10:53               ` Eli Zaretskii
@ 2022-12-17 15:26               ` martin rudalics
  2022-12-17 15:59                 ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: martin rudalics @ 2022-12-17 15:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60096, juri

 > It is needed in this case because it forces redisplay to recompute the
 > window list (in propagate_buffer_redisplay, if not earlier).  If the
 > above is not done, Vwindow_list will be reused, and the problematic
 > (bogus?) windows in it _will_ cause a crash.
 >
 > Specifically, here's what happens:
 >
 >  . We call
 >
 > 	      wset_buffer (w, other_buffer_safely (Fcurrent_buffer ()));
 >
 >  . other_buffer_safely cannot find a single buffer that satisfies the
 >    candidate_buffer condition, so it ends up recreating *scratch*
 >    (whose deletion caused this mess to begin with) by calling
 >    get-scratch-buffer-create in Lisp
 >  . get-scratch-buffer-create calls substitute-command-keys to produce
 >    the blurb we put in the comment at the beginning of *scratch*
 >  . substitute-command-keys uses a temporary buffer to format the
 >    message, and calls kill-buffer to delete that buffer when it's done
 >  . kill-buffer calls replace_buffer_in_windows, which calls
 >    replace-buffer-in-windows in Lisp
 >  . replace-buffer-in-windows calls window-list-1, which calls
 >    window_list, which fills Vwindow_list with windows that have no
 >    buffer:
 >
 >     (gdb) pp Vwindow_list
 >     (#<window 8> #<window 4>)
 >
 >  . one of these windows gets assigned a buffer, eventually, since it's
 >    a selected-window, but the other window stays without a buffer, and
 >    causes a crash in the following redisplay

Thanks for the explanation.  I must have tested with my own version of
'replace-buffer-in-windows' which starts with

   (let ((buffer (window-normalize-buffer buffer-or-name)))
     ;; Don't scan 'window-list-1' unless necessary (often it isn't, for
     ;; example, when killing a temporary buffer).
     (when (> (buffer-windows-count buffer) 0)
       (dolist (window (window-list-1 nil nil t))

But since there's no guarantee that a temporary buffer will not be shown
in a window temporarily, your patch is a bit safer.  Alternatively, we
could exclude windows with a nil buffer in add_window_to_list (think of
the case that within the blurb producing code someone wants to consult
the window list).  In either case, we'd be accepting a temporarily
broken basic invariant - that a live window always shows a live buffer.

Principally, we should never run 'replace-buffer-in-windows' from within
'set-window-configuration'.  That bloated window list is just the tip of
an iceberg here.

 > What about the other parts of the changeset I installed -- do they
 > look okay to you? any comments?

I see

-  return safe_call (1, Qget_scratch_buffer_create);
+  /* This function must return a valid buffer, since it is frequently
+     our last line of defense in the face of the expected buffers
+     becoming dead under our feet.  safe_call below could return nil
+     if recreating *scratch* in Lisp, which does some fancy stuff,
+     signals an error in some weird use case.  */
+  buf = safe_call (1, Qget_scratch_buffer_create);
+  if (NILP (buf))
+    {
+      AUTO_STRING (scratch, "*scratch*");
+      buf = Fget_buffer_create (scratch, Qnil);
+    }
+  return buf;

and

+      Fset_buffer_major_mode (buf);

which look okay to me.  Unless, again, the latter would try to deal with
the window list or do some other nasty stuff.  Then other_buffer_safely
should not be allowed to recreate *scratch* but rather some fallback
buffer in fundamental mode with no hooks run and any buffer lists having
it as single element.

martin





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 15:26               ` martin rudalics
@ 2022-12-17 15:59                 ` Eli Zaretskii
  2022-12-17 17:05                   ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-17 15:59 UTC (permalink / raw)
  To: martin rudalics; +Cc: 60096, juri

> Date: Sat, 17 Dec 2022 16:26:36 +0100
> Cc: juri@linkov.net, 60096@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
> Alternatively, we could exclude windows with a nil buffer in
> add_window_to_list (think of the case that within the blurb
> producing code someone wants to consult the window list).

Maybe we should try this on master.  I indeed expected
add_window_to_list to filter such invalid windows and was surprised
that it didn't.  Basically, I don't understand how we never had such
windows in the list before, since there's no code which actively
removes them and thus protects the list from holding such windows.  I
think we just have been lucky.

> Principally, we should never run 'replace-buffer-in-windows' from within
> 'set-window-configuration'.

This can no longer be guaranteed, given that other_buffer_safely calls
into Lisp, and so do a few other primitives.

>  > What about the other parts of the changeset I installed -- do they
>  > look okay to you? any comments?
> 
> I see
> 
> -  return safe_call (1, Qget_scratch_buffer_create);
> +  /* This function must return a valid buffer, since it is frequently
> +     our last line of defense in the face of the expected buffers
> +     becoming dead under our feet.  safe_call below could return nil
> +     if recreating *scratch* in Lisp, which does some fancy stuff,
> +     signals an error in some weird use case.  */
> +  buf = safe_call (1, Qget_scratch_buffer_create);
> +  if (NILP (buf))
> +    {
> +      AUTO_STRING (scratch, "*scratch*");
> +      buf = Fget_buffer_create (scratch, Qnil);
> +    }
> +  return buf;
> 
> and
> 
> +      Fset_buffer_major_mode (buf);
> 
> which look okay to me.  Unless, again, the latter would try to deal with
> the window list or do some other nasty stuff.  Then other_buffer_safely
> should not be allowed to recreate *scratch* but rather some fallback
> buffer in fundamental mode with no hooks run and any buffer lists having
> it as single element.

You are right in principle, but other_buffer_safely was doing the
above for many years before we acquired get-scratch-buffer-create and
started calling it from here.  So I think we are relatively safe
(again, maybe by pure chance).

Thanks.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 15:59                 ` Eli Zaretskii
@ 2022-12-17 17:05                   ` martin rudalics
  2022-12-17 17:52                     ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: martin rudalics @ 2022-12-17 17:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60096, juri

 >> Alternatively, we could exclude windows with a nil buffer in
 >> add_window_to_list (think of the case that within the blurb
 >> producing code someone wants to consult the window list).
 >
 > Maybe we should try this on master.  I indeed expected
 > add_window_to_list to filter such invalid windows and was surprised
 > that it didn't.  Basically, I don't understand how we never had such
 > windows in the list before, since there's no code which actively
 > removes them and thus protects the list from holding such windows.  I
 > think we just have been lucky.

Probably so far we never tried to call 'kill-buffer' from within
'set-window-configuration'.  If the only "live" window shows *scratch*,
*scratch* gets killed and we kill a temporary buffer before we were able
to recreate *scratch*, window_list will return the empty list.

 >> Principally, we should never run 'replace-buffer-in-windows' from within
 >> 'set-window-configuration'.
 >
 > This can no longer be guaranteed, given that other_buffer_safely calls
 > into Lisp, and so do a few other primitives.

What if such a call into Lisp tries to run 'set-window-configuration'?

 > You are right in principle, but other_buffer_safely was doing the
 > above for many years before we acquired get-scratch-buffer-create and
 > started calling it from here.  So I think we are relatively safe
 > (again, maybe by pure chance).

Then not calling 'get-scratch-buffer-create' from other_window_safely
would be more safe.

martin





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-16 15:03         ` Eli Zaretskii
  2022-12-17  9:17           ` martin rudalics
@ 2022-12-17 17:23           ` Juri Linkov
  2022-12-17 17:59             ` Eli Zaretskii
  1 sibling, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2022-12-17 17:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60096, martin rudalics

>> Note that the *scratch* buffer is re-created in this scenario without
>> the blurb we put into it, and in Fundamental mode.
>
> I've fixed at least the last part, so the major mode of the re-created
> *scratch* is now as expected.

Now it doesn't crash, but outputs an error in the *Messages* buffer:

  Error during redisplay: (get-scratch-buffer-create)
  signaled (wrong-type-argument window-live-p #<window 7>)





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 17:05                   ` martin rudalics
@ 2022-12-17 17:52                     ` Eli Zaretskii
  2022-12-18  9:18                       ` martin rudalics
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-17 17:52 UTC (permalink / raw)
  To: martin rudalics; +Cc: 60096, juri

> Date: Sat, 17 Dec 2022 18:05:58 +0100
> Cc: juri@linkov.net, 60096@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> Alternatively, we could exclude windows with a nil buffer in
>  >> add_window_to_list (think of the case that within the blurb
>  >> producing code someone wants to consult the window list).
>  >
>  > Maybe we should try this on master.  I indeed expected
>  > add_window_to_list to filter such invalid windows and was surprised
>  > that it didn't.  Basically, I don't understand how we never had such
>  > windows in the list before, since there's no code which actively
>  > removes them and thus protects the list from holding such windows.  I
>  > think we just have been lucky.
> 
> Probably so far we never tried to call 'kill-buffer' from within
> 'set-window-configuration'.  If the only "live" window shows *scratch*,
> *scratch* gets killed and we kill a temporary buffer before we were able
> to recreate *scratch*, window_list will return the empty list.

Why the empty list?  The buffer gets killed, but windows don't get
killed.  We still have the frame with at least two windows (including
the mini-window).  Right?

>  >> Principally, we should never run 'replace-buffer-in-windows' from within
>  >> 'set-window-configuration'.
>  >
>  > This can no longer be guaranteed, given that other_buffer_safely calls
>  > into Lisp, and so do a few other primitives.
> 
> What if such a call into Lisp tries to run 'set-window-configuration'?

Indeed.  Maybe we should protect set-window-configuration from being
re-entered?

>  > You are right in principle, but other_buffer_safely was doing the
>  > above for many years before we acquired get-scratch-buffer-create and
>  > started calling it from here.  So I think we are relatively safe
>  > (again, maybe by pure chance).
> 
> Then not calling 'get-scratch-buffer-create' from other_window_safely
> would be more safe.

You mean, return to what we did before get-scratch-buffer-create was
invented?  It's possible.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 17:23           ` Juri Linkov
@ 2022-12-17 17:59             ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-17 17:59 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 60096, rudalics

> From: Juri Linkov <juri@linkov.net>
> Cc: martin rudalics <rudalics@gmx.at>,  60096@debbugs.gnu.org
> Date: Sat, 17 Dec 2022 19:23:27 +0200
> 
> >> Note that the *scratch* buffer is re-created in this scenario without
> >> the blurb we put into it, and in Fundamental mode.
> >
> > I've fixed at least the last part, so the major mode of the re-created
> > *scratch* is now as expected.
> 
> Now it doesn't crash, but outputs an error in the *Messages* buffer:
> 
>   Error during redisplay: (get-scratch-buffer-create)
>   signaled (wrong-type-argument window-live-p #<window 7>)

Only once, right?

This is the error which (indirectly) caused the crash you reported
originally.  The fix just falls back on a safer method of re-creating
*scratch* when this error happens.

It is a very unusual situation that *scratch* needs to be re-created
when we call set-window-configuration; it is caused by the fact that
your recipe kills one of the two only buffers Emacs has to play
around (the other one being *Messages*).





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-17 17:52                     ` Eli Zaretskii
@ 2022-12-18  9:18                       ` martin rudalics
  2022-12-18 11:40                         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: martin rudalics @ 2022-12-18  9:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60096, juri

 >>   >> Alternatively, we could exclude windows with a nil buffer in
 >>   >> add_window_to_list (think of the case that within the blurb
 >>   >> producing code someone wants to consult the window list).
 >>   >
 >>   > Maybe we should try this on master.  I indeed expected
 >>   > add_window_to_list to filter such invalid windows and was surprised
 >>   > that it didn't.  Basically, I don't understand how we never had such
 >>   > windows in the list before, since there's no code which actively
 >>   > removes them and thus protects the list from holding such windows.  I
 >>   > think we just have been lucky.
 >>
 >> Probably so far we never tried to call 'kill-buffer' from within
 >> 'set-window-configuration'.  If the only "live" window shows *scratch*,
 >> *scratch* gets killed and we kill a temporary buffer before we were able
 >> to recreate *scratch*, window_list will return the empty list.
 >
 > Why the empty list?  The buffer gets killed, but windows don't get
 > killed.  We still have the frame with at least two windows (including
 > the mini-window).  Right?

Not if we exclude windows with a nil buffer as suggested above.  The
delete_all_child_windows call in 'set-window-configuration' sets the
contents field of every live window on that frame to nil and as long as
we have not been able to get a live buffer for that window, it will stay
nil.  That's where all those windows with a nil buffer in your
investigations come from.  It's simply not safe to deal with windows
before 'set-window-configuration' has done its work completely.  If we
think of running Lisp in this time, we have to do it in a completely
restricted way: Any window, including the selected one, can legitimately
have a nil buffer then.

martin





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#60096: 29.0.60; Crash in format_mode_line_unwind_data
  2022-12-18  9:18                       ` martin rudalics
@ 2022-12-18 11:40                         ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2022-12-18 11:40 UTC (permalink / raw)
  To: martin rudalics; +Cc: 60096, juri

> Date: Sun, 18 Dec 2022 10:18:18 +0100
> Cc: juri@linkov.net, 60096@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> Probably so far we never tried to call 'kill-buffer' from within
>  >> 'set-window-configuration'.  If the only "live" window shows *scratch*,
>  >> *scratch* gets killed and we kill a temporary buffer before we were able
>  >> to recreate *scratch*, window_list will return the empty list.
>  >
>  > Why the empty list?  The buffer gets killed, but windows don't get
>  > killed.  We still have the frame with at least two windows (including
>  > the mini-window).  Right?
> 
> Not if we exclude windows with a nil buffer as suggested above.  The
> delete_all_child_windows call in 'set-window-configuration' sets the
> contents field of every live window on that frame to nil and as long as
> we have not been able to get a live buffer for that window, it will stay
> nil.

Well, you forget *Messages*.  But I get your point.

> That's where all those windows with a nil buffer in your
> investigations come from.  It's simply not safe to deal with windows
> before 'set-window-configuration' has done its work completely.  If we
> think of running Lisp in this time, we have to do it in a completely
> restricted way: Any window, including the selected one, can legitimately
> have a nil buffer then.

With the current code, this is what happens: the window-related
functions called from set-window-configuration either manage to get
along with such windows, or are called via safe_call, which catches
any errors.  And AFAIU the code in set-window-configuration attempts
to make sure that every window we reinstate from the saved
window-configuration will have a valid buffer when we are done looping
over all of the saved windows.





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-12-18 11:40 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-15 17:04 bug#60096: 29.0.60; Crash in format_mode_line_unwind_data Juri Linkov
2022-12-15 21:42 ` Eli Zaretskii
2022-12-16  7:31   ` Juri Linkov
2022-12-16 11:46     ` Eli Zaretskii
2022-12-16 14:39       ` Eli Zaretskii
2022-12-16 15:03         ` Eli Zaretskii
2022-12-17  9:17           ` martin rudalics
2022-12-17 10:00             ` Eli Zaretskii
2022-12-17 10:53               ` Eli Zaretskii
2022-12-17 15:26               ` martin rudalics
2022-12-17 15:59                 ` Eli Zaretskii
2022-12-17 17:05                   ` martin rudalics
2022-12-17 17:52                     ` Eli Zaretskii
2022-12-18  9:18                       ` martin rudalics
2022-12-18 11:40                         ` Eli Zaretskii
2022-12-17 17:23           ` Juri Linkov
2022-12-17 17:59             ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.