[systemd-commits] 2 commits - src/libsystemd

Tom Gundersen tomegun at kemper.freedesktop.org
Thu Mar 27 16:51:12 PDT 2014


 src/libsystemd/sd-rtnl/rtnl-message.c |   11 ++--
 src/libsystemd/sd-rtnl/sd-rtnl.c      |   91 ++++++++--------------------------
 2 files changed, 29 insertions(+), 73 deletions(-)

New commits:
commit 9f5bbfe354c52cd9e28cca32c35596b73e8d738b
Author: Tom Gundersen <teg at jklm.no>
Date:   Fri Mar 28 00:46:45 2014 +0100

    sd-rtnl: message - fix memory leak

diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
index 690466e..b4421af 100644
--- a/src/libsystemd/sd-rtnl/rtnl-message.c
+++ b/src/libsystemd/sd-rtnl/rtnl-message.c
@@ -279,7 +279,7 @@ sd_rtnl_message *sd_rtnl_message_unref(sd_rtnl_message *m) {
 
                 free(m->hdr);
 
-                for (i = 0; i < m->n_containers; i++)
+                for (i = 0; i <= m->n_containers; i++)
                         free(m->rta_offset_tb[i]);
 
                 free(m);

commit 8c57830308a612b06b53f5fd31cfb765d8710d68
Author: Tom Gundersen <teg at jklm.no>
Date:   Sun Mar 23 15:33:24 2014 +0100

    sd-rtnl: message - don't reference associated rtnl object
    
    The object is not currently used, so just drop the refenence. If/when we end up
    using the object in the future, we must make sure to deal with possible mutual
    references between rtnl busses and their queued messages; as is done in sd-bus.

diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
index 84a8ffa..690466e 100644
--- a/src/libsystemd/sd-rtnl/rtnl-message.c
+++ b/src/libsystemd/sd-rtnl/rtnl-message.c
@@ -46,6 +46,11 @@ int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initial_size) {
         assert_return(ret, -EINVAL);
         assert_return(initial_size >= sizeof(struct nlmsghdr), -EINVAL);
 
+        /* Note that 'rtnl' is curretly unused, if we start using it internally
+           we must take care to avoid problems due to mutual references between
+           busses and their queued messages. See sd-bus.
+         */
+
         m = new0(sd_rtnl_message, 1);
         if (!m)
                 return -ENOMEM;
@@ -61,9 +66,6 @@ int message_new(sd_rtnl *rtnl, sd_rtnl_message **ret, size_t initial_size) {
         m->hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
         m->sealed = false;
 
-        if (rtnl)
-                m->rtnl = sd_rtnl_ref(rtnl);
-
         *ret = m;
 
         return 0;
@@ -275,7 +277,6 @@ sd_rtnl_message *sd_rtnl_message_unref(sd_rtnl_message *m) {
         if (m && REFCNT_DEC(m->n_ref) <= 0) {
                 unsigned i;
 
-                sd_rtnl_unref(m->rtnl);
                 free(m->hdr);
 
                 for (i = 0; i < m->n_containers; i++)
diff --git a/src/libsystemd/sd-rtnl/sd-rtnl.c b/src/libsystemd/sd-rtnl/sd-rtnl.c
index 695a2da..9f3a4f3 100644
--- a/src/libsystemd/sd-rtnl/sd-rtnl.c
+++ b/src/libsystemd/sd-rtnl/sd-rtnl.c
@@ -104,6 +104,9 @@ int sd_rtnl_open(sd_rtnl **ret, uint32_t groups) {
 }
 
 sd_rtnl *sd_rtnl_ref(sd_rtnl *rtnl) {
+        assert_return(rtnl, NULL);
+        assert_return(!rtnl_pid_changed(rtnl), NULL);
+
         if (rtnl)
                 assert_se(REFCNT_INC(rtnl->n_ref) >= 2);
 
@@ -111,87 +114,39 @@ sd_rtnl *sd_rtnl_ref(sd_rtnl *rtnl) {
 }
 
 sd_rtnl *sd_rtnl_unref(sd_rtnl *rtnl) {
-        unsigned long refs;
-
         if (!rtnl)
                 return NULL;
 
-        /*
-         * If our ref-cnt is exactly the number of internally queued messages
-         * plus the ref-cnt to be dropped, then we know there's no external
-         * reference to us. Hence, we look through all queued messages and if
-         * they also have no external references, we're about to drop the last
-         * ref. Flush the queues so the REFCNT_DEC() below will drop to 0.
-         * We must be careful not to introduce inter-message references or this
-         * logic will fall apart..
-         */
-
-        refs = rtnl->rqueue_size + rtnl->wqueue_size + 1;
+        assert_return(!rtnl_pid_changed(rtnl), NULL);
 
-        if (REFCNT_GET(rtnl->n_ref) <= refs) {
+        if (REFCNT_DEC(rtnl->n_ref) <= 0) {
                 struct match_callback *f;
-                bool q = true;
                 unsigned i;
 
-                for (i = 0; i < rtnl->rqueue_size; i++) {
-                        if (REFCNT_GET(rtnl->rqueue[i]->n_ref) > 1) {
-                                q = false;
-                                break;
-                        } else if (rtnl->rqueue[i]->rtnl != rtnl)
-                                --refs;
-                }
-
-                if (q) {
-                        for (i = 0; i < rtnl->wqueue_size; i++) {
-                                if (REFCNT_GET(rtnl->wqueue[i]->n_ref) > 1) {
-                                        q = false;
-                                        break;
-                                } else if (rtnl->wqueue[i]->rtnl != rtnl)
-                                        --refs;
-                        }
-                }
-
-                if (q && REFCNT_GET(rtnl->n_ref) == refs) {
-                        /* Drop our own ref early to avoid recursion from:
-                         *   sd_rtnl_message_unref()
-                         *     sd_rtnl_unref()
-                         * These must enter sd_rtnl_unref() with a ref-cnt
-                         * smaller than us. */
-                        REFCNT_DEC(rtnl->n_ref);
-
-                        for (i = 0; i < rtnl->rqueue_size; i++)
-                                sd_rtnl_message_unref(rtnl->rqueue[i]);
-                        free(rtnl->rqueue);
+                for (i = 0; i < rtnl->rqueue_size; i++)
+                        sd_rtnl_message_unref(rtnl->rqueue[i]);
+                free(rtnl->rqueue);
 
-                        for (i = 0; i < rtnl->wqueue_size; i++)
-                                sd_rtnl_message_unref(rtnl->wqueue[i]);
-                        free(rtnl->wqueue);
+                for (i = 0; i < rtnl->wqueue_size; i++)
+                        sd_rtnl_message_unref(rtnl->wqueue[i]);
+                free(rtnl->wqueue);
 
-                        assert_se(REFCNT_GET(rtnl->n_ref) == 0);
+                hashmap_free_free(rtnl->reply_callbacks);
+                prioq_free(rtnl->reply_callbacks_prioq);
 
-                        hashmap_free_free(rtnl->reply_callbacks);
-                        prioq_free(rtnl->reply_callbacks_prioq);
+                sd_event_source_unref(rtnl->io_event_source);
+                sd_event_source_unref(rtnl->time_event_source);
+                sd_event_source_unref(rtnl->exit_event_source);
+                sd_event_unref(rtnl->event);
 
-                        while ((f = rtnl->match_callbacks)) {
-                                LIST_REMOVE(match_callbacks, rtnl->match_callbacks, f);
-                                free(f);
-                        }
-
-                        safe_close(rtnl->fd);
-
-                        sd_event_source_unref(rtnl->io_event_source);
-                        sd_event_source_unref(rtnl->time_event_source);
-                        sd_event_source_unref(rtnl->exit_event_source);
-                        sd_event_unref(rtnl->event);
-
-                        free(rtnl);
-
-                        return NULL;
+                while ((f = rtnl->match_callbacks)) {
+                        LIST_REMOVE(match_callbacks, rtnl->match_callbacks, f);
+                        free(f);
                 }
-        }
 
-        assert_se(REFCNT_GET(rtnl->n_ref) > 0);
-        REFCNT_DEC(rtnl->n_ref);
+                safe_close(rtnl->fd);
+                free(rtnl);
+        }
 
         return NULL;
 }



More information about the systemd-commits mailing list