Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in sd-bus polling #282

Merged
merged 1 commit into from
Nov 21, 2023
Merged

Conversation

cillian64
Copy link
Contributor

sd-bus requires that sd_bus_get_fd, sd_bus_get_events, and sd_bus_get_timeout are called for every invocation of poll, and the returned FD, events, and timeout used. In particular, sd-bus may return events=0 and timeout=0 if it already has dbus messages to process in its receive queue.

xdpw currently doesn't call sd_bus_get_events or sd_bus_get_timeout at all. This can result in the situation where xdpw goes into a poll with no timeout and a message in sd-bus' receive queue is never handled until another dbus message arrives. I have a case where this ignored messages causes xdg-desktop-portal to hang.

This commit changes the polling code to match sd-bus' requirements.

Fixes #281

@name-snrl
Copy link

name-snrl commented Nov 8, 2023

As I said in #281, this PR completely solves the random startup delay for me. I hope it will be merged in the near future

src/core/main.c Outdated
// sd-bus requires that we update FD/events/timeout every time we poll
pollfds[EVENT_LOOP_DBUS].fd = sd_bus_get_fd(state.bus);
if (pollfds[EVENT_LOOP_DBUS].fd < 0) {
logprint(ERROR, "sd_bus_get_fd failed: %s", strerror(errno));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: it doesn't seem like sd-bus populates errno. It seems like the error number is returned instead.

@emersion
Copy link
Owner

LGTM apart from this nit.

sd-bus requires that `sd_bus_get_fd`, `sd_bus_get_events`, and
`sd_bus_get_timeout` are called for every invocation of `poll`, and the
returned FD, events, and timeout used.  In particular, sd-bus may return
`events=0` and `timeout=0` if it already has dbus messages to process in
its receive queue.

xdpw currently doesn't call `sd_bus_get_events` or `sd_bus_get_timeout`
at all.  This can result in the situation where xdpw goes into a `poll`
with no timeout and a message in sd-bus' receive queue is never handled
until another dbus message arrives.  I have a case where this ignored
messages causes xdg-desktop-portal to hang.

This commit changes the polling code to match sd-bus' requirements.

Fixes emersion#281
Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@emersion emersion merged commit b5f3878 into emersion:master Nov 21, 2023
3 checks passed
@cillian64
Copy link
Contributor Author

Thanks for the review!

@cillian64 cillian64 deleted the sdbus_polling branch November 21, 2023 10:30
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.

Bug in sd-bus polling code causes hang
3 participants