[multipathd] memory and threading audit

Some leaks plugged.

Still, there is one left.
You can trigger it by submitting successive "add map $map" command to
the CLI. Each time RSS/VSZ take 32 units more.

Please help on this one.

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 2f9285b..d8777cf 100644 (file)
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -623,7 +623,7 @@ dm_mapname(int major, int minor)
                goto bad;
        }
 
-       response = strdup(dm_task_get_name(dmt));
+       response = STRDUP((char *)dm_task_get_name(dmt));
        dm_task_destroy(dmt);
        return response;
 bad:

diff --git a/libmultipath/memory.c b/libmultipath/memory.c
index e846b87..71dfe40 100644 (file)
--- a/libmultipath/memory.c
+++ b/libmultipath/memory.c
@@ -133,6 +133,49 @@ dbg_malloc(unsigned long size, char *file, char *function, int line)
        return buf;
 }
 
+char *
+dbg_strdup(char *str, char *file, char *function, int line)
+{
+       void *buf;
+       int i = 0;
+       long check;
+       long size;
+
+       size = strlen(str) + 1;
+       buf = zalloc(size + sizeof (long));
+       strcat(buf, str);
+
+       check = 0xa5a5 + size;
+       *(long *) ((char *) buf + size) = check;
+
+       while (i < number_alloc_list) {
+               if (alloc_list[i].type == 0)
+                       break;
+               i++;
+       }
+
+       if (i == number_alloc_list)
+               number_alloc_list++;
+
+       assert(number_alloc_list < MAX_ALLOC_LIST);
+
+       alloc_list[i].ptr = buf;
+       alloc_list[i].size = size;
+       alloc_list[i].file = file;
+       alloc_list[i].func = function;
+       alloc_list[i].line = line;
+       alloc_list[i].csum = check;
+       alloc_list[i].type = 9;
+
+       if (debug & 1)
+               printf("strdup[%3d:%3d], %p, %4ld at %s, %3d, %s\n",
+                      i, number_alloc_list, buf, size, file, line,
+                      function);
+
+       n++;
+       return buf;
+}
+
 
 
 /* Display a buffer into a HEXA formated output */

diff --git a/libmultipath/memory.h b/libmultipath/memory.h
index 55b58bb..ab98094 100644 (file)
--- a/libmultipath/memory.h
+++ b/libmultipath/memory.h
@@ -51,11 +51,14 @@ int debug;
                       (__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
 #define REALLOC(b,n) ( dbg_realloc((b), (n), \
                       (__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
+#define STRDUP(n)    ( dbg_strdup((n), \
+                      (__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
 
 /* Memory debug prototypes defs */
 extern char *dbg_malloc(unsigned long, char *, char *, int);
 extern int dbg_free(void *, char *, char *, int);
 extern void *dbg_realloc(void *, unsigned long, char *, char *, int);
+extern char *dbg_strdup(char *, char *, char *, int);
 extern void dbg_free_final(char *);
 
 #else
@@ -63,6 +66,7 @@ extern void dbg_free_final(char *);
 #define MALLOC(n)    (zalloc(n))
 #define FREE(p)      (xfree(p))
 #define REALLOC(p,n) (realloc((p),(n)))
+#define STRDUP(n)    (strdup(n))
 
 #endif
 

diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 82761d8..2c1c38b 100644 (file)
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -31,7 +31,7 @@ free_path (struct path * pp)
                return;
 
        if (pp->checker_context)
-               FREE(pp->checker_context);
+               free(pp->checker_context);
 
        if (pp->fd > 0)
                close(pp->fd);

diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index d0223b0..a529557 100644 (file)
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -88,8 +88,10 @@ vector_del_slot(vector v, int slot)
 
        v->allocated -= VECTOR_DEFAULT_SIZE;
 
-       if (!v->allocated)
+       if (!v->allocated) {
+               FREE(v->slot);
                v->slot = NULL;
+       }
        else
                v = REALLOC(v->slot, sizeof (void *) * v->allocated);
 }
@@ -129,8 +131,8 @@ free_strvec(vector strvec)
        if (!strvec)
                return;
 
-       for (i = 0; i < VECTOR_SIZE(strvec); i++)
-               if ((str = VECTOR_SLOT(strvec, i)) != NULL)
+       vector_foreach_slot (strvec, str, i)
+               if (str)
                        FREE(str);
 
        vector_free(strvec);

diff --git a/multipathd/cli.c b/multipathd/cli.c
index ef5e461..1bc62f3 100644 (file)
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -28,7 +28,7 @@ add_key (vector vec, char * str, int code, int has_param)
 
        kw->code = code;
        kw->has_param = has_param;
-       kw->str = strdup(str);
+       kw->str = STRDUP(str);
 
        if (!kw->str)
                goto out;
@@ -81,16 +81,28 @@ free_key (struct key * kw)
        FREE(kw);
 }
 
-static void
+void
 free_keys (vector vec)
 {
        int i;
        struct key * kw;
 
-       vector_foreach_slot(vec, kw, i)
+       vector_foreach_slot (vec, kw, i)
                free_key(kw);
 
-       FREE(vec);
+       vector_free(vec);
+}
+
+void
+free_handlers (vector vec)
+{
+       int i;
+       struct handler * h;
+
+       vector_foreach_slot (vec, h, i)
+               FREE(h);
+
+       vector_free(vec);
 }
 
 int
@@ -117,6 +129,7 @@ load_keys (void)
 
        if (r) {
                free_keys(keys);
+               keys = NULL;
                return 1;
        }
 

diff --git a/multipathd/cli.h b/multipathd/cli.h
index 4b39ada..486ea39 100644 (file)
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -30,3 +30,5 @@ int add_handler (int fp, char * (*fn)(void *, void *));
 char * parse_cmd (char * cmd, void *);
 int load_keys (void);
 char * get_keyparam (vector v, int code);
+void free_keys (vector vec);
+void free_handlers (vector vec);

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index d740a47..8526714 100644 (file)
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -31,7 +31,7 @@ add_path (void * v, void * data)
        if (uev_add_path(param, allpaths))
                return NULL;
 
-       return strdup("ok");
+       return STRDUP("ok");
 }
 
 char *
