lib: refactor memstats logging, fix ACTIVEATEXIT

Move the various destinations handling into lib/memory.c, include
"normal" logging as target, and make `ACTIVEATEXIT` properly non-error
as it was intended to be.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/lib/libfrr.c b/lib/libfrr.c
index 00ee57d8aad8581d1dca29ecb8573c044d32fefe..b9b70f60f73aa1c63aa432b1e875df7440473ac4 100644 (file)
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -1245,10 +1245,6 @@ void frr_early_fini(void)
 
 void frr_fini(void)
 {
-       FILE *fp;
-       char filename[128];
-       int have_leftovers = 0;
-
        hook_call(frr_fini);
 
        vty_terminate();
@@ -1289,26 +1285,7 @@ void frr_fini(void)
 
        frrmod_terminate();
 
-       /* also log memstats to stderr when stderr goes to a file*/
-       if (debug_memstats_at_exit || !isatty(STDERR_FILENO))
-               have_leftovers = log_memstats(stderr, di->name);
-
-       /* in case we decide at runtime that we want exit-memstats for
-        * a daemon
-        * (only do this if we actually have something to print though)
-        */
-       if (!debug_memstats_at_exit || !have_leftovers)
-               return;
-
-       snprintf(filename, sizeof(filename), "/tmp/frr-memstats-%s-%llu-%llu",
-                di->name, (unsigned long long)getpid(),
-                (unsigned long long)time(NULL));
-
-       fp = fopen(filename, "w");
-       if (fp) {
-               log_memstats(fp, di->name);
-               fclose(fp);
-       }
+       log_memstats(di->name, debug_memstats_at_exit);
 }
 
 struct json_object *frr_daemon_state_load(void)

diff --git a/lib/log.c b/lib/log.c
index 04b789b5da5d622da91d17ffab0575340b3ede6b..2b049cebe41e1631125748b55022d515bae22682 100644 (file)
--- a/lib/log.c
+++ b/lib/log.c
@@ -306,7 +306,7 @@ void memory_oom(size_t size, const char *name)
             "out of memory: failed to allocate %zu bytes for %s object",
             size, name);
        zlog_backtrace(LOG_CRIT);
-       log_memstats(stderr, "log");
+       log_memstats(zlog_progname, true);
        abort();
 }
 

diff --git a/lib/memory.c b/lib/memory.c
index ac39516edd75c58e203729970942dfc60a756ebe..9da46bdb8995c3156f03c1983c2953ed3552ee65 100644 (file)
--- a/lib/memory.c
+++ b/lib/memory.c
@@ -148,35 +148,108 @@ int qmem_walk(qmem_walk_fn *func, void *arg)
 }
 
 struct exit_dump_args {
-       FILE *fp;
-       const char *prefix;
+       const char *daemon_name;
+       bool do_log;
+       bool do_file;
+       bool do_stderr;
        int error;
+       FILE *fp;
+       struct memgroup *last_mg;
 };
 
