[PATCH weston 1/3] compositor: add weston_client_start()

[Derek Foreman]

On 27/08/14 05:41 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> weston_client_start() is a new wrapper around weston_client_launch(),
> that does the process tracking on its own, and logs the process exit
> status.
> 
> When users of weston_client_start() want to know when the process exits,
> they should hook into the wl_client destroy signal. This works for cases
> where the client is not expected to disconnect without exiting.
> 
> As wl_client destructor and the sigchld handler run in arbitary order,
> it is usually difficult for users to maintain both struct weston_process
> and a struct wl_client pointer. You would need to wait for both
> destructor and handler to have run, before attempting to respawn the
> client.
> 
> This new function relieves the caller from the burden of maintaining the
> struct weston_process, assuming the caller is only interested in client
> disconnects.
> 
> Cc: Boyan Ding <stu_dby at 126.com>
> Cc: Derek Foreman <derekf at osg.samsung.com>
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  src/compositor.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/compositor.h |  3 +++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 96e3435..c62077c 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -319,6 +319,66 @@ weston_client_launch(struct weston_compositor *compositor,
>  	return client;
>  }
>  
> +struct process_info {
> +	struct weston_process proc;
> +	char *path;
> +};
> +
> +static void
> +process_handle_sigchld(struct weston_process *process, int status)
> +{
> +	struct process_info *pinfo =
> +		container_of(process, struct process_info, proc);
> +
> +	/*
> +	 * There are no guarantees whether this runs before or after
> +	 * the wl_client destructor.
> +	 */
> +
> +	if (WIFEXITED(status)) {
> +		weston_log("%s exited with status %d\n", pinfo->path,
> +			   WEXITSTATUS(status));

If I might be immensely pedantic, this may in fact only indicate that
the process that was going to exec pinfo->path failed.  So, the log
message may indicate that lala/foo exited with status 255 when lala/foo
was actually ENOENT.

I find that to be a little confusing, but I can get past it if nobody
else minds.  :)


> +	} else if (WIFSIGNALED(status)) {
> +		weston_log("%s died on signal %d\n", pinfo->path,
> +			   WTERMSIG(status));
> +	} else {
> +		weston_log("%s disappeared\n", pinfo->path);
> +	}
> +
> +	free(pinfo->path);
> +	free(pinfo);
> +}
> +
> +WL_EXPORT struct wl_client *
> +weston_client_start(struct weston_compositor *compositor, const char *path)
> +{
> +	struct process_info *pinfo;
> +	struct wl_client *client;
> +
> +	pinfo = zalloc(sizeof *pinfo);
> +	if (!pinfo)
> +		return NULL;
> +
> +	pinfo->path = strdup(path);
> +	if (!pinfo->path)
> +		goto out_free;
> +
> +	client = weston_client_launch(compositor, &pinfo->proc, path,
> +				      process_handle_sigchld);
> +	if (!client)
> +		goto out_str;
> +
> +	return client;
> +
> +out_str:
> +	free(pinfo->path);
> +
> +out_free:
> +	free(pinfo);
> +
> +	return NULL;
> +}
> +
>  static void
>  region_init_infinite(pixman_region32_t *region)
>  {
> diff --git a/src/compositor.h b/src/compositor.h
> index c0fc0a6..24a9768 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -1345,6 +1345,9 @@ weston_client_launch(struct weston_compositor *compositor,
>  		     const char *path,
>  		     weston_process_cleanup_func_t cleanup);
>  
> +struct wl_client *
> +weston_client_start(struct weston_compositor *compositor, const char *path);
> +
>  void
>  weston_watch_process(struct weston_process *process);
>  
>