@@ -43,7 +43,7 @@ del_path (void * v, void * data)
        if (uev_remove_path(param, allpaths))
                return NULL;
 
-       return strdup("ok");
+       return STRDUP("ok");
 }
 
 char *
@@ -55,7 +55,7 @@ add_map (void * v, void * data)
        if (uev_add_map(param, allpaths))
                return NULL;
 
-       return strdup("ok");
+       return STRDUP("ok");
 }
 
 char *
@@ -67,7 +67,7 @@ del_map (void * v, void * data)
        if (uev_remove_map(param, allpaths))
                return NULL;
 
-       return strdup("ok");
+       return STRDUP("ok");
 }
 
 char *
@@ -78,5 +78,5 @@ switch_group(void * v, void * data)
        
        dm_switchgroup(mapname, groupnum);
 
-       return strdup("ok");
+       return STRDUP("ok");
 }

diff --git a/multipathd/log.c b/multipathd/log.c
index 121d77c..90015a4 100644 (file)
--- a/multipathd/log.c
+++ b/multipathd/log.c
@@ -4,6 +4,8 @@
 #include <string.h>
 #include <syslog.h>
 
+#include <memory.h>
+
 #include "log.h"
 
 #define ALIGN(len, s) (((len)+(s)-1)/(s)*(s))
@@ -32,7 +34,7 @@ static void dump_logarea (void)
 static int logarea_init (int size)
 {
        logdbg(stderr,"enter logarea_init\n");
-       la = malloc(sizeof(struct logarea));
+       la = (struct logarea *)MALLOC(sizeof(struct logarea));
        
        if (!la)
                return 1;
@@ -40,11 +42,11 @@ static int logarea_init (int size)
        if (size < MAX_MSG_SIZE)
                size = DEFAULT_AREA_SIZE;
 
-       la->start = malloc(size);
+       la->start = MALLOC(size);
        memset(la->start, 0, size);
 
        if (!la->start) {
-               free(la);
+               FREE(la);
                return 1;
        }
 
@@ -53,11 +55,11 @@ static int logarea_init (int size)
        la->head = la->start;
        la->tail = la->start;
 
-       la->buff = malloc(MAX_MSG_SIZE + sizeof(struct logmsg));
+       la->buff = MALLOC(MAX_MSG_SIZE + sizeof(struct logmsg));
 
        if (!la->buff) {
-               free(la->start);
-               free(la);
+               FREE(la->start);
+               FREE(la);
                return 1;
        }
        return 0;
@@ -77,9 +79,9 @@ int log_init(char *program_name, int size)
 
 void free_logarea (void)
 {
-       free(la->start);
-       free(la->buff);
-       free(la);
+       FREE(la->start);
+       FREE(la->buff);
+       FREE(la);
        return;
 }
 

diff --git a/multipathd/log_pthread.c b/multipathd/log_pthread.c
index 0b9a606..3ac13a7 100644 (file)
--- a/multipathd/log_pthread.c
+++ b/multipathd/log_pthread.c
@@ -4,6 +4,8 @@
 #include <pthread.h>
 #include <sys/mman.h>
 
+#include <memory.h>
+
 #include "log_pthread.h"
 #include "log.h"
 

diff --git a/multipathd/main.c b/multipathd/main.c
index 5933bb5..e2a3c90 100644 (file)
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -81,6 +81,9 @@
 #define unlock(a) pthread_mutex_unlock(a)
 #endif
 
+pthread_cond_t exit_cond = PTHREAD_COND_INITIALIZER;
+pthread_mutex_t exit_mutex = PTHREAD_MUTEX_INITIALIZER;
+
 /*
  * structs
  */
@@ -104,6 +107,12 @@ alloc_waiter (void)
 }
 
 static void
