summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Gundersen <teg@jklm.no>2014-03-23 15:33:24 +0100
committerTom Gundersen <teg@jklm.no>2014-03-28 00:50:50 +0100
commit8c57830308a612b06b53f5fd31cfb765d8710d68 (patch)
tree963edd7f57665b022a0e2f06cc47167657078dde
parentbf81e792f3c0aed54edf004c1c95cc6f6d81d0ee (diff)
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.
-rw-r--r--src/libsystemd/sd-rtnl/rtnl-message.c9
-rw-r--r--src/libsystemd/sd-rtnl/sd-rtnl.c91
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;
}