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 8273af89e06bdd8d2e655d346111402a598b4fa3..f1eb5a09710afdcbe56a862c417813c25b142ebe 100644 (file)
--- 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 e174858ef67bcfb86f564970703d131409451197..1200d670959e2ed78db70199f52dff8a3d9ef3bc 100644 (file)
--- 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 8a5fababe38f5609ed7ecdb8c11d7a233b514b20..e0db20fdc6f62a6e4f7bd52b89dce3a53c96a6c4 100644 (file)
--- 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),
 };
 
 /**