bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Sam Brightman]

Initially discussed here:

https://emacs.stackexchange.com/questions/61088/emacs-27-1-daemon-segfaulting-when-loading-theme-as-emacsclient-connects

1. Start emacs -Q -l test.el --fg-daemon. (see test.el below)
2. Connect with emacsclient -t.
3. Suspend the first client.
4. Connect with another emacsclient -t.
5. Server crashes.

I've seen other variations, and sometimes seen many clients connect
without issue and then suddenly another one causes it to crash. This
method appears to reproduce every time. I am connecting the client
from another mate-terminal tab. It also works in the same terminal
with --bg-daemon.

test.el:

(defun my/after-make-frame-functions-hook (frame)
  (with-selected-frame frame
    (load-theme 'tango-dark t)))

(if (daemonp)
    (add-hook 'after-make-frame-functions 'my/after-make-frame-functions-hook))

Does not occur in 26.3, occurs in 27.1 or later. Does not occur if
(server-start) is used in an existing Emacs instance.

GDB:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x0000000000490e45 in load_color2 (f=f@entry=0xa1d440,
face=face@entry=0xb1e930, name=XIL(0x7ffff4d337dc),
target_index=target_index@entry=LFACE_FOREGROUND_INDEX,
color=color@entry=0x7fffffff80c0) at xfaces.c:1195
#2  0x0000000000491a6c in load_color
(target_index=LFACE_FOREGROUND_INDEX, name=<optimised out>,
face=0xb1e930, f=0xa1d440) at xfaces.c:1258
#3  map_tty_color (f=f@entry=0xa1d440, face=face@entry=0xb1e930,
idx=idx@entry=LFACE_FOREGROUND_INDEX, defaulted=<optimised out>) at
xfaces.c:6185
#4  0x0000000000491db7 in realize_tty_face (cache=0xa96c20,
attrs=0x7fffffff8130) at xfaces.c:6259
#5  realize_face (cache=cache@entry=0xa96c20,
attrs=attrs@entry=0x7fffffff8130,
former_face_id=former_face_id@entry=0) at xfaces.c:5829
#6  0x00000000004933be in realize_default_face (f=<optimised out>) at
xfaces.c:5745
#7  realize_basic_faces (f=f@entry=0xa1d440) at xfaces.c:5603
#8  0x0000000000495ae0 in update_face_from_frame_parameter
(f=f@entry=0xa1d440, param=param@entry=XIL(0x2340),
new_value=new_value@entry=XIL(0x7ffff4d337fc)) at xfaces.c:3669
#9  0x0000000000419773 in Fmodify_frame_parameters (frame=<optimised
out>, alist=<optimised out>) at frame.c:3353
#10 0x000000000050ff53 in Ffuncall (nargs=3,
args=args@entry=0x7fffffff8300) at eval.c:2806
#11 0x0000000000546958 in exec_byte_code (bytestr=XIL(0x7ffff4cbc72c),
vector=XIL(0x7ffff4cbc6cd), maxdepth=<optimised out>,
args_template=<optimised out>, nargs=nargs@entry=140737300383440,
args=<optimised out>, args@entry=0x18) at bytecode.c:632
#12 0x000000000050fc11 in fetch_and_exec_byte_code (args=0x18,
nargs=140737300383440, syms_left=<optimised out>,
fun=XIL(0x7ffff4cbc6d0)) at eval.c:2928
#13 funcall_lambda (fun=XIL(0x7ffff4cbc6d0), nargs=nargs@entry=3,
arg_vector=0x18, arg_vector@entry=0x7fffffff8488) at eval.c:3009
#14 0x000000000050fe9f in Ffuncall (nargs=4,
args=args@entry=0x7fffffff8480) at eval.c:2820
#15 0x0000000000546958 in exec_byte_code (bytestr=XIL(0x7ffff4d43e34),
vector=XIL(0x7ffff4d43ab5), maxdepth=<optimised out>,
args_template=<optimised out>, nargs=nargs@entry=140737300937400,
args=<optimised out>, args@entry=0x17) at bytecode.c:632
#16 0x000000000050fc11 in fetch_and_exec_byte_code (args=0x17,
nargs=140737300937400, syms_left=<optimised out>,
fun=XIL(0x7ffff4d43ab8)) at eval.c:2928
#17 funcall_lambda (fun=XIL(0x7ffff4d43ab8), nargs=nargs@entry=1,
arg_vector=0x17, arg_vector@entry=0x7fffffff86b8) at eval.c:3009
#18 0x000000000050fe9f in Ffuncall (nargs=2,
args=args@entry=0x7fffffff86b0) at eval.c:2820
#19 0x0000000000546958 in exec_byte_code (bytestr=XIL(0x7ffff4e983ec),
vector=XIL(0x7ffff4d43805), maxdepth=<optimised out>,
args_template=<optimised out>, nargs=nargs@entry=2, args=<optimised
out>, args@entry=0x16) at bytecode.c:632
#20 0x000000000050fc11 in fetch_and_exec_byte_code (args=0x16,
nargs=2, syms_left=<optimised out>, fun=XIL(0x7ffff4d43808)) at
eval.c:2928
#21 funcall_lambda (fun=XIL(0x7ffff4d43808), fun@entry=<error reading
variable: Cannot access memory at address 0x308>,
nargs=nargs@entry=140737488324816, arg_vector=0x16,
arg_vector@entry=0x2) at eval.c:3009
#22 0x000000000051278c in apply_lambda (fun=<error reading variable:
Cannot access memory at address 0x308>, args=<optimised out>,
count=<error reading variable: Cannot access memory at address 0x300>)
at eval.c:2953
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Lisp Backtrace:
"modify-frame-parameters" (0xffff8308)
"set-frame-parameter" (0xffff8488)
"disable-theme" (0xffff86b8)
"load-theme" (0xffff88d0)
"progn" (0xffff8a18)
"unwind-protect" (0xffff8ad8)
"let" (0xffff8bf8)
"my/after-make-frame-functions-hook" (0xffff8e70)
"run-hook-with-args" (0xffff8e68)
"make-frame" (0xffff9130)
"server-create-tty-frame" (0xffff93c8)
"server-process-filter" (0xffff9948)

