udevd - Big memory and thread cleanup
authorAlex Hornung <ahornung@gmail.com>
Sat, 17 Jul 2010 08:49:20 +0000 (09:49 +0100)
committerAlex Hornung <ahornung@gmail.com>
Sat, 17 Jul 2010 08:54:58 +0000 (09:54 +0100)
* Adjust read_xml() to allocate just the right size of memory, so we
  don't waste anything by allocating 12 MB per client, when the usual
  client xml is a few kB.

* Detach threads so that they can free resources whenever they return
  from their startup function.

* Release the memory associated with a particular iterator and with some
  of the fields in the event filters, which were previously leaking.

* Let the monitor thread check every few (2) seconds if the underlying
  connection is still alive. Previously the threads were staying around
  even after the connection was closed when no event (attach/detach)
  occured.

* Clean up the pdev array on a clean exit so it's easier to find memory
  leaks, since the amount of allocations done by proplib on behalf of
  the pdev stuff is huge.

* Incidentally, don't forget to update the pdev array when a device is
  detached.

Reported-by: Antonio Huete (tuxillo@)
sbin/udevd/udevd.c
sbin/udevd/udevd.h
sbin/udevd/udevd_client.c
sbin/udevd/udevd_monitor.c
sbin/udevd/udevd_pdev.c
sbin/udevd/udevd_socket.c

index d5f69e8..84fc9d9 100644 (file)
@@ -214,7 +214,7 @@ again:
                if (pa == NULL)
                        goto out;
                prop_array_remove(pa, idx);
-               //pdev_array_entry_insert(pa);
+               pdev_array_entry_insert(pa);
                break;
 
        case UDEV_EV_KEY_UPDATE:
@@ -292,6 +292,7 @@ killed(int sig __unused)
 {
        syslog(LOG_ERR, "udevd stopped");
        unlink("/var/run/udevd.pid");
+       pdev_array_clean();
 }
 
 int
