[lxc-devel] [PATCH] fix fd leak in test-concurrent

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> Opening a debug log for every thread at every iteration of test-concurrent
> causes it to quickly run out of fd's because this fd is leaked. Fix this
> by adding a new api: lxc_log_close().
> 
> As Caglar noted, the log handling is in general a bit "interesting" because
> a logfile can be opened through the per-container api
> c->set_config_item("lxc.logfile") but lxc_log_fd is now per-thread data. It
> just so happens in test-concurrent that there is a 1:1 mapping of threads
> to logfiles.

Will at_exit work at thread exit?

> Split out getting debug logs from quiet since I think they are useful
> separately. If debug is specified, get a log of any mode, not just during
> start.
> 
> Signed-off-by: Dwight Engen <dwight.engen at oracle.com>

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

> ---
>  src/lxc/log.c          | 10 ++++++++++
>  src/lxc/lxccontainer.h |  5 +++++
>  src/tests/concurrent.c | 19 ++++++++++++++-----
>  3 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lxc/log.c b/src/lxc/log.c
> index a9f98a4..d5b862e 100644
> --- a/src/lxc/log.c
> +++ b/src/lxc/log.c
> @@ -368,6 +368,16 @@ extern int lxc_log_init(const char *name, const char *file,
>  	return ret;
>  }
>  
> +extern void lxc_log_close(void)
> +{
> +	if (lxc_log_fd == -1)
> +		return;
> +	close(lxc_log_fd);
> +	lxc_log_fd = -1;
> +	free(log_fname);
> +	log_fname = NULL;
> +}
> +
>  /*
>   * This is called when we read a lxc.loglevel entry in a lxc.conf file.  This
>   * happens after processing command line arguments, which override the .conf
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index b9873eb..ba15ab7 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -865,6 +865,11 @@ int list_active_containers(const char *lxcpath, char ***names, struct lxc_contai
>   */
>  int list_all_containers(const char *lxcpath, char ***names, struct lxc_container ***cret);
>  
> +/*!
> + * \brief Close log file.
> + */
> +void lxc_log_close(void);
> +
>  #ifdef  __cplusplus
>  }
>  #endif
> diff --git a/src/tests/concurrent.c b/src/tests/concurrent.c
> index c4d2ad5..acabbed 100644
> --- a/src/tests/concurrent.c
> +++ b/src/tests/concurrent.c
> @@ -29,6 +29,7 @@
>  
>  static int nthreads = 5;
>  static int iterations = 1;
> +static int debug = 0;
>  static int quiet = 0;
>  static int delay = 0;
>  static const char *template = "busybox";
> @@ -40,6 +41,7 @@ static const struct option options[] = {
>      { "delay",       required_argument, NULL, 'd' },
>      { "modes",       required_argument, NULL, 'm' },
>      { "quiet",       no_argument,       NULL, 'q' },
> +    { "debug",       no_argument,       NULL, 'D' },
>      { "help",        no_argument,       NULL, '?' },
>      { 0, 0, 0, 0 },
>  };
> @@ -54,6 +56,7 @@ static void usage(void) {
>          "  -d, --delay=N                Delay in seconds between start and stop\n"
>          "  -m, --modes=<mode,mode,...>  Modes to run (create, start, stop, destroy)\n"
>          "  -q, --quiet                  Don't produce any output\n"
> +        "  -D, --debug                  Create a debug log\n"
>          "  -?, --help                   Give this help list\n"
>          "\n"
>          "Mandatory or optional arguments to long options are also mandatory or optional\n"
> @@ -81,6 +84,11 @@ static void do_function(void *arguments)
>          return;
>      }
>  
> +    if (debug) {
> +	c->set_config_item(c, "lxc.loglevel", "DEBUG");
> +	c->set_config_item(c, "lxc.logfile", name);
> +    }
> +
>      if (strcmp(args->mode, "create") == 0) {
>          if (!c->is_defined(c)) {
>              if (!c->create(c, template, NULL, NULL, 1, NULL)) {
> @@ -91,10 +99,6 @@ static void do_function(void *arguments)
>      } else if(strcmp(args->mode, "start") == 0) {
>          if (c->is_defined(c) && !c->is_running(c)) {
>              c->want_daemonize(c, true);
> -            if (!quiet) {
> -                c->set_config_item(c, "lxc.loglevel", "DEBUG");
> -                c->set_config_item(c, "lxc.logfile", name);
> -            }
>              if (!c->start(c, false, NULL)) {
>                  fprintf(stderr, "Starting the container (%s) failed...\n", name);
>                  goto out;
> @@ -127,6 +131,8 @@ static void do_function(void *arguments)
>      args->return_code = 0;
>  out:
>      lxc_container_put(c);
> +    if (debug)
> +	lxc_log_close();
>  }
>  
>  static void *concurrent(void *arguments)
> @@ -148,7 +154,7 @@ int main(int argc, char *argv[]) {
>  
>      pthread_attr_init(&attr);
>  
> -    while ((opt = getopt_long(argc, argv, "j:i:t:d:m:q", options, NULL)) != -1) {
> +    while ((opt = getopt_long(argc, argv, "j:i:t:d:m:qD", options, NULL)) != -1) {
>          switch(opt) {
>          case 'j':
>              nthreads = atoi(optarg);
> @@ -165,6 +171,9 @@ int main(int argc, char *argv[]) {
>          case 'q':
>              quiet = 1;
>              break;
> +	case 'D':
> +	    debug = 1;
> +	    break;
>          case 'm': {
>              char *mode_tok, *tok, *saveptr = NULL;
>  
> -- 
> 1.8.5.3
> 
>