Skip to content

Fix returned file descriptors getting closed immediately - #121

Merged
igo95862 merged 2 commits into
python-sdbus:masterfrom
luleyleo:dup-returned-h
Aug 8, 2026
Merged

Fix returned file descriptors getting closed immediately#121
igo95862 merged 2 commits into
python-sdbus:masterfrom
luleyleo:dup-returned-h

Conversation

@luleyleo

@luleyleo luleyleo commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

When a DBus method returns a file handler (h), the handler must be duplicated before the message is freed, otherwise the file will be closed before the Python client can do anything with it.

See man "sd_bus_message_read_basic(3)".

See `man "sd_bus_message_read_basic(3)"`
@igo95862

igo95862 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

You are correct. The only issue I see that there is no error checking if fnctl call has succeeded.

@luleyleo

luleyleo commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

You are correct. The only issue I see that there is no error checking if fnctl call has succeeded.

Good point. So, technically, it would make sense to wrap the fnctl call in CALL_SD_BUS_AND_CHECK, but it does feel a bit wrong because of the _SD_BUS_ in its name. Copying the content of the macro doesn't seem nice either, though. Maybe we could rename the macro to a more generic CALL_AND_CHECK?

@igo95862

igo95862 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

It can be renamed later. I already use that macro on functions not part of libsystemd:

self->timer_fd_int = CALL_SD_BUS_AND_CHECK(timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC));

@luleyleo

luleyleo commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

I've added the check now.

@igo95862
igo95862 merged commit 90432a8 into python-sdbus:master Aug 8, 2026
@igo95862

igo95862 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

@luleyleo
luleyleo deleted the dup-returned-h branch August 9, 2026 15:27
@luleyleo luleyleo mentioned this pull request Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants