Merge pull request #25483 from poettering/ppoll-usec-eintr

ppoll() + EINTR fixes
This commit is contained in:
Yu Watanabe
2022-11-24 09:42:48 +09:00
committed by GitHub
11 changed files with 101 additions and 39 deletions
+18
View File
@@ -161,6 +161,21 @@ int ppoll_usec(struct pollfd *fds, size_t nfds, usec_t timeout) {
assert(fds || nfds == 0);
/* This is a wrapper around ppoll() that does primarily two things:
*
* ✅ Takes a usec_t instead of a struct timespec
*
* ✅ Guarantees that if an invalid fd is specified we return EBADF (i.e. converts POLLNVAL to
* EBADF). This is done because EBADF is a programming error usually, and hence should bubble up
* as error, and not be eaten up as non-error POLLNVAL event.
*
* ⚠️ ⚠️ ⚠️ Note that this function does not add any special handling for EINTR. Don't forget
* poll()/ppoll() will return with EINTR on any received signal always, there is no automatic
* restarting via SA_RESTART available. Thus, typically you want to handle EINTR not as an error,
* but just as reason to restart things, under the assumption you use a more appropriate mechanism
* to handle signals, such as signalfd() or signal handlers. ⚠️ ⚠️ ⚠️
*/
if (nfds == 0)
return 0;
@@ -188,6 +203,9 @@ int fd_wait_for_event(int fd, int event, usec_t timeout) {
};
int r;
/* ⚠️ ⚠️ ⚠️ Keep in mind you almost certainly want to handle -EINTR gracefully in the caller, see
* ppoll_usec() above! ⚠️ ⚠️ ⚠️ */
r = ppoll_usec(&pollfd, 1, timeout);
if (r <= 0)
return r;
+5 -2
View File
@@ -2017,9 +2017,12 @@ static int wait_for_devlink(const char *path) {
if (w >= until)
return log_error_errno(SYNTHETIC_ERRNO(ETIMEDOUT), "Device link %s still hasn't shown up, giving up.", path);
r = fd_wait_for_event(inotify_fd, POLLIN, usec_sub_unsigned(until, w));
if (r < 0)
r = fd_wait_for_event(inotify_fd, POLLIN, until - w);
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r))
continue;
return log_error_errno(r, "Failed to watch inotify: %m");
}
(void) flush_fd(inotify_fd);
}
+4 -1
View File
@@ -1308,8 +1308,11 @@ int bus_socket_process_opening(sd_bus *b) {
assert(b->state == BUS_OPENING);
events = fd_wait_for_event(b->output_fd, POLLOUT, 0);
if (events < 0)
if (events < 0) {
if (ERRNO_IS_TRANSIENT(events))
return 0;
return events;
}
if (!(events & (POLLOUT|POLLERR|POLLHUP)))
return 0;
+15 -3
View File
@@ -2465,8 +2465,11 @@ _public_ int sd_bus_call(
left = UINT64_MAX;
r = bus_poll(bus, true, left);
if (r < 0)
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r))
continue;
goto fail;
}
if (r == 0) {
r = -ETIMEDOUT;
goto fail;
@@ -3321,6 +3324,7 @@ static int bus_poll(sd_bus *bus, bool need_more, uint64_t timeout_usec) {
}
_public_ int sd_bus_wait(sd_bus *bus, uint64_t timeout_usec) {
int r;
assert_return(bus, -EINVAL);
assert_return(bus = bus_resolve(bus), -ENOPKG);
@@ -3335,7 +3339,11 @@ _public_ int sd_bus_wait(sd_bus *bus, uint64_t timeout_usec) {
if (bus->rqueue_size > 0)
return 0;
return bus_poll(bus, false, timeout_usec);
r = bus_poll(bus, false, timeout_usec);
if (r < 0 && ERRNO_IS_TRANSIENT(r))
return 1; /* treat EINTR as success, but let's exit, so that the caller will call back into us soon. */
return r;
}
_public_ int sd_bus_flush(sd_bus *bus) {
@@ -3377,8 +3385,12 @@ _public_ int sd_bus_flush(sd_bus *bus) {
return 0;
r = bus_poll(bus, false, UINT64_MAX);
if (r < 0)
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r))
continue;
return r;
}
}
}
+6 -1
View File
@@ -464,13 +464,18 @@ static int netlink_poll(sd_netlink *nl, bool need_more, usec_t timeout_usec) {
}
int sd_netlink_wait(sd_netlink *nl, uint64_t timeout_usec) {
int r;
assert_return(nl, -EINVAL);
assert_return(!netlink_pid_changed(nl), -ECHILD);
if (nl->rqueue_size > 0)
return 0;
return netlink_poll(nl, false, timeout_usec);
r = netlink_poll(nl, false, timeout_usec);
if (r < 0 && ERRNO_IS_TRANSIENT(r)) /* Convert EINTR to "something happened" and give user a chance to run some code before calling back into us */
return 1;
return r;
}
static int timeout_compare(const void *a, const void *b) {
+16 -4
View File
@@ -868,11 +868,14 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
}
static int sendmsg_loop(int fd, struct msghdr *mh, int flags) {
usec_t end;
int r;
assert(fd >= 0);
assert(mh);
end = usec_add(now(CLOCK_MONOTONIC), SEND_TIMEOUT_USEC);
for (;;) {
if (sendmsg(fd, mh, flags) >= 0)
return 0;
@@ -881,20 +884,26 @@ static int sendmsg_loop(int fd, struct msghdr *mh, int flags) {
if (errno != EAGAIN)
return -errno;
r = fd_wait_for_event(fd, POLLOUT, SEND_TIMEOUT_USEC);
if (r < 0)
r = fd_wait_for_event(fd, POLLOUT, LESS_BY(end, now(CLOCK_MONOTONIC)));
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r))
continue;
return r;
}
if (r == 0)
return -ETIMEDOUT;
}
}
static int write_loop(int fd, void *message, size_t length) {
usec_t end;
int r;
assert(fd >= 0);
assert(message);
end = usec_add(now(CLOCK_MONOTONIC), SEND_TIMEOUT_USEC);
for (;;) {
if (write(fd, message, length) >= 0)
return 0;
@@ -903,9 +912,12 @@ static int write_loop(int fd, void *message, size_t length) {
if (errno != EAGAIN)
return -errno;
r = fd_wait_for_event(fd, POLLOUT, SEND_TIMEOUT_USEC);
if (r < 0)
r = fd_wait_for_event(fd, POLLOUT, LESS_BY(end, now(CLOCK_MONOTONIC)));
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r))
continue;
return r;
}
if (r == 0)
return -ETIMEDOUT;
}
+13 -17
View File
@@ -235,8 +235,7 @@ int ask_password_plymouth(
if (notify < 0)
return -errno;
r = inotify_add_watch(notify, flag_file, IN_ATTRIB); /* for the link count */
if (r < 0)
if (inotify_add_watch(notify, flag_file, IN_ATTRIB) < 0) /* for the link count */
return -errno;
}
@@ -244,8 +243,7 @@ int ask_password_plymouth(
if (fd < 0)
return -errno;
r = connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un));
if (r < 0)
if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0)
return -errno;
if (FLAGS_SET(flags, ASK_PASSWORD_ACCEPT_CACHED)) {
@@ -469,10 +467,9 @@ int ask_password_tty(
new_termios.c_cc[VMIN] = 1;
new_termios.c_cc[VTIME] = 0;
if (tcsetattr(ttyfd, TCSADRAIN, &new_termios) < 0) {
r = -errno;
r = RET_NERRNO(tcsetattr(ttyfd, TCSADRAIN, &new_termios));
if (r < 0)
goto finish;
}
reset_tty = true;
}
@@ -496,11 +493,11 @@ int ask_password_tty(
else
timeout = USEC_INFINITY;
if (flag_file)
if (access(flag_file, F_OK) < 0) {
r = -errno;
if (flag_file) {
r = RET_NERRNO(access(flag_file, F_OK));
if (r < 0)
goto finish;
}
}
r = ppoll_usec(pollfd, notify >= 0 ? 2 : 1, timeout);
if (r == -EINTR)
@@ -752,10 +749,10 @@ int ask_password_agent(
r = -errno;
goto finish;
}
if (inotify_add_watch(notify, "/run/systemd/ask-password", IN_ATTRIB /* for mtime */) < 0) {
r = -errno;
r = RET_NERRNO(inotify_add_watch(notify, "/run/systemd/ask-password", IN_ATTRIB /* for mtime */));
if (r < 0)
goto finish;
}
}
fd = mkostemp_safe(temp);
@@ -818,10 +815,9 @@ int ask_password_agent(
final[sizeof(final)-10] = 's';
final[sizeof(final)-9] = 'k';
if (rename(temp, final) < 0) {
r = -errno;
r = RET_NERRNO(rename(temp, final));
if (r < 0)
goto finish;
}
zero(pollfd);
pollfd[FD_SOCKET].fd = socket_fd;
+1 -3
View File
@@ -92,7 +92,6 @@
*/
int barrier_create(Barrier *b) {
_unused_ _cleanup_(barrier_destroyp) Barrier *staging = b;
int r;
assert(b);
@@ -104,8 +103,7 @@ int barrier_create(Barrier *b) {
if (b->them < 0)
return -errno;
r = pipe2(b->pipe, O_CLOEXEC | O_NONBLOCK);
if (r < 0)
if (pipe2(b->pipe, O_CLOEXEC | O_NONBLOCK) < 0)
return -errno;
staging = NULL;
+10 -5
View File
@@ -12,6 +12,7 @@
#include <utmpx.h>
#include "alloc-util.h"
#include "errno-util.h"
#include "fd-util.h"
#include "hostname-util.h"
#include "io-util.h"
@@ -292,13 +293,15 @@ static int write_to_terminal(const char *tty, const char *message) {
assert(message);
fd = open(tty, O_WRONLY|O_NONBLOCK|O_NOCTTY|O_CLOEXEC);
if (fd < 0 || !isatty(fd))
if (fd < 0)
return -errno;
if (!isatty(fd))
return -ENOTTY;
p = message;
left = strlen(message);
end = now(CLOCK_MONOTONIC) + TIMEOUT_USEC;
end = usec_add(now(CLOCK_MONOTONIC), TIMEOUT_USEC);
while (left > 0) {
ssize_t n;
@@ -306,19 +309,21 @@ static int write_to_terminal(const char *tty, const char *message) {
int k;
t = now(CLOCK_MONOTONIC);
if (t >= end)
return -ETIME;
k = fd_wait_for_event(fd, POLLOUT, end - t);
if (k < 0)
if (k < 0) {
if (ERRNO_IS_TRANSIENT(k))
continue;
return k;
}
if (k == 0)
return -ETIME;
n = write(fd, p, left);
if (n < 0) {
if (errno == EAGAIN)
if (ERRNO_IS_TRANSIENT(errno))
continue;
return -errno;
+9 -2
View File
@@ -1025,7 +1025,7 @@ static void handle_revents(Varlink *v, int revents) {
if ((revents & (POLLOUT|POLLHUP)) == 0)
return;
varlink_log(v, "Anynchronous connection completed.");
varlink_log(v, "Asynchronous connection completed.");
v->connecting = false;
} else {
/* Note that we don't care much about POLLIN/POLLOUT here, we'll just try reading and writing
@@ -1075,6 +1075,9 @@ int varlink_wait(Varlink *v, usec_t timeout) {
return events;
r = fd_wait_for_event(fd, events, t);
if (r < 0 && ERRNO_IS_TRANSIENT(r)) /* Treat EINTR as not a timeout, but also nothing happened, and
* the caller gets a chance to call back into us */
return 1;
if (r <= 0)
return r;
@@ -1161,8 +1164,12 @@ int varlink_flush(Varlink *v) {
}
r = fd_wait_for_event(v->fd, POLLOUT, USEC_INFINITY);
if (r < 0)
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r))
continue;
return varlink_log_errno(v, r, "Poll failed on fd: %m");
}
assert(r != 0);
+4 -1
View File
@@ -242,8 +242,11 @@ static int run(int argc, char *argv[]) {
};
r = ppoll_usec(p, ELEMENTSOF(p), t);
if (r < 0)
if (r < 0) {
if (ERRNO_IS_TRANSIENT(r)) /* don't be bothered by signals, i.e. EINTR */
continue;
return log_error_errno(r, "ppoll() failed: %m");
}
}
return 0;