debuggers.hg

changeset 21186:d005aa895b5a

xenstore,libxl: cleanup of xenstore connections across fork()

Provide a new function xs_daemon_destroy_postfork which can be called
by a libxenstore user who has called fork, to close the fd for the
connection to xenstored and free the memory, without trying to do
anything to any threads which libxenstore may have created.

Use this new function in libxl_fork, to avoid accidental use of a
xenstore connection in both parent and child.

Also, fix the doc comment for libxl_spawn_spawn to have the success
return codes the right way round.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
author Keir Fraser <keir.fraser@citrix.com>
date Mon Apr 12 17:41:58 2010 +0100 (2010-04-12)
parents 86e82ab8d4de
children e226618aa715
files tools/libxl/libxl.c tools/libxl/libxl.h tools/libxl/libxl_internal.h tools/libxl/libxl_utils.c tools/xenstore/xs.c tools/xenstore/xs.h
line diff
     1.1 --- a/tools/libxl/libxl.c	Mon Apr 12 17:41:05 2010 +0100
     1.2 +++ b/tools/libxl/libxl.c	Mon Apr 12 17:41:58 2010 +0100
     1.3 @@ -66,7 +66,7 @@ int libxl_ctx_free(struct libxl_ctx *ctx
     1.4      libxl_free_all(ctx);
     1.5      free(ctx->alloc_ptrs);
     1.6      xc_interface_close(ctx->xch);
     1.7 -    xs_daemon_close(ctx->xsh); 
     1.8 +    if (ctx->xsh) xs_daemon_close(ctx->xsh); 
     1.9      return 0;
    1.10  }
    1.11  
     2.1 --- a/tools/libxl/libxl.h	Mon Apr 12 17:41:05 2010 +0100
     2.2 +++ b/tools/libxl/libxl.h	Mon Apr 12 17:41:58 2010 +0100
     2.3 @@ -250,6 +250,7 @@ enum {
     2.4  int libxl_ctx_init(struct libxl_ctx *ctx, int version);
     2.5  int libxl_ctx_free(struct libxl_ctx *ctx);
     2.6  int libxl_ctx_set_log(struct libxl_ctx *ctx, libxl_log_callback log_callback, void *log_data);
     2.7 +int libxl_ctx_postfork(struct libxl_ctx *ctx);
     2.8  
     2.9  /* domain related functions */
    2.10  int libxl_domain_make(struct libxl_ctx *ctx, libxl_domain_create_info *info, uint32_t *domid);
     3.1 --- a/tools/libxl/libxl_internal.h	Mon Apr 12 17:41:05 2010 +0100
     3.2 +++ b/tools/libxl/libxl_internal.h	Mon Apr 12 17:41:58 2010 +0100
     3.3 @@ -185,8 +185,8 @@ int libxl_spawn_spawn(struct libxl_ctx *
     3.4                        void (*intermediate_hook)(void *for_spawn, pid_t innerchild));
     3.5    /* Logs errors.  A copy of "what" is taken.  Return values:
     3.6     *  < 0   error, for_spawn need not be detached
     3.7 -   *   +1   caller is now the inner child, should probably call libxl_exec
     3.8 -   *    0   caller is the parent, must call detach on *for_spawn eventually
     3.9 +   *   +1   caller is the parent, must call detach on *for_spawn eventually
    3.10 +   *    0   caller is now the inner child, should probably call libxl_exec
    3.11     * Caller, may pass 0 for for_spawn, in which case no need to detach.
    3.12     */
    3.13  int libxl_spawn_detach(struct libxl_ctx *ctx,
     4.1 --- a/tools/libxl/libxl_utils.c	Mon Apr 12 17:41:05 2010 +0100
     4.2 +++ b/tools/libxl/libxl_utils.c	Mon Apr 12 17:41:58 2010 +0100
     4.3 @@ -282,6 +282,13 @@ READ_WRITE_EXACTLY(read, 1, /* */)
     4.4  READ_WRITE_EXACTLY(write, 0, const)
     4.5  
     4.6  
     4.7 +int libxl_ctx_postfork(struct libxl_ctx *ctx) {
     4.8 +    if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
     4.9 +    ctx->xsh = xs_daemon_open();
    4.10 +    if (!ctx->xsh) return ERROR_FAIL;
    4.11 +    return 0;
    4.12 +}
    4.13 +
    4.14  pid_t libxl_fork(struct libxl_ctx *ctx)
    4.15  {
    4.16      pid_t pid;
    4.17 @@ -292,6 +299,14 @@ pid_t libxl_fork(struct libxl_ctx *ctx)
    4.18          return -1;
    4.19      }
    4.20  
    4.21 +    if (!pid) {
    4.22 +        if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
    4.23 +        ctx->xsh = 0;
    4.24 +        /* This ensures that anyone who forks but doesn't exec,
    4.25 +         * and doesn't reinitialise the libxl_ctx, is OK.
    4.26 +         * It also means they can safely call libxl_ctx_free. */
    4.27 +    }
    4.28 +
    4.29      return pid;
    4.30  }
    4.31  
     5.1 --- a/tools/xenstore/xs.c	Mon Apr 12 17:41:05 2010 +0100
     5.2 +++ b/tools/xenstore/xs.c	Mon Apr 12 17:41:58 2010 +0100
     5.3 @@ -223,10 +223,39 @@ struct xs_handle *xs_domain_open(void)
     5.4  	return get_handle(xs_domain_dev());
     5.5  }
     5.6  
     5.7 +static void close_free_msgs(struct xs_handle *h) {
     5.8 +	struct xs_stored_msg *msg, *tmsg;
     5.9 +
    5.10 +	list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
    5.11 +		free(msg->body);
    5.12 +		free(msg);
    5.13 +	}
    5.14 +
    5.15 +	list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
    5.16 +		free(msg->body);
    5.17 +		free(msg);
    5.18 +	}
    5.19 +}
    5.20 +
    5.21 +static void close_fds_free(struct xs_handle *h) {
    5.22 +	if (h->watch_pipe[0] != -1) {
    5.23 +		close(h->watch_pipe[0]);
    5.24 +		close(h->watch_pipe[1]);
    5.25 +	}
    5.26 +
    5.27 +        close(h->fd);
    5.28 +        
    5.29 +	free(h);
    5.30 +}
    5.31 +
    5.32 +void xs_daemon_destroy_postfork(struct xs_handle *h)
    5.33 +{
    5.34 +        close_free_msgs(h);
    5.35 +        close_fds_free(h);
    5.36 +}
    5.37 +
    5.38  void xs_daemon_close(struct xs_handle *h)
    5.39  {
    5.40 -	struct xs_stored_msg *msg, *tmsg;
    5.41 -
    5.42  	mutex_lock(&h->request_mutex);
    5.43  	mutex_lock(&h->reply_mutex);
    5.44  	mutex_lock(&h->watch_mutex);
    5.45 @@ -239,28 +268,13 @@ void xs_daemon_close(struct xs_handle *h
    5.46  	}
    5.47  #endif
    5.48  
    5.49 -	list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
    5.50 -		free(msg->body);
    5.51 -		free(msg);
    5.52 -	}
    5.53 -
    5.54 -	list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
    5.55 -		free(msg->body);
    5.56 -		free(msg);
    5.57 -	}
    5.58 +        close_free_msgs(h);
    5.59  
    5.60  	mutex_unlock(&h->request_mutex);
    5.61  	mutex_unlock(&h->reply_mutex);
    5.62  	mutex_unlock(&h->watch_mutex);
    5.63  
    5.64 -	if (h->watch_pipe[0] != -1) {
    5.65 -		close(h->watch_pipe[0]);
    5.66 -		close(h->watch_pipe[1]);
    5.67 -	}
    5.68 -
    5.69 -	close(h->fd);
    5.70 -
    5.71 -	free(h);
    5.72 +        close_fds_free(h);
    5.73  }
    5.74  
    5.75  static bool read_all(int fd, void *data, unsigned int len)
     6.1 --- a/tools/xenstore/xs.h	Mon Apr 12 17:41:05 2010 +0100
     6.2 +++ b/tools/xenstore/xs.h	Mon Apr 12 17:41:58 2010 +0100
     6.3 @@ -48,6 +48,9 @@ struct xs_handle *xs_daemon_open_readonl
     6.4  /* Close the connection to the xs daemon. */
     6.5  void xs_daemon_close(struct xs_handle *);
     6.6  
     6.7 +/* Throw away the connection to the xs daemon, for use after fork(). */
     6.8 +void xs_daemon_destroy_postfork(struct xs_handle *);
     6.9 +
    6.10  /* Get contents of a directory.
    6.11   * Returns a malloced array: call free() on it after use.
    6.12   * Num indicates size.