(gdb) p *f
$5 = {header = {size = 4611686018578567187}, name = 0xad1f44,
icon_name = 0x0, title = 0x0, focus_frame = 0x0, root_window =
0xa1dab5, selected_window = 0xa1dab5, old_selected_window = 0xa1dab5,
minibuffer_window = 0xa8dc65, param_alist = 0xbb8f43, scroll_bars =
0x0, condemned_scroll_bars = 0x0,
menu_bar_items = 0xaa622d, face_alist = 0xa7ada3, menu_bar_vector =
0x0, buffer_predicate = 0x0, buffer_list = 0xb77bd3,
buried_buffer_list = 0x0, tab_bar_items = 0x0, tool_bar_items = 0x0,
face_cache = 0xa96c20, last_tab_bar_item = 0, menu_bar_items_used = 0,
current_pool = 0x99f060, desired_pool = 0x99efa0,
desired_matrix = 0xacbf50, current_matrix = 0xad7200,
glyphs_initialized_p = true, resized_p = false, default_face_done_p =
false, already_hscrolled_p = false, updated_p = true, fonts_changed =
false, cursor_type_changed = false, redisplay = true, visible = 0,
iconified = false, garbaged = true,
wants_modeline = true, auto_raise = false, auto_lower = false,
no_split = false, explicit_name = false, window_change = false,
window_state_change = false, mouse_moved = false, pointer_invisible =
false, frozen_window_starts = false, output_method = output_termcap,
new_pixelwise = false,
can_set_window_size = true, after_make_frame = true,
tab_bar_redisplayed = false, tab_bar_resized = false,
tool_bar_redisplayed = false, tool_bar_resized = false,
inhibit_horizontal_resize = false, inhibit_vertical_resize = false,
face_change = false, inhibit_clear_image_cache = false, change_stamp =
2,
number_of_windows = 2, tab_bar_lines = 0, tab_bar_height = 0,
n_tab_bar_rows = 0, n_tab_bar_items = 0, tool_bar_lines = 0,
tool_bar_height = 0, n_tool_bar_rows = 0, n_tool_bar_items = 0,
decode_mode_spec_buffer = 0xaebbd0 '-' <repeats 200 times>...,
insert_line_cost = 0xaecad0, delete_line_cost = 0xaecef0,
insert_n_lines_cost = 0xaecd90, delete_n_lines_cost = 0xaecc30,
text_cols = 317, text_lines = 84, total_cols = 317, total_lines = 85,
text_width = 317, text_height = 84, new_width = 0, new_height = 0,
left_pos = 0, top_pos = 0, pixel_width = 317, pixel_height = 85,
win_gravity = 0, size_hint_flags = 0,
border_width = 0, internal_border_width = 0, right_divider_width = 0,
bottom_divider_width = 0, left_fringe_width = 0, right_fringe_width =
0, fringe_cols = 0, menu_bar_lines = 1, menu_bar_height = 1,
column_width = 1, line_height = 1, terminal = 0xa1d130, output_data =
{tty = 0x99eee0, x = 0x99eee0,
w32 = 0x99eee0, ns = 0x99eee0}, font_driver_list = 0x0, desired_cursor
= FILLED_BOX_CURSOR, cursor_width = 0, blink_off_cursor =
FILLED_BOX_CURSOR, blink_off_cursor_width = 0, config_scroll_bar_width
= 0, config_scroll_bar_cols = 0, config_scroll_bar_height = 0,
config_scroll_bar_lines = 0,
cost_calculation_baud_rate = 38400, alpha = {0, 0}, gamma = 0,
extra_line_spacing = 0, background_pixel = 18446744073709551613,
foreground_pixel = 18446744073709551614}
(gdb) pv f
No symbol "intern" in current context.
(gdb) p f
$2 = (struct frame *) 0xa1d440
(gdb) pr
#<INVALID_LISP_OBJECT 0x00a1d440>
(gdb) pp f
#<INVALID_LISP_OBJECT 0x00a1d440>
(gdb) p FRAME_TERMINAL(f)->defined_color_hook
$3 = (_Bool (*)(struct frame *, const char *, Emacs_Color *, _Bool, _Bool)) 0x0
(gdb) l
1190   || target_index == LFACE_STRIKE_THROUGH_INDEX
1191   || target_index == LFACE_BOX_INDEX);
1192
1193  /* if the color map is full, defined_color_hook will return a best match
1194     to the values in an existing cell. */
1195  if (!FRAME_TERMINAL (f)->defined_color_hook
1196      (f, SSDATA (name), color, true, true))
1197    {
1198      add_to_log ("Unable to load color \"%s\"", name);
1199

It seems like the frame is invalid even though it's running after
frame creation? I am not familiar with Emacs internals at all.

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu)
of 2020-10-09 built on sbrightmanlinux
Repository revision: 5824c209ba17b97978519ea62478c57010311e88
Repository branch: master
System Description: Ubuntu 16.04.6 LTS

