From: Tobias Brunner Date: Fri, 21 Jul 2023 08:01:41 +0000 (+0200) Subject: watcher: Change handling of POLLERR and remove WATCHER_EXCEPT X-Git-Tag: android-2.4.2~7^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ba9228ab00c4babb4272da9d97f1c4179fc1e85f;p=thirdparty%2Fstrongswan.git watcher: Change handling of POLLERR and remove WATCHER_EXCEPT We can't actually explicitly listen for errors by passing POLLERR in `events` (the man page for poll() clearly states it's ignored). On the other hand, POLLERR can be returned for any FD and, even worse, it might be the only event indicated. The latter caused an infinite loop as we didn't notify the callback nor clear the error by calling `getsockopt(..., SOL_SOCKET, SO_ERROR, ...)`. And while the latter would be able to reset the state to break the loop, it seems to leave the FD in a defunct state where no further events will be returned by poll(). Notifying the callback works better (the error is then reported by e.g. recvfrom()) and automatically happened already if POLLERR was returned together with e.g. POLLIN. So we now treat POLLERR like the other error indicators we handle (POLLHUP and POLLINVAL) and just notify the callbacks. --- diff --git a/src/libstrongswan/networking/streams/stream.c b/src/libstrongswan/networking/streams/stream.c index 8273af89e0..f1eb5a0971 100644 --- a/src/libstrongswan/networking/streams/stream.c +++ b/src/libstrongswan/networking/streams/stream.c @@ -188,8 +188,6 @@ static bool watch(private_stream_t *this, int fd, watcher_event_t event) this->write_cb = cb; } break; - case WATCHER_EXCEPT: - break; } return keep; } diff --git a/src/libstrongswan/processing/watcher.c b/src/libstrongswan/processing/watcher.c index e174858ef6..1200d67095 100644 --- a/src/libstrongswan/processing/watcher.c +++ b/src/libstrongswan/processing/watcher.c @@ -251,20 +251,17 @@ static void notify_end(notify_data_t *data) if (removed) { - DBG3(DBG_JOB, "removed fd %d[%s%s%s] from watcher after callback", data->fd, + DBG3(DBG_JOB, "removed fd %d[%s%s] from watcher after callback", data->fd, data->event & WATCHER_READ ? "r" : "", - data->event & WATCHER_WRITE ? "w" : "", - data->event & WATCHER_EXCEPT ? "e" : ""); + data->event & WATCHER_WRITE ? "w" : ""); } else if (updated) { - DBG3(DBG_JOB, "updated fd %d[%s%s%s] to %d[%s%s%s] after callback", data->fd, + DBG3(DBG_JOB, "updated fd %d[%s%s] to %d[%s%s] after callback", data->fd, (updated | data->event) & WATCHER_READ ? "r" : "", - (updated | data->event) & WATCHER_WRITE ? "w" : "", - (updated | data->event) & WATCHER_EXCEPT ? "e" : "", data->fd, + (updated | data->event) & WATCHER_WRITE ? "w" : "", data->fd, updated & WATCHER_READ ? "r" : "", - updated & WATCHER_WRITE ? "w" : "", - updated & WATCHER_EXCEPT ? "e" : ""); + updated & WATCHER_WRITE ? "w" : ""); } free(data); } @@ -335,27 +332,6 @@ static inline int find_revents(struct pollfd *pfd, int count, int fd) return 0; } -/** - * Check if entry is waiting for a specific event, and if it got signaled - */ -static inline bool entry_ready(entry_t *entry, watcher_event_t event, - int revents) -{ - if (entry->events & event) - { - switch (event) - { - case WATCHER_READ: - return (revents & (POLLIN | POLLHUP | POLLNVAL)) != 0; - case WATCHER_WRITE: - return (revents & (POLLOUT | POLLHUP | POLLNVAL)) != 0; - case WATCHER_EXCEPT: - return (revents & (POLLERR | POLLHUP | POLLNVAL)) != 0; - } - } - return FALSE; -} - #if DEBUG_LEVEL >= 2 #define reset_log(buf, pos, len) ({ buf[0] = '\0'; pos = buf; len = sizeof(buf); }) #define reset_event_log(buf, pos) ({ pos = buf; }) @@ -431,11 +407,6 @@ static job_requeue_t watch(private_watcher_t *this) log_event(eventpos, 'w'); pfd[count].events |= POLLOUT; } - if (entry->events & WATCHER_EXCEPT) - { - log_event(eventpos, 'e'); - pfd[count].events |= POLLERR; - } end_event_log(eventpos); log_fd(logpos, loglen, entry->fd, eventbuf); count++; @@ -505,23 +476,27 @@ static job_requeue_t watch(private_watcher_t *this) } reset_event_log(eventbuf, eventpos); revents = find_revents(pfd, count, entry->fd); - if (entry_ready(entry, WATCHER_EXCEPT, revents)) + if (revents & POLLERR) { log_event(eventpos, 'e'); - notify(this, entry, WATCHER_EXCEPT); } - else + if (revents & POLLIN) { - if (entry_ready(entry, WATCHER_READ, revents)) - { - log_event(eventpos, 'r'); - notify(this, entry, WATCHER_READ); - } - if (entry_ready(entry, WATCHER_WRITE, revents)) - { - log_event(eventpos, 'w'); - notify(this, entry, WATCHER_WRITE); - } + log_event(eventpos, 'r'); + } + if (revents & POLLOUT) + { + log_event(eventpos, 'w'); + } + if (entry->events & WATCHER_READ && + revents & (POLLIN | POLLERR | POLLHUP | POLLNVAL)) + { + notify(this, entry, WATCHER_READ); + } + if (entry->events & WATCHER_WRITE && + revents & (POLLOUT | POLLERR | POLLHUP | POLLNVAL)) + { + notify(this, entry, WATCHER_WRITE); } end_event_log(eventpos); log_fd(logpos, loglen, entry->fd, eventbuf); @@ -571,10 +546,9 @@ METHOD(watcher_t, add, void, .data = data, ); - DBG3(DBG_JOB, "adding fd %d[%s%s%s] to watcher", fd, + DBG3(DBG_JOB, "adding fd %d[%s%s] to watcher", fd, events & WATCHER_READ ? "r" : "", - events & WATCHER_WRITE ? "w" : "", - events & WATCHER_EXCEPT ? "e" : ""); + events & WATCHER_WRITE ? "w" : ""); this->mutex->lock(this->mutex); add_entry(this, entry); @@ -631,10 +605,9 @@ METHOD(watcher_t, remove_, void, { update_and_unlock(this); - DBG3(DBG_JOB, "removed fd %d[%s%s%s] from watcher", fd, + DBG3(DBG_JOB, "removed fd %d[%s%s] from watcher", fd, found & WATCHER_READ ? "r" : "", - found & WATCHER_WRITE ? "w" : "", - found & WATCHER_EXCEPT ? "e" : ""); + found & WATCHER_WRITE ? "w" : ""); } else { diff --git a/src/libstrongswan/processing/watcher.h b/src/libstrongswan/processing/watcher.h index 8a5fababe3..e0db20fdc6 100644 --- a/src/libstrongswan/processing/watcher.h +++ b/src/libstrongswan/processing/watcher.h @@ -55,7 +55,6 @@ typedef bool (*watcher_cb_t)(void *data, int fd, watcher_event_t event); enum watcher_event_t { WATCHER_READ = (1<<0), WATCHER_WRITE = (1<<1), - WATCHER_EXCEPT = (1<<2), }; /**