+cleanup_lock (void * data)
+{
+       unlock((pthread_mutex_t *)data);
+}
+
+static void
 set_paths_owner (struct paths * allpaths, struct multipath * mpp)
 {
        int i;
@@ -167,6 +176,24 @@ update_multipath_status (struct multipath *mpp)
 static int
 update_multipath_strings (struct multipath *mpp, vector pathvec)
 {
+       if (mpp->selector) {
+               FREE(mpp->selector);
+               mpp->selector = NULL;
+       }
+
+       if (mpp->features) {
+               FREE(mpp->features);
+               mpp->features = NULL;
+       }
+
+       if (mpp->hwhandler) {
+               FREE(mpp->hwhandler);
+               mpp->hwhandler = NULL;
+       }
+
+       free_pgvec(mpp->pg, KEEP_PATHS);
+       mpp->pg = NULL;
+
        if (update_multipath_table(mpp, pathvec))
                return 1;
 
@@ -240,6 +267,7 @@ update_multipath (struct paths *allpaths, char *mapname)
        int i, j;
        int r = 1;
 
+       pthread_cleanup_push(cleanup_lock, allpaths->lock);
        lock(allpaths->lock);
        mpp = find_mp(allpaths->mpvec, mapname);
 
@@ -274,7 +302,7 @@ update_multipath (struct paths *allpaths, char *mapname)
        }
        r = 0;
 out:
-       unlock(allpaths->lock);
+       pthread_cleanup_pop(1);
 
        if (r)
                condlog(0, "failed to update multipath");
@@ -462,6 +490,19 @@ remove_map (struct multipath * mpp, struct paths * allpaths)
        mpp = NULL;
 }
 
+static void
+remove_maps (struct paths * allpaths)
+{
+       int i;
+       struct multipath * mpp;
+
+       vector_foreach_slot (allpaths->mpvec, mpp, i)
+               remove_map(mpp, allpaths);
+
+       vector_free(allpaths->mpvec);
+       allpaths->mpvec = NULL;
+}
+
 int
 uev_add_map (char * devname, struct paths * allpaths)
 {
@@ -698,17 +739,18 @@ uxsock_trigger (char * str, void * trigger_data)
 
        allpaths = (struct paths *)trigger_data;
 
+       pthread_cleanup_push(cleanup_lock, allpaths->lock);
        lock(allpaths->lock);
 
        reply = parse_cmd(str, allpaths);
 
        if (!reply)
-               reply = strdup("fail\n");
+               reply = STRDUP("fail\n");
 
        else if (strlen(reply) == 0)
-               reply = strdup("ok\n");
+               reply = STRDUP("ok\n");
 
-       unlock(allpaths->lock);
+       pthread_cleanup_pop(1);
 
        return reply;
 }
@@ -721,6 +763,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
        struct paths * allpaths;
 
        allpaths = (struct paths *)trigger_data;
+       pthread_cleanup_push(cleanup_lock, allpaths->lock);
        lock(allpaths->lock);
 
        if (strncmp(uev->devpath, "/block", 6))
@@ -760,7 +803,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 
 out:
        FREE(uev);
-       unlock(allpaths->lock);
+       pthread_cleanup_pop(1);
        return r;
 }
 
@@ -794,19 +837,6 @@ uxlsnrloop (void * ap)
        return NULL;
 }
 