Configured using:
'configure --with-x-toolkit=no --without-x --with-gnutls'

Configured features:
SOUND NOTIFY INOTIFY GNUTLS LIBXML2 ZLIB XIM MODULES THREADS JSON
PDUMPER

Important settings:
value of $LC_MONETARY: en_GB.UTF-8
value of $LC_NUMERIC: en_GB.UTF-8
value of $LC_TIME: en_GB.UTF-8
value of $LANG: en_GB.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
electric-indent-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow regexp-opt sort mail-extr apropos emacsbug message rmc puny
dired dired-loaddefs rfc822 mml easymenu mml-sec epa derived epg
epg-config gnus-util rmail tool-bar rmail-loaddefs auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json map
text-property-search time-date subr-x seq mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs cl-lib
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tango-dark-theme term/xterm xterm byte-opt gv bytecomp byte-compile
cconv server tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads inotify multi-tty
make-network-process emacs)

Memory information:
((conses 16 57553 8763)
(symbols 48 7127 1)
(strings 32 19314 2014)
(string-bytes 1 643593)
(vectors 16 10666)
(vector-slots 8 131757 7149)
(floats 8 29 251)
(intervals 56 174 0)
(buffers 992 12)
(heap 1024 4552 749))

-- 
Sam Brightman

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Eli Zaretskii]