index cc19122..672a5e3 100644 (file)
@@ -93,13 +93,14 @@ struct cmd_function {
 int    init_local_server(const char *sockfile, int socktype, int nonblock);
 int    block_descriptor(int s);
 int    unblock_descriptor(int s);
-int    read_xml(int s, char *buf, size_t buf_sz);
+int    read_xml(int s, char **buf);
 int    send_xml(int s, char *xml);
 
 /* From udevd_pdev.c */
 void   pdev_array_entry_ref(struct pdev_array_entry *pae);
 void   pdev_array_entry_unref(struct pdev_array_entry *pae);
 void   pdev_array_entry_insert(prop_array_t pa);
+void   pdev_array_clean(void);
 struct pdev_array_entry *pdev_array_entry_get(int64_t generation);
 struct pdev_array_entry *pdev_array_entry_get_last(void);
 
index 79f25f9..212fb3a 100644 (file)
@@ -54,6 +54,7 @@
 #include <syslog.h>
 #include <unistd.h>
 #include <pthread.h>
+#include <assert.h>
 
 #include <libprop/proplib.h>
 #include <sys/udev.h>
@@ -100,14 +101,16 @@ client_thread(void *arg)
        char    *xml;
        int     r, n, error;
 
+       r = pthread_detach(pthread_self());
+       assert(r == 0);
+
        r = ignore_signal(SIGPIPE);
        if (r != 0)
                err(1, "could not ignore_signal SIGPIPE");
 
        cli = (struct client_info *)arg;
-       xml = malloc(12*1024*1024); /* generous 12 MB */
        for (;;) {
-               n = read_xml(cli->fd, xml, 12*1024*1024);
+               n = read_xml(cli->fd, &xml);
                if (n == 0)
                        goto cli_disconnect;
                else if (n < 0)
@@ -116,6 +119,8 @@ client_thread(void *arg)
                xml[n+1] = '\0';
 
                dict = prop_dictionary_internalize(xml);
+               free(xml);
+
                if (dict == NULL) {
                        syslog(LOG_ERR, "internalization of received XML failed");
                        goto error_out;
@@ -150,7 +155,6 @@ error_out:
 
 cli_disconnect:
        close(cli->fd);
-       free(xml);
        cli->fd = -1;
        free(cli);
        return NULL;
@@ -205,6 +209,7 @@ client_cmd_getdevs(struct client_info *cli, prop_dictionary_t cli_dict)
                        }
                }
 
+               prop_object_iterator_release(iter);
                udev_monitor_free(udm);
        } else {
                pa = pae->pdev_array;
index 4dfcd4e..bc9fb1a 100644 (file)
@@ -148,6 +148,11 @@ udev_monitor_free(struct udev_monitor *udm)
 
        while ((evf = TAILQ_FIRST(&udm->ev_filt)) != NULL) {
                TAILQ_REMOVE(&udm->ev_filt, evf, link);
+               free(evf->key);
+               if (evf->type == EVENT_FILTER_TYPE_WILDCARD)
+                       free(evf->wildcard_match);
+               else if (evf->type == EVENT_FILTER_TYPE_REGEX)
+                       regfree(&evf->regex_match);
                free(evf);
        }
 
@@ -162,9 +167,11 @@ client_cmd_monitor(struct client_info *cli, prop_dictionary_t dict)
        prop_object_t   po;
        struct udev_monitor     *udm;
        struct udev_monitor_event       *udm_ev;
+       struct timespec abstime;
+       struct pollfd fds[1];
        char *xml;
        ssize_t r;
-       int ok = 1;
+       int ret, ok, dummy;
 
        pa = NULL;
        po = prop_dictionary_get(dict, "filters");
@@ -176,13 +183,30 @@ client_cmd_monitor(struct client_info *cli, prop_dictionary_t dict)
        if (udm == NULL)
                return 1;
 
+       ok = 1;
+       fds[0].fd = cli->fd;
+       fds[0].events = POLLRDNORM;
+
        MONITOR_LOCK();
        TAILQ_INSERT_TAIL(&udev_monitor_list, udm, link);
        MONITOR_UNLOCK();
 
        pthread_mutex_lock(&udm->q_lock);
        while (ok) {
-               pthread_cond_wait(&udm->cond, &udm->q_lock);
+               clock_gettime(CLOCK_REALTIME,&abstime);
+               abstime.tv_sec += 2;
+               ret = pthread_cond_timedwait(&udm->cond, &udm->q_lock, &abstime);
+
+               if (ret == EINVAL) {
+                       syslog(LOG_ERR, "pthread_cond_timedwait error: EINVAL");
+                       goto end_nofree;
+               }
+
+               if ((ret = poll(fds, 1, 0)) > 0) {
+                       ret = recv(fds[0].fd, &dummy, sizeof(dummy), MSG_DONTWAIT);
+                       if ((ret == 0) || ((ret < 0) && (errno != EAGAIN)))
+                               goto end_nofree;
+               }
 
                udm_ev = TAILQ_FIRST(&udm->ev_queue);
                if (udm_ev == NULL)
@@ -205,10 +229,12 @@ client_cmd_monitor(struct client_info *cli, prop_dictionary_t dict)
                free(xml);
                continue;
 end:
+               free(xml);
+end_nofree:
                pthread_mutex_unlock(&udm->q_lock);
                close(cli->fd);
                ok = 0;
-               free(xml);
+               
        }
 
        MONITOR_LOCK();
@@ -236,6 +262,7 @@ _parse_filter_prop(struct udev_monitor *udm, prop_array_t pa)
 
        while ((dict = prop_object_iterator_next(iter)) != NULL) {
                evf = malloc(sizeof(struct event_filter));
+               bzero(evf, sizeof(struct event_filter));
                if (evf == NULL)
                        goto error_alloc;
 
@@ -325,7 +352,6 @@ match_filter(struct event_filter *evf, prop_dictionary_t ev_dict)
 
        prop_object_retain(ev_dict);
 
-       assert(prop_dictionary_externalize(ev_dict) != NULL);
        if ((po = prop_dictionary_get(ev_dict, evf->key)) == NULL)
                goto no_match;
 
index 5992a13..dcb50e6 100644 (file)
@@ -106,6 +106,18 @@ pdev_array_entry_insert(prop_array_t pa)
                pdev_array_entry_unref(opae);
 }
 
+void
+pdev_array_clean(void)
+{
+       struct pdev_array_entry *pae;
+
+       while ((pae = TAILQ_LAST(&pdev_array_list, pdev_array_list_head)) != NULL) {
+               TAILQ_REMOVE(&pdev_array_list, pae, link);
+               prop_object_release(pae->pdev_array);
+               free(pae);
+       }
+}
+
 struct pdev_array_entry *
 pdev_array_entry_get(int64_t generation)
 {
index f6a4bfd..d977fe2 100644 (file)
@@ -78,23 +78,30 @@ send_xml(int s, char *xml)
 }
 
 int
-read_xml(int s, char *buf, size_t buf_sz)
+read_xml(int s, char **buf)
 {
+       char *xml;
        size_t sz;
        int n, r;
 
+       *buf = NULL;
+
        n = recv(s, &sz, sizeof(sz), MSG_WAITALL);
-       if (n <= 0)
+       if ((n <= 0) || (sz > 12*1024*1024)) /* Arbitrary limit */
                return n;
 
+       xml = malloc(sz+2);     
        r = 0;
-       while ((r < (ssize_t)sz) && (r < (ssize_t)buf_sz)) {
-               n = recv(s, buf+r, sz-r, MSG_WAITALL);
-               if (n <= 0)
+       while (r < (ssize_t)sz) {
+               n = recv(s, xml+r, sz-r, MSG_WAITALL);
+               if (n <= 0) {
+                       free(xml);
                        return n;
+               }
                r += n;
        }
 
+       *buf = xml;
        return r;
 }