-static void
-strvec_free (vector vec)
-{
-       int i;
-       char * str;
-
-       vector_foreach_slot (vec, str, i)
-               if (str)
-                       FREE(str);
-
-       vector_free(vec);
-}
-
 static int
 exit_daemon (int status)
 {
@@ -821,10 +851,11 @@ exit_daemon (int status)
 
        condlog(2, "--------shut down-------");
        
-       if (logsink)
-               log_thread_stop();
+       lock(&exit_mutex);
+       pthread_cond_signal(&exit_cond);
+       unlock(&exit_mutex);
 
-       exit(status);
+       return status;
 }
 
 /*
@@ -937,6 +968,7 @@ checkerloop (void *ap)
        condlog(2, "path checkers start up");
 
        while (1) {
+               pthread_cleanup_push(cleanup_lock, allpaths->lock);
                lock(allpaths->lock);
                condlog(4, "tick");
 
@@ -1050,7 +1082,7 @@ checkerloop (void *ap)
                        count = MAPGCINT;
                }
                
-               unlock(allpaths->lock);
+               pthread_cleanup_pop(1);
                sleep(1);
        }
        return NULL;
@@ -1123,10 +1155,12 @@ prepare_namespace(void)
        if (stat(CALLOUT_DIR, buf) < 0) {
                if (mkdir(CALLOUT_DIR, mode) < 0) {
                        condlog(0, "cannot create " CALLOUT_DIR);
+                       FREE(buf);
                        return -1;
                }
                condlog(4, "created " CALLOUT_DIR);
        }
+       FREE(buf);
 
        /*
         * compute the optimal ramdisk size
@@ -1168,7 +1202,8 @@ prepare_namespace(void)
                }
                condlog(4, "cp %s in ramfs", bin);
        }
-       strvec_free(conf->binvec);
+       free_strvec(conf->binvec);
+       conf->binvec = NULL;
 
        /*
         * bind the ramfs to :
@@ -1345,11 +1380,38 @@ child (void * param)
        pthread_create(&check_thr, &attr, checkerloop, allpaths);
        pthread_create(&uevent_thr, &attr, ueventloop, allpaths);
        pthread_create(&uxlsnr_thr, &attr, uxlsnrloop, allpaths);
-       pthread_join(check_thr, NULL);
-       pthread_join(uevent_thr, NULL);
-       pthread_join(uxlsnr_thr, NULL);
 
-       return 0;
+       pthread_cond_wait(&exit_cond, &exit_mutex);
+
+       /*
+        * exit path
+        */
+       lock(allpaths->lock);
+       remove_maps(allpaths);
+       free_pathvec(allpaths->pathvec, FREE_PATHS);
+
+       pthread_cancel(check_thr);
+       pthread_cancel(uevent_thr);
+       pthread_cancel(uxlsnr_thr);
+
+       free_keys(keys);
+       free_handlers(handlers);
+       free_polls();
+
+       unlock(allpaths->lock);
+       pthread_mutex_destroy(allpaths->lock);
+       FREE(allpaths->lock);
+       FREE(allpaths);
+       free_config(conf);
+
+       if (logsink)
+               log_thread_stop();
+
+#ifdef _DEBUG_
+       dbg_free_final(NULL);
+#endif
+
+       exit(0);
 }
 
 int
@@ -1386,6 +1448,7 @@ main (int argc, char *argv[])
        switch(arg) {
                case 'd':
                        logsink = 0;
+                       //debug=1; /* ### comment me out ### */
                        break;
                case 'v':
                        if (sizeof(optarg) > sizeof(char *) ||

diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
index f40b393..cc850bf 100644 (file)
--- a/multipathd/uxclnt.c
+++ b/multipathd/uxclnt.c
@@ -12,6 +12,7 @@
 #include <readline/history.h>
 
 #include <uxsock.h>
+#include <memory.h>
 
 /*
  * process the client 
@@ -33,7 +34,7 @@ static void process(int fd)
                        add_history(line);
 
                free(line);
-               free(reply);
+               FREE(reply);
        }
 }
 
@@ -46,7 +47,7 @@ static void process_req(int fd, char * inbuf)
        recv_packet(fd, &reply, &len);
 
        printf("%s\n", reply);
-       free(reply);
+       FREE(reply);
 }
        
 /*

diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index b0ed6f5..44c2588 100644 (file)
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -19,6 +19,8 @@
 #include <structs.h>
 #include <uxsock.h>
 
+#include "uxlsnr.h"
+
 #define SLEEP_TIME 5000
 
 struct client {
@@ -66,6 +68,11 @@ static void dead_client(struct client *c)
        num_clients--;
 }
 
+void free_polls (void)
+{
+       FREE(polls);
+}
+
 /*
  * entry point
  */
@@ -75,7 +82,6 @@ void * uxsock_listen(char * (*uxsock_trigger)(char *, void * trigger_data),
        int ux_sock;
        size_t len;
        char *inbuf;
-       struct pollfd *polls = NULL;
        char *reply;
 
        ux_sock = ux_socket_listen(SOCKET_NAME);
@@ -85,6 +91,8 @@ void * uxsock_listen(char * (*uxsock_trigger)(char *, void * trigger_data),
                exit(1);
        }
 
+       polls = (struct pollfd *)MALLOC(0);
+
        while (1) {
                struct client *c;
                int i, poll_count;

diff --git a/multipathd/uxlsnr.h b/multipathd/uxlsnr.h
index 2afade1..31fcbd5 100644 (file)
--- a/multipathd/uxlsnr.h
+++ b/multipathd/uxlsnr.h
@@ -1,3 +1,6 @@
+struct pollfd *polls;
+
+void free_polls(void);
 void * uxsock_listen(char * (*uxsock_trigger)
                        (char *, void * trigger_data),
                        void * trigger_data);