[lxc-devel] [PATCH] fix fd leak in test-concurrent
S.Çağlar Onur
caglar at 10ur.org
Sun Mar 9 02:14:42 UTC 2014
Hey Dwight,
On Fri, Mar 7, 2014 at 6:48 PM, Dwight Engen <dwight.engen at oracle.com> wrote:
> On Fri, 7 Mar 2014 16:12:23 -0600
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
>
>> 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?
>
> I don't think it does. I thought a little about trying to do the close
> automatically internal to lxc on the last ref in lxc_container_put()
> but that doesn't really make sense. I guess that's what seems weird to
> me, that opening the log is via a per-container api, but logging is
> really per-process (or per-thread with Caglar's recent change).
Just wondering, why do you think closing it automatically is not a good idea?
>> > 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
>> >
>> >
>
--
S.Çağlar Onur <caglar at 10ur.org>
More information about the lxc-devel
mailing list