+static void qmem_exit_fopen(struct exit_dump_args *eda)
+{
+       char filename[128];
+
+       if (eda->fp || !eda->do_file || !eda->daemon_name)
+               return;
+
+       snprintf(filename, sizeof(filename), "/tmp/frr-memstats-%s-%llu-%llu", eda->daemon_name,
+                (unsigned long long)getpid(), (unsigned long long)time(NULL));
+       eda->fp = fopen(filename, "w");
+
+       if (!eda->fp) {
+               zlog_err("failed to open memstats dump file %pSQq: %m", filename);
+               /* don't try opening file over and over again */
+               eda->do_file = false;
+       }
+}
+
 static int qmem_exit_walker(void *arg, struct memgroup *mg, struct memtype *mt)
 {
        struct exit_dump_args *eda = arg;
+       const char *prefix = eda->daemon_name ?: "NONE";
+       char size[32];
+
+       if (!mt)
+               /* iterator calls mg=X, mt=NULL first */
+               return 0;
 
-       if (!mt) {
-               fprintf(eda->fp,
-                       "%s: showing active allocations in memory group %s\n",
-                       eda->prefix, mg->name);
+       if (!mt->n_alloc)
+               return 0;
 
-       } else if (mt->n_alloc) {
-               char size[32];
-               if (!mg->active_at_exit)
-                       eda->error++;
+       if (mt->size != SIZE_VAR)
                snprintf(size, sizeof(size), "%10zu", mt->size);
-               fprintf(eda->fp, "%s: memstats:  %-30s: %6zu * %s\n",
-                       eda->prefix, mt->name, mt->n_alloc,
-                       mt->size == SIZE_VAR ? "(variably sized)" : size);
+       else
+               snprintf(size, sizeof(size), "(variably sized)");
+
+       if (mg->active_at_exit) {
+               /* not an error - this memgroup has allocations remain active
+                * at exit.  Only printed to zlog_debug.
+                */
+               if (!eda->do_log)
+                       return 0;
+
+               if (eda->last_mg != mg) {
+                       zlog_debug("showing active allocations in memory group %s (not an error)",
+                                  mg->name);
+                       eda->last_mg = mg;
+               }
+               zlog_debug("memstats:  %-30s: %6zu * %s", mt->name, mt->n_alloc, size);
+               return 0;
        }
+
+       eda->error++;
+       if (eda->do_file)
+               qmem_exit_fopen(eda);
+
+       if (eda->last_mg != mg) {
+               if (eda->do_log)
+                       zlog_warn("showing active allocations in memory group %s", mg->name);
+               if (eda->do_stderr)
+                       fprintf(stderr, "%s: showing active allocations in memory group %s\n",
+                               prefix, mg->name);
+               if (eda->fp)
+                       fprintf(eda->fp, "%s: showing active allocations in memory group %s\n",
+                               prefix, mg->name);
+               eda->last_mg = mg;
+       }
+
+       if (eda->do_log)
+               zlog_warn("memstats:  %-30s: %6zu * %s", mt->name, mt->n_alloc, size);
+       if (eda->do_stderr)
+               fprintf(stderr, "%s: memstats:  %-30s: %6zu * %s\n", prefix, mt->name, mt->n_alloc,
+                       size);
+       if (eda->fp)
+               fprintf(eda->fp, "%s: memstats:  %-30s: %6zu * %s\n", prefix, mt->name, mt->n_alloc,
+                       size);
        return 0;
 }
 
-int log_memstats(FILE *fp, const char *prefix)
+int log_memstats(const char *daemon_name, bool enabled)
 {
-       struct exit_dump_args eda = {.fp = fp, .prefix = prefix, .error = 0};
+       struct exit_dump_args eda = {
+               .daemon_name = daemon_name,
+               .do_log = enabled,
+               .do_file = enabled,
+               .do_stderr = enabled || !isatty(STDERR_FILENO),
+               .error = 0,
+       };
+
        qmem_walk(qmem_exit_walker, &eda);
+       if (eda.fp)
+               fclose(eda.fp);
+       if (eda.error && eda.do_log)
+               zlog_warn("exiting with %d leaked MTYPEs", eda.error);
        return eda.error;
 }

diff --git a/lib/memory.h b/lib/memory.h
index 957a9611b548baad40bcb4b40edea2212065d89c..865801883295c6318eac19926106de528f5f25a2 100644 (file)
--- a/lib/memory.h
+++ b/lib/memory.h
@@ -181,8 +181,7 @@ static inline size_t mtype_stats_alloc(struct memtype *mt)
  * last value from qmem_walk_fn. */
 typedef int qmem_walk_fn(void *arg, struct memgroup *mg, struct memtype *mt);
 extern int qmem_walk(qmem_walk_fn *func, void *arg);
-extern int log_memstats(FILE *fp, const char *);
-#define log_memstats_stderr(prefix) log_memstats(stderr, prefix)
+extern int log_memstats(const char *daemon_name, bool enabled);
 
 extern __attribute__((__noreturn__)) void memory_oom(size_t size,
                                                     const char *name);

diff --git a/tests/isisd/test_fuzz_isis_tlv.c b/tests/isisd/test_fuzz_isis_tlv.c
index 627ccfee6fa37be57d2c8c7c26789d294ecf1d07..a3acd0786f61f2cb6a06fe67db5806e92b7c8ca6 100644 (file)
--- a/tests/isisd/test_fuzz_isis_tlv.c
+++ b/tests/isisd/test_fuzz_isis_tlv.c
@@ -22,7 +22,7 @@ static bool atexit_registered;
 
 static void show_meminfo_at_exit(void)
 {
-       log_memstats(stderr, "isis fuzztest");
+       log_memstats(NULL, true);
 }
 
 static int comp_line(const void *p1, const void *p2)

diff --git a/tests/isisd/test_isis_spf.c b/tests/isisd/test_isis_spf.c
index e5a8f7a5139220cc8960a2dea610b9662f9c067d..9c1ce3d193b5a7fcb3d1d596ff822b1bb4dc7b3b 100644 (file)
--- a/tests/isisd/test_isis_spf.c
+++ b/tests/isisd/test_isis_spf.c
@@ -475,7 +475,7 @@ static void vty_do_exit(int isexit)
        yang_terminate();
        event_master_free(master);
 
-       log_memstats(stderr, "test-isis-spf");
+       log_memstats(NULL, true);
        if (!isexit)
                exit(0);
 }

