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.

2 files changed, 28 insertions, 72 deletions

diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c b/src/libsystemd/sd-rtnl/rtnl-message.c
index 84a8ffa59e..690466e2f0 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 695a2daccf..9f3a4f3dea 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;
 }