Generate transient property signatures by probing systemd

CHANGES.md

@@ -6,6 +6,8 @@ the one you find pypi, but its the one in development.
 
 dev
 =====
+* Generate transient unit property signatures by probing systemd 255, 257 and 259 instead of keeping them by hand: fixes `Conditions` and `Asserts`, adds 227 missing properties, and drops 52 that systemd does not let you set
+* Fix leaked D-Bus messages, errors and passed file descriptors when `DBus.call_method` or `DBus.get_property` fails
 
 0.15.3
 =====

CONTRIBUTING.md

@@ -61,6 +61,22 @@ each commit:
 uv run pre-commit install
 ```
 
+### Transient unit property signatures
+
+`pystemd/systemd1/transient_properties.py` lists the D-Bus signature systemd
+accepts for each property of a transient unit. It is generated, so do not edit
+it by hand. The data comes from asking a running systemd, one file per version
+in `tools/transient-properties/`.
+
+To add a systemd version, run the probe as root inside a throwaway container
+or VM with that systemd (it creates and stops transient units), then
+regenerate the module:
+
+```bash
+python3 tools/probe_transient_properties.py tools/transient-properties/systemd-NNN.json
+python3 tools/generate_transient_properties.py
+```
+
 ## Sending a pull request
 Have a fix or feature? Awesome! When you send the pull request we suggest you
 include a build output.

MANIFEST.in

@@ -1,3 +1,5 @@
 include README.md LICENSE
 recursive-include tests *.py
 recursive-include pystemd *.pyx *.pxd *.pxi
+include tools/generate_transient_properties.py
+recursive-include tools/transient-properties *.json

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)

e2e/test_transient_units.py

@@ -55,3 +55,21 @@ def test_transient_unit_with_dependencies():
 
     # Cleanup
     unit.Unit.Stop(b"replace")
+
+
+def test_transient_unit_with_conditions():
+    """Conditions is set as a(sbbs), not read type a(sbbsi)"""
+    if os.geteuid() != 0:
+        pytest.skip("Requires root privileges")
+
+    unit = pystemd.run(
+        [b"/bin/true"],
+        extra={
+            b"Conditions": [(b"ConditionPathExists", False, False, b"/")],
+        },
+        remain_after_exit=True,
+    )
+
+    assert unit.Unit.Conditions[0][:4] == (b"ConditionPathExists", False, False, b"/")
+
+    unit.Unit.Stop(b"replace")

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