[lxc-devel] [PATCH] fix fd leak in test-concurrent
S.Çağlar Onur
caglar at 10ur.org
Sun Mar 9 02:29:49 UTC 2014
Hey Dwight,
On Sat, Mar 8, 2014 at 9:14 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
> 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?
I realized the issue just after writing this, so please forget this question :)
>>> > 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>
--
S.Çağlar Onur <caglar at 10ur.org>
More information about the lxc-devel
mailing list