Fix leaks on failed call_method and get_property

CHANGES.md

@@ -6,6 +6,7 @@ the one you find pypi, but its the one in development.
 
 dev
 =====
+* Fix leaked D-Bus messages, errors and passed file descriptors when `DBus.call_method` or `DBus.get_property` fails
 
 0.15.3
 =====

e2e/test_dbus.py

@@ -1,9 +1,38 @@
 """E2E tests for D-Bus integration"""
 
+import os
+
+import pytest
+
 from pystemd.dbuslib import DBus
 
 
 def test_dbus_connection():
     """Test basic D-Bus connection"""
     with DBus() as bus:
         assert bus is not None
+
+
+def test_failed_call_does_not_leak():
+    """A failed method call must release its message, including passed fds"""
+    fd = os.open("/dev/null", os.O_RDONLY)
+    try:
+        with DBus() as bus:
+
+            def fail():
+                with pytest.raises(Exception, match="NoSuchMethod"):
+                    bus.call_method(
+                        b"org.freedesktop.DBus",
+                        b"/org/freedesktop/DBus",
+                        b"org.freedesktop.DBus",
+                        b"NoSuchMethod",
+                        [(ord(b"h"), fd)],
+                    )
+
+            fail()
+            before = len(os.listdir("/proc/self/fd"))
+            for _ in range(100):
+                fail()
+            assert len(os.listdir("/proc/self/fd")) == before
+    finally:
+        os.close(fd)

pystemd/dbuslib.pyx

@@ -413,7 +413,6 @@ cdef class DBus:
             int arg_type
 
             dbusc.sd_bus_message *msg_call = NULL
-            dbusc.sd_bus_message *msg_reply = NULL
             dbusc.sd_bus_error error = dbusc.sd_bus_error(NULL, NULL, 0)
 
             DbusMessage msg = DbusMessage()
@@ -430,26 +429,27 @@ cdef class DBus:
         if r < 0:
             raise DBusError(r, None, 'Could not create DBus method')
 
-        for narg, (arg_type, arg_value) in enumerate(args or []):
-            try:
-                self._msg_append(msg_call, arg_type, arg_value)
-            except TypeError as e:
-              raise TypeError(
-                str(e) + ' for input "{}", element '
-                'number {} in input series:\n{}'.format(
-                  arg_value, narg, pformat(args)))
+        # msg_call (and any fds appended to it) and error must be released on
+        # every path, including failed calls and bad arguments.
+        try:
+            for narg, (arg_type, arg_value) in enumerate(args or []):
+                try:
+                    self._msg_append(msg_call, arg_type, arg_value)
+                except TypeError as e:
+                  raise TypeError(
+                    str(e) + ' for input "{}", element '
+                    'number {} in input series:\n{}'.format(
+                      arg_value, narg, pformat(args)))
 
+            r = dbusc.sd_bus_call(self.bus, msg_call, 0, &error, msg.ref())
 
-        r = dbusc.sd_bus_call(self.bus, msg_call, 0, &error, msg.ref())
-
-        if r < 0:
-            raise DBusError(r, error.name, error.message)
-
-        msg.process_reply(False)
+            if r < 0:
+                raise DBusError(r, error.name, error.message)
 
-        dbusc.sd_bus_message_unref(msg_call)
-        dbusc.sd_bus_message_unref(msg_reply)
-        dbusc.sd_bus_error_free(&error)
+            msg.process_reply(False)
+        finally:
+            dbusc.sd_bus_message_unref(msg_call)
+            dbusc.sd_bus_error_free(&error)
 
         return msg
 
@@ -467,19 +467,22 @@ cdef class DBus:
 
           DbusMessage msg = DbusMessage()
 
-        r = dbusc.sd_bus_get_property(
-          self.bus,
-          destination,
-          path,
-          interface,
-          property,
-          &error,
-          msg.ref(),
-          rtype
-        )
+        try:
+            r = dbusc.sd_bus_get_property(
+              self.bus,
+              destination,
+              path,
+              interface,
+              property,
+              &error,
+              msg.ref(),
+              rtype
+            )
 
-        if r < 0:
-            raise DBusError(r, error.name, error.message)
+            if r < 0:
+                raise DBusError(r, error.name, error.message)
+        finally:
+            dbusc.sd_bus_error_free(&error)
 
         msg.process_reply(False)
         return msg.body