diff --git a/tests/lib/cli/common_cli.c b/tests/lib/cli/common_cli.c
index 342a91cc79a70e1651ae20648629b7796caf4951..5c23a712582d10f5d9c8a313fe99b698d628ccff 100644 (file)
--- a/tests/lib/cli/common_cli.c
+++ b/tests/lib/cli/common_cli.c
@@ -43,7 +43,7 @@ static void vty_do_exit(int isexit)
        yang_terminate();
        event_master_free(master);
 
-       log_memstats(stderr, "testcli");
+       log_memstats(NULL, true);
        if (!isexit)
                exit(0);
 }

diff --git a/tests/lib/northbound/test_oper_data.c b/tests/lib/northbound/test_oper_data.c
index fdc9e53ca3eaf38fba16fdcf81a4103005fc6098..3d700d8a19b12d58115318c4313997e466a4f346 100644 (file)
--- a/tests/lib/northbound/test_oper_data.c
+++ b/tests/lib/northbound/test_oper_data.c
@@ -427,7 +427,7 @@ static void vty_do_exit(int isexit)
        yang_terminate();
        event_master_free(master);
 
-       log_memstats(stderr, "test-nb-oper-data");
+       log_memstats(NULL, true);
        if (!isexit)
                exit(0);
 }

diff --git a/tests/lib/test_typelist.c b/tests/lib/test_typelist.c
index 070a30433573f6d65f7cb50170f9cae952e71be2..3ce6683ae584eb15f40426a77244c1ebb1e4bddb 100644 (file)
--- a/tests/lib/test_typelist.c
+++ b/tests/lib/test_typelist.c
@@ -156,6 +156,6 @@ int main(int argc, char **argv)
        test_ATOMSORT_UNIQ();
        test_ATOMSORT_NONUNIQ();
 
-       log_memstats_stderr("test: ");
+       log_memstats(NULL, true);
        return 0;
 }

diff --git a/tests/lib/test_zmq.c b/tests/lib/test_zmq.c
index 2cd9d47cb4d4854503de8eb1715392d977df0e9e..5cb518d81a87924cfa5aedfdb504e25d6737d4b2 100644 (file)
--- a/tests/lib/test_zmq.c
+++ b/tests/lib/test_zmq.c
@@ -285,7 +285,7 @@ static void run_server(int syncfd)
        zmq_close(zmqsock);
        frrzmq_finish();
        event_master_free(master);
-       log_memstats_stderr("test");
+       log_memstats(NULL, true);
 }
 
 int main(void)