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

[Quentin Glidic]

On 09/09/2016 19:02, Derek Foreman wrote:
> 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...

Sure, reverted:
6967f0e..e30b5fb  master -> master

Cheers,


>
> 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
>>>
>>
>>
>


-- 

Quentin “Sardem FF7” Glidic