[PATCH] desktop-shell: No longer segfault on cleanup

[Derek Foreman]

That's the intent, but the patch as is introduces a new bug - now we can
have a double free somewhere else. :(

Removing the notifier from the list in the sigchld handler may be a
better solution (a quick test appears ok), but I think there's still a
tiny window where sigchld can fire just before the notifier is set up.

I think we probably need to stop calling launch_desktop_shell_process()
from inside a signal handler...

The short description of the problem is:
weston_client_launch() returns successfully (forked ok)
we add a destroy listener for the new client
sigchld fires when the forked process can't exec
the sigchld handler starts over at weston_client_launch()
we add the same destroy listener struct (still on a list!) to a new list

Now two linked lists share pointers to that listener...

at some point later the destroy notification fires and walks a broken
linked list resulting in a segfault.

On 22/08/14 05:47 PM, Eoff, Ullysses A wrote:
> Does this fix https://bugs.freedesktop.org/show_bug.cgi?id=82957 ?
> 
> ----
> U. Artie 
> 
>> -----Original Message-----
>> From: wayland-devel [mailto:wayland-devel-bounces at lists.freedesktop.org] On Behalf Of Derek Foreman
>> Sent: Friday, August 22, 2014 2:20 PM
>> To: wayland-devel at lists.freedesktop.org
>> Cc: Derek Foreman
>> Subject: [PATCH] desktop-shell: No longer segfault on cleanup
>>
>> When desktop-shell's sigchld handler attempts to re-launch
>> weston-desktop-shell it also registers a destroy notifier.
>>
>> This destroy notifier may still be on another linked list - their does not
>> appear to be any guarantee that the destroy hander will have been called
>> before the cleanup callback.
>>
>> So, when the cleanup callback is run before the destroy notifications are
>> emitted, the list will become corrupted, causing a segfault when the
>> notifications are emit.
>>
>> Since the destroy notifier just sets a pointer to NULL - which will happen
>> in the cleanup handler anyway - I've simply removed the destroy handler.
>>
>> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>> ---
>>  desktop-shell/shell.c | 16 ----------------
>>  desktop-shell/shell.h |  1 -
>>  2 files changed, 17 deletions(-)
>>
>> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
>> index 99f3343..07700cf 100644
>> --- a/desktop-shell/shell.c
>> +++ b/desktop-shell/shell.c
>> @@ -5322,17 +5322,6 @@ desktop_shell_sigchld(struct weston_process *process, int status)
>>  }
>>
>>  static void
>> -desktop_shell_client_destroy(struct wl_listener *listener, void *data)
>> -{
>> -	struct desktop_shell *shell;
>> -
>> -	shell = container_of(listener, struct desktop_shell,
>> -			     child.client_destroy_listener);
>> -
>> -	shell->child.client = NULL;
>> -}
>> -
>> -static void
>>  launch_desktop_shell_process(void *data)
>>  {
>>  	struct desktop_shell *shell = data;
>> @@ -5344,11 +5333,6 @@ launch_desktop_shell_process(void *data)
>>
>>  	if (!shell->child.client)
>>  		weston_log("not able to start %s\n", shell->client);
>> -
>> -	shell->child.client_destroy_listener.notify =
>> -		desktop_shell_client_destroy;
>> -	wl_client_add_destroy_listener(shell->child.client,
>> -				       &shell->child.client_destroy_listener);
>>  }
>>
>>  static void
>> diff --git a/desktop-shell/shell.h b/desktop-shell/shell.h
>> index c6ea328..3ddaa89 100644
>> --- a/desktop-shell/shell.h
>> +++ b/desktop-shell/shell.h
>> @@ -137,7 +137,6 @@ struct desktop_shell {
>>  		struct weston_process process;
>>  		struct wl_client *client;
>>  		struct wl_resource *desktop_shell;
>> -		struct wl_listener client_destroy_listener;
>>
>>  		unsigned deathcount;
>>  		uint32_t deathstamp;
>> --
>> 2.1.0.rc1
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>