[PATCH weston] terminal: Fix crash due to race condition in init

[Derek Foreman]

On 31/08/16 05:56 PM, Quentin Glidic wrote:
> On 30/08/2016 04:07, Yong Bakos wrote:
>> On Aug 29, 2016, at 4:28 PM, Bryce Harrington <bryce at osg.samsung.com>
>> wrote:
>>>
>>> weston-terminal intermittently crashes on startup.  This occurs because
>>> some parameters in the weston_terminal structure such as data_pitch,
>>> don't get set to non-zero until the resize_handler() callback gets
>>> triggered.  That callback makes a call to terminal_resize_cells(), to
>>> calculate the proper values for these parameters.
>>>
>>> On occasion, the resize handler call is slow to resolve, and the program
>>> proceeds to start processing characters for the terminal window.  With
>>> the parameters defaulting to zero, certain calculations come out wrong,
>>> leading the program to attempt to scroll the buffer when it shouldn't,
>>> and thus follows the crash.
>>>
>>> Instead, force the call to terminal_resize_cells() during the init, with
>>> some dummy defaults, to ensure the parameters are always non-zero.
>>>
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=97539
>>> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
>>
>> I couldn't replicate the issue, but the fix seems innocuous enough.
>> Someone with more experience should chime in, but this is:
>>
>> Reviewed-by: Yong Bakos <ybakos at humanoriented.com>
> 
> 
> Don’t see any harm having it as a quick fix. I wonder if xdg_shell_v6
> initial configure can be used as a sync point for all this?
> 
> Pushed:
> c6ae812..5c611d9  master -> master

Hey, can we revert this immediately? :)
(Unless someone has a better patch for the problem - I haven't actually
seen that crash myself.)

Turns out when you launch weston from a console with the drm backend,
then start a weston-terminal, it sets weston's controlling terminal to
be 20x5.  The ioctl() at the bottom of terminal_resize_cells() appears
to be hitting weston's control terminal.

When you exit weston you get a really great surprise.

My system has systemd-logind, but I see it when running from
weston-launch too...

Thanks,
Derek

> Cheers,
> 
>>
>>> ---
>>> clients/terminal.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/clients/terminal.c b/clients/terminal.c
>>> index 6257cb7..34bc2c9 100644
>>> --- a/clients/terminal.c
>>> +++ b/clients/terminal.c
>>> @@ -2976,6 +2976,7 @@ terminal_create(struct display *display)
>>>     cairo_surface_destroy(surface);
>>>
>>>     terminal_resize(terminal, 20, 5); /* Set minimum size first */
>>> +    terminal_resize_cells(terminal, 20, 5);
>>>     terminal_resize(terminal, 80, 25);
>>>
>>>     wl_list_insert(terminal_list.prev, &terminal->link);
>>> -- 
>>> 1.9.1
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
> 
>