Repository navigation
Conversation
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 <noreply@anthropic.com>
KNOWN_UNIT_SIGNATURES was kept by hand and had drifted: Conditions and
Asserts were listed with their read type a(sbbsi) instead of a(sbbs), so
systemd rejected them ("Bad message"); 52 entries are properties systemd
does not let you set at all (unit state such as ActiveState, reverse
dependencies, removed names, a misspelled SmackLabelIPIn); and over 200
settable properties were missing.
systemd does not publish the signatures it accepts when setting a
property, and they often differ from the introspection XML (ExecStart is
read as a(sasbttttuii) but set as a(sasb); some properties are
write-only). tools/probe_transient_properties.py measures them instead: it
sends each candidate name with each candidate signature to
StartTransientUnit on a throwaway system and classifies systemd's answer.
Candidates come from the machine itself (introspection XML, unit file
directives, string literals in libsystemd-core).
The results for systemd 255 (Ubuntu 24.04), 257 (Debian 13) and 259
(Ubuntu 26.04) are in tools/transient-properties/, and
tools/generate_transient_properties.py turns them into
pystemd/systemd1/transient_properties.py. No property changed signature
between those versions.
KNOWN_UNIT_SIGNATURES is now that table plus pystemd's own conveniences
(RuntimeMaxSec, ListenStream, ...), which are kept separate and tested to
target real properties with the right signature.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-version probing and D-Bus ownership changes warrant human validation, and the source-distribution test dependency remains unresolved.
1 open issue
What changed in this PR
Replaces pystemd’s hand-maintained transient property signatures with generated probe data, while preserving convenience conversions.
Changes:
- Adds probing and generation tools with results from three systemd versions.
- Corrects and expands property signatures, with regression and freshness checks.
- Includes the D-Bus resource-leak fix from #124.
| File | Description |
|---|---|
| tools/transient-properties/systemd-259.json | Records systemd 259 probe results. |
| tools/transient-properties/systemd-257.json | Records systemd 257 probe results. |
| tools/transient-properties/systemd-255.json | Records systemd 255 probe results. |
| tools/probe_transient_properties.py | Discovers accepted property signatures. |
| tools/generate_transient_properties.py | Generates and checks the signature table. |
| tests/test_unit_signatures.py | Checks signatures, conveniences, and freshness. |
| pystemd/systemd1/unit_signatures.py | Combines generated signatures with conveniences. |
| pystemd/systemd1/transient_properties.py | Provides the generated signature table. |
| pystemd/dbuslib.pyx | Releases resources on failed calls. |
| e2e/test_transient_units.py | Tests Conditions serialization. |
| e2e/test_dbus.py | Tests file-descriptor leak prevention. |
| CONTRIBUTING.md | Documents probing and regeneration. |
| CHANGES.md | Records signature and leak fixes. |
Review effort: Balanced
Comment on lines
+67
to
+68
| tool = Path(__file__).parent.parent / "tools/generate_transient_properties.py" | ||
| subprocess.run([sys.executable, str(tool), "--check"], check=True) |
Author
There was a problem hiding this comment.
Good catch, confirmed: the sdist shipped tests/ but not tools/, so test_up_to_date couldn't find the generator. Fixed in 9f939c3 by adding both lines to MANIFEST.in. I built the sdist, unpacked it on Debian 13, and tests/test_unit_signatures.py now passes from it (6 passed).
tests/ is in the sdist, and test_unit_signatures checks the generated module against tools/transient-properties/ by running tools/generate_transient_properties.py, so both need to be shipped too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Depends on #124 (its commit is the first one here); the probe makes a few hundred thousand failing calls and runs out of file descriptors without it.
KNOWN_UNIT_SIGNATURESis kept by hand and has drifted from systemd:ConditionsandAssertsare listed with their read typea(sbbsi)instead ofa(sbbs), so systemd rejects them with "Bad message".ActiveState,SubState,Can*, ...), reverse dependencies (WantedBy,RequiredBy, ...), removed names (StartLimitIntervalSec,Service) and a misspelledSmackLabelIPIn(systemd's isSmackLabelIPin).systemd doesn't publish the signature it accepts when setting a property, and it often differs from the introspection XML:
ExecStartis read asa(sasbttttuii)but set asa(sasb),TimersMonotonicis read asa(stt)but set asa(st), and properties likeOnCalendarorMemoryMaxScaleare write-only.So this measures them.
tools/probe_transient_properties.pysends one property at a time toStartTransientUniton a throwaway system, with a zero value of each candidate signature, and classifies systemd's answer. systemd checks the name before reading the value and reads the value before validating it, so "unknown property", "wrong type" and "type accepted" are distinguishable. Candidate names and signatures come from the machine itself: the introspection XML,systemd --dump-configuration-items, and string literals inlibsystemd-core. Checked against v257'ssrc/core/dbus-*.c, it finds everystreq(name, ...)setter plus the names systemd builds at runtime (Limit*,Exec*, dependency types). The single exception isJobTimeoutUSec, which no released systemd lets you set because of a missingreturn(fixed upstream in systemd/systemd@041e70d7f7).I ran it in nspawn containers on systemd 255 (Ubuntu 24.04), 257 (Debian 13) and 259 (Ubuntu 26.04); the results are in
tools/transient-properties/. No property changed signature between those versions. On 259 the cgroup v1 properties (CPUShares,MemoryLimit,BlockIO*, ...) are accepted but ignored, and the generated table notes that.tools/generate_transient_properties.pyturns the results intopystemd/systemd1/transient_properties.py.KNOWN_UNIT_SIGNATURESandsignature_arraykeep their names and behaviour; the table is now the generated one plus pystemd's own conveniences (RuntimeMaxSec,ListenStream, ...), which stay hand-written but are tested to target real properties with the right signature. A unit test fails if the generated module is stale. CONTRIBUTING.md explains how to add a systemd version.Unit and e2e tests (68) pass on all three systems. The new
Conditionse2e test fails onmain.🤖 Generated with Claude Code