> From: Sam Brightman <sam.brightman@gmail.com>
> Date: Fri, 9 Oct 2020 16:58:38 +0100
> 
> 1. Start emacs -Q -l test.el --fg-daemon. (see test.el below)
> 2. Connect with emacsclient -t.
> 3. Suspend the first client.
> 4. Connect with another emacsclient -t.
> 5. Server crashes.
> 
> I've seen other variations, and sometimes seen many clients connect
> without issue and then suddenly another one causes it to crash. This
> method appears to reproduce every time. I am connecting the client
> from another mate-terminal tab. It also works in the same terminal
> with --bg-daemon.
> 
> test.el:
> 
> (defun my/after-make-frame-functions-hook (frame)
>   (with-selected-frame frame
>     (load-theme 'tango-dark t)))
> 
> (if (daemonp)
>     (add-hook 'after-make-frame-functions 'my/after-make-frame-functions-hook))

This after-make-frame-function makes no sense: a theme is turned on or
off globally, so you don't need to do that for every frame, just for
the first one.

Of course, Emacs shouldn't crash, so I have now made a change on the
emacs-27 branch which should simply signal an error in this case (it
is possible you will not see the error message, just see the 2nd
client exit immediately).

(In general, when a TTY frame is suspended, doing anything that
iterates over all the frames might produce surprising results.)

Thanks.

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Sam Brightman]

That patch indeed prevents a crash but the second client exiting seems
a bit strong/unhelpful - how would you suggest changing the theme more
appropriately?

I believe code of this type was introduced because new frames were not
getting themed properly:

https://stackoverflow.com/questions/18904529/after-emacs-deamon-i-can-not-see-new-theme-in-emacsclient-frame-it-works-fr

Some of the later answers indeed try to avoid repeatedly loading the
theme. However - even removing that hook - it is now impossible to
load a theme when a client is suspended (which is basically always for
me - I'll have half a dozen or more clients open at any time, one of
them will be suspended).

On Fri, 9 Oct 2020 at 20:32, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Sam Brightman <sam.brightman@gmail.com>
> > Date: Fri, 9 Oct 2020 16:58:38 +0100
> >
> > 1. Start emacs -Q -l test.el --fg-daemon. (see test.el below)
> > 2. Connect with emacsclient -t.
> > 3. Suspend the first client.
> > 4. Connect with another emacsclient -t.
> > 5. Server crashes.
> >
> > I've seen other variations, and sometimes seen many clients connect
> > without issue and then suddenly another one causes it to crash. This
> > method appears to reproduce every time. I am connecting the client
> > from another mate-terminal tab. It also works in the same terminal
> > with --bg-daemon.
> >
> > test.el:
> >
> > (defun my/after-make-frame-functions-hook (frame)
> >   (with-selected-frame frame
> >     (load-theme 'tango-dark t)))
> >
> > (if (daemonp)
> >     (add-hook 'after-make-frame-functions 'my/after-make-frame-functions-hook))
>
> This after-make-frame-function makes no sense: a theme is turned on or
> off globally, so you don't need to do that for every frame, just for
> the first one.
>
> Of course, Emacs shouldn't crash, so I have now made a change on the
> emacs-27 branch which should simply signal an error in this case (it
> is possible you will not see the error message, just see the 2nd
> client exit immediately).
>
> (In general, when a TTY frame is suspended, doing anything that
> iterates over all the frames might produce surprising results.)
>
> Thanks.



-- 
sam brightman

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Eli Zaretskii]

> From: Sam Brightman <sam.brightman@gmail.com>
> Date: Sat, 10 Oct 2020 08:09:56 +0100
> Cc: 43886@debbugs.gnu.org
> 
> That patch indeed prevents a crash but the second client exiting seems
> a bit strong/unhelpful - how would you suggest changing the theme more
> appropriately?

Check for the theme being already enabled?  Introduce a variable that
is set non-nil when the theme is first enabled?

> However - even removing that hook - it is now impossible to
> load a theme when a client is suspended (which is basically always for
> me - I'll have half a dozen or more clients open at any time, one of
> them will be suspended).

It is indeed impossible/unsafe to load a theme when some frame is
suspended, because suspending a frame makes it unable to handle the
change of the faces.  I don't see any way around that, because
skipping such a frame when a theme is enabled would mean that frame's
faces are not recalculated to obey the theme, which is a subtler and
nastier bug, IMO.

You will have to unsuspend the frames when you load a theme.

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Eli Zaretskii]

> Date: Sat, 10 Oct 2020 10:21:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 43886@debbugs.gnu.org
> 
> > However - even removing that hook - it is now impossible to
> > load a theme when a client is suspended (which is basically always for
> > me - I'll have half a dozen or more clients open at any time, one of
> > them will be suspended).
> 
> It is indeed impossible/unsafe to load a theme when some frame is
> suspended, because suspending a frame makes it unable to handle the
> change of the faces.  I don't see any way around that, because
> skipping such a frame when a theme is enabled would mean that frame's
> faces are not recalculated to obey the theme, which is a subtler and
> nastier bug, IMO.
> 
> You will have to unsuspend the frames when you load a theme.

Upon further thought, I found a simpler, nicer solution for this
conundrum, and pushed it to the emacs-27 branch.

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Sam Brightman]

Yes, that seems much better.

On Sat, 10 Oct 2020 at 09:17, Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Sat, 10 Oct 2020 10:21:44 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: 43886@debbugs.gnu.org
> >
> > > However - even removing that hook - it is now impossible to
> > > load a theme when a client is suspended (which is basically always for
> > > me - I'll have half a dozen or more clients open at any time, one of
> > > them will be suspended).
> >
> > It is indeed impossible/unsafe to load a theme when some frame is
> > suspended, because suspending a frame makes it unable to handle the
> > change of the faces.  I don't see any way around that, because
> > skipping such a frame when a theme is enabled would mean that frame's
> > faces are not recalculated to obey the theme, which is a subtler and
> > nastier bug, IMO.
> >
> > You will have to unsuspend the frames when you load a theme.
>
> Upon further thought, I found a simpler, nicer solution for this
> conundrum, and pushed it to the emacs-27 branch.



-- 
sam brightman

bug#43886: 28.0.50; daemon segfaults when loading theme as second emacsclient connects

[Eli Zaretskii]

> From: Sam Brightman <sam.brightman@gmail.com>
> Date: Sat, 10 Oct 2020 14:27:02 +0100
> Cc: 43886@debbugs.gnu.org
> 
> Yes, that seems much better.

Thanks, I'm therefore closing this bug report.