<div dir="ltr">Hi,<div><br></div><div>Reverting this causes a 100% reproducible crash on startup in one of my CI tests for weston-terminal. Here is some of the debugging info:</div><div><br></div><div><div>Program terminated with signal SIGSEGV, Segmentation fault.</div><div>#0 0x000000000040ab93 in handle_char (terminal=0x6348a0, utf8=...) at clients/terminal.c:1991</div><div>1991<span class="Apple-tab-span" style="white-space:pre"> </span>row[terminal->column] = utf8;</div><div>Missing separate debuginfos, use: dnf debuginfo-install weston-1.12.0-1.fc25.x86_64</div><div>(gdb) bt</div><div>#0 0x000000000040ab93 in handle_char (terminal=0x6348a0, utf8=...) at clients/terminal.c:1991</div><div>#1 0x000000000040b38d in terminal_data (terminal=0x6348a0, data=0x7fffffffdb40 "sh-4.3$ ", </div><div> length=8) at clients/terminal.c:2172</div><div>#2 0x000000000040d296 in io_handler (task=0x6348c8, events=1) at clients/terminal.c:3021</div><div>#3 0x0000000000419d7f in display_run (display=0x62e8c0) at clients/window.c:6506</div><div>#4 0x000000000040d5a2 in main (argc=1, argv=0x7fffffffde98) at clients/terminal.c:3109</div><div>(gdb) p terminal->column</div><div>$1 = 0</div><div>(gdb) p row</div><div>$2 = (union utf8_char *) 0x0</div><div>(gdb) l</div><div>1986<span class="Apple-tab-span" style="white-space:pre"> </span>row = terminal_get_row(terminal, terminal->row);</div><div>1987<span class="Apple-tab-span" style="white-space:pre"> </span>attr_row = terminal_get_attr_row(terminal, terminal->row);</div><div>1988<span class="Apple-tab-span" style="white-space:pre"> </span></div><div>1989<span class="Apple-tab-span" style="white-space:pre"> </span>if (terminal->mode & MODE_IRM)</div><div>1990<span class="Apple-tab-span" style="white-space:pre"> </span>terminal_shift_line(terminal, +1);</div><div>1991<span class="Apple-tab-span" style="white-space:pre"> </span>row[terminal->column] = utf8;</div><div>1992<span class="Apple-tab-span" style="white-space:pre"> </span>attr_row[terminal->column++] = terminal->curr_attr;</div><div>1993<span class="Apple-tab-span" style="white-space:pre"> </span></div><div>1994<span class="Apple-tab-span" style="white-space:pre"> </span>if (terminal->row + terminal->start + 1 > terminal->end)</div><div>1995<span class="Apple-tab-span" style="white-space:pre"> </span>terminal->end = terminal->row + terminal->start + 1;</div><div>(gdb) p terminal->row</div><div>$3 = -1</div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 9, 2016 at 1:55 PM Quentin Glidic <<a href="mailto:sardemff7%2Bwayland@sardemff7.net">sardemff7+wayland@sardemff7.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 09/09/2016 19:02, Derek Foreman wrote:<br class="gmail_msg"> > On 31/08/16 05:56 PM, Quentin Glidic wrote:<br class="gmail_msg"> >> On 30/08/2016 04:07, Yong Bakos wrote:<br class="gmail_msg"> >>> On Aug 29, 2016, at 4:28 PM, Bryce Harrington <<a href="mailto:bryce@osg.samsung.com" class="gmail_msg" target="_blank">bryce@osg.samsung.com</a>><br class="gmail_msg"> >>> wrote:<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> weston-terminal intermittently crashes on startup. This occurs because<br class="gmail_msg"> >>>> some parameters in the weston_terminal structure such as data_pitch,<br class="gmail_msg"> >>>> don't get set to non-zero until the resize_handler() callback gets<br class="gmail_msg"> >>>> triggered. That callback makes a call to terminal_resize_cells(), to<br class="gmail_msg"> >>>> calculate the proper values for these parameters.<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> On occasion, the resize handler call is slow to resolve, and the program<br class="gmail_msg"> >>>> proceeds to start processing characters for the terminal window. With<br class="gmail_msg"> >>>> the parameters defaulting to zero, certain calculations come out wrong,<br class="gmail_msg"> >>>> leading the program to attempt to scroll the buffer when it shouldn't,<br class="gmail_msg"> >>>> and thus follows the crash.<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> Instead, force the call to terminal_resize_cells() during the init, with<br class="gmail_msg"> >>>> some dummy defaults, to ensure the parameters are always non-zero.<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> Fixes: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=97539" rel="noreferrer" class="gmail_msg" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=97539</a><br class="gmail_msg"> >>>> Signed-off-by: Bryce Harrington <<a href="mailto:bryce@osg.samsung.com" class="gmail_msg" target="_blank">bryce@osg.samsung.com</a>><br class="gmail_msg"> >>><br class="gmail_msg"> >>> I couldn't replicate the issue, but the fix seems innocuous enough.<br class="gmail_msg"> >>> Someone with more experience should chime in, but this is:<br class="gmail_msg"> >>><br class="gmail_msg"> >>> Reviewed-by: Yong Bakos <<a href="mailto:ybakos@humanoriented.com" class="gmail_msg" target="_blank">ybakos@humanoriented.com</a>><br class="gmail_msg"> >><br class="gmail_msg"> >><br class="gmail_msg"> >> Don’t see any harm having it as a quick fix. I wonder if xdg_shell_v6<br class="gmail_msg"> >> initial configure can be used as a sync point for all this?<br class="gmail_msg"> >><br class="gmail_msg"> >> Pushed:<br class="gmail_msg"> >> c6ae812..5c611d9 master -> master<br class="gmail_msg"> ><br class="gmail_msg"> > Hey, can we revert this immediately? :)<br class="gmail_msg"> > (Unless someone has a better patch for the problem - I haven't actually<br class="gmail_msg"> > seen that crash myself.)<br class="gmail_msg"> ><br class="gmail_msg"> > Turns out when you launch weston from a console with the drm backend,<br class="gmail_msg"> > then start a weston-terminal, it sets weston's controlling terminal to<br class="gmail_msg"> > be 20x5. The ioctl() at the bottom of terminal_resize_cells() appears<br class="gmail_msg"> > to be hitting weston's control terminal.<br class="gmail_msg"> ><br class="gmail_msg"> > When you exit weston you get a really great surprise.<br class="gmail_msg"> ><br class="gmail_msg"> > My system has systemd-logind, but I see it when running from<br class="gmail_msg"> > weston-launch too...<br class="gmail_msg"> <br class="gmail_msg"> Sure, reverted:<br class="gmail_msg"> 6967f0e..e30b5fb master -> master<br class="gmail_msg"> <br class="gmail_msg"> Cheers,<br class="gmail_msg"> <br class="gmail_msg"> <br class="gmail_msg"> ><br class="gmail_msg"> > Thanks,<br class="gmail_msg"> > Derek<br class="gmail_msg"> ><br class="gmail_msg"> >> Cheers,<br class="gmail_msg"> >><br class="gmail_msg"> >>><br class="gmail_msg"> >>>> ---<br class="gmail_msg"> >>>> clients/terminal.c | 1 +<br class="gmail_msg"> >>>> 1 file changed, 1 insertion(+)<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> diff --git a/clients/terminal.c b/clients/terminal.c<br class="gmail_msg"> >>>> index 6257cb7..34bc2c9 100644<br class="gmail_msg"> >>>> --- a/clients/terminal.c<br class="gmail_msg"> >>>> +++ b/clients/terminal.c<br class="gmail_msg"> >>>> @@ -2976,6 +2976,7 @@ terminal_create(struct display *display)<br class="gmail_msg"> >>>> cairo_surface_destroy(surface);<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> terminal_resize(terminal, 20, 5); /* Set minimum size first */<br class="gmail_msg"> >>>> + terminal_resize_cells(terminal, 20, 5);<br class="gmail_msg"> >>>> terminal_resize(terminal, 80, 25);<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> wl_list_insert(terminal_list.prev, &terminal->link);<br class="gmail_msg"> >>>> --<br class="gmail_msg"> >>>> 1.9.1<br class="gmail_msg"> >>>><br class="gmail_msg"> >>>> _______________________________________________<br class="gmail_msg"> >>>> wayland-devel mailing list<br class="gmail_msg"> >>>> <a href="mailto:wayland-devel@lists.freedesktop.org" class="gmail_msg" target="_blank">wayland-devel@lists.freedesktop.org</a><br class="gmail_msg"> >>>> <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" class="gmail_msg" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a><br class="gmail_msg"> >>><br class="gmail_msg"> >>> _______________________________________________<br class="gmail_msg"> >>> wayland-devel mailing list<br class="gmail_msg"> >>> <a href="mailto:wayland-devel@lists.freedesktop.org" class="gmail_msg" target="_blank">wayland-devel@lists.freedesktop.org</a><br class="gmail_msg"> >>> <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" class="gmail_msg" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a><br class="gmail_msg"> >>><br class="gmail_msg"> >><br class="gmail_msg"> >><br class="gmail_msg"> ><br class="gmail_msg"> <br class="gmail_msg"> <br class="gmail_msg"> --<br class="gmail_msg"> <br class="gmail_msg"> Quentin “Sardem FF7” Glidic<br class="gmail_msg"> _______________________________________________<br class="gmail_msg"> wayland-devel mailing list<br class="gmail_msg"> <a href="mailto:wayland-devel@lists.freedesktop.org" class="gmail_msg" target="_blank">wayland-devel@lists.freedesktop.org</a><br class="gmail_msg"> <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" class="gmail_msg" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a><br class="gmail_msg"> </blockquote></div>