From e630fab521513ec15fd69756894910b09c4c2dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9tur=20Ingi=20Egilsson?= Date: Sun, 18 Oct 2026 05:37:57 +0200 Subject: [PATCH] Fix leaks on failed call_method and get_property When sd_bus_call failed, call_method raised before unref-ing the call message and freeing the sd_bus_error. Any file descriptor appended to the message (type "h") was leaked with it, so a long-running process making failing calls eventually hit EMFILE. get_property never freed its error. Release both in a finally block, and add an e2e test that makes 100 failing calls with an fd argument and checks the fd count stays flat. Co-Authored-By: Claude Opus 5.5 --- CHANGES.md | 1 + e2e/test_dbus.py | 29 +++++++++++++++++++++ pystemd/dbuslib.pyx | 63 ++++++++++++++++++++++++--------------------- 3 files changed, 63 insertions(+), 30 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 790937d..e3bb890 100644 --- a/CHANGES.md +++ b/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 ===== diff --git a/e2e/test_dbus.py b/e2e/test_dbus.py index c8386ee..0155c34 100644 --- a/e2e/test_dbus.py +++ b/e2e/test_dbus.py @@ -1,5 +1,9 @@ """E2E tests for D-Bus integration""" +import os + +import pytest + from pystemd.dbuslib import DBus @@ -7,3 +11,28 @@ 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) diff --git a/pystemd/dbuslib.pyx b/pystemd/dbuslib.pyx index a0dd89b..0051512 100644 --- a/pystemd/dbuslib.pyx +++ b/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