dbus: Unref replies to avoid leaks
Testing showed that failing to free DBus message replies was leaking
800-900kiB per boot. After adding the appropriate unrefs, mboxd memory
consumption stayed stable at 71920kiB across multiple reboots of the
host.
The root cause was identified using a DBus capture of the host's IPMI
traffic during boot, then reducing the output to mboxd-specific messages
and turning them into commands that could be run with ipmitool. Adding
all of these commands to a script and running `pmap` between ipmitool
invocations showed the growth in memory usage across the "boot" process,
but this did not correlate with any particular set of commands to mboxd.
The lack of correlation lead to the hypothesis that we might be able to
reproduce by sending a lot of dbus messages, such as:
```
root@witherspoon:/tmp# for i in `seq 1 10000`; do \
busctl call xyz.openbmc_project.Hiomapd \
/xyz/openbmc_project/Hiomapd \
xyz.openbmc_project.Hiomapd.Protocol.V2 \
GetInfo y 2; \
done
```
Spamming the daemon in this way demonstrated the growth in memory seen
during a regular boot process, confirming that just sending DBus
messages was enough.
Add the necessary unrefs for the replies at the end of each method
handler to ensure the replies are appropriately freed.
Testing and confirmation of the fix were performed on a Witherspoon
system.
Change-Id: If5fe7576aaca1be617181526bf64511ccee1dd9f
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/control_dbus.c b/control_dbus.c
index 322c743..4fb9e33 100644
--- a/control_dbus.c
+++ b/control_dbus.c
@@ -43,7 +43,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int control_dbus_ping(sd_bus_message *m, void *userdata,
@@ -190,7 +192,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int control_dbus_get_u8(sd_bus *bus, const char *path,
diff --git a/control_legacy.c b/control_legacy.c
index 3e9a6df..2a3461d 100644
--- a/control_legacy.c
+++ b/control_legacy.c
@@ -278,6 +278,7 @@
}
rc = sd_bus_send(NULL, n, NULL); /* Send response */
+ sd_bus_message_unref(n);
if (rc < 0)
MSG_ERR("sd_bus_send failed: %d\n", rc);
diff --git a/transport_dbus.c b/transport_dbus.c
index 42fb018..b6eb0e1 100644
--- a/transport_dbus.c
+++ b/transport_dbus.c
@@ -93,7 +93,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_get_info(sd_bus_message *m, void *userdata,
@@ -146,7 +148,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_get_flash_info(sd_bus_message *m, void *userdata,
@@ -181,7 +185,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_create_window(struct mbox_context *context,
@@ -225,7 +231,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_create_read_window(sd_bus_message *m, void *userdata,
@@ -274,7 +282,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
@@ -308,7 +318,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_write_flush(sd_bus_message *m, void *userdata,
@@ -334,7 +346,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_ack(sd_bus_message *m, void *userdata,
@@ -367,7 +381,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_erase(sd_bus_message *m, void *userdata,
@@ -400,7 +416,9 @@
return rc;
}
- return sd_bus_send(NULL, n, NULL);
+ rc = sd_bus_send(NULL, n, NULL);
+ sd_bus_message_unref(n);
+ return rc;
}
static int transport_dbus_get_property(sd_bus *bus,