iochannel: remove fd from poll() when we don't care from events

This should make it unlikely that we loop on SIGHUP indefinitely.

Also, this makes it possible for callbacks not to process all events and
still not busy loop.

2 files changed, 75 insertions, 57 deletions

diff --git a/src/pulsecore/iochannel.c b/src/pulsecore/iochannel.c
index b40c9815..48330079 100644
--- a/src/pulsecore/iochannel.c
+++ b/src/pulsecore/iochannel.c
@@ -28,7 +28,6 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
-
 #ifdef HAVE_SYS_SOCKET_H
 #include <sys/socket.h>
 #endif
@@ -65,24 +64,71 @@ struct pa_iochannel {
     pa_io_event* input_event, *output_event;
 };
 
-static void enable_mainloop_sources(pa_iochannel *io) {
+static void callback(pa_mainloop_api* m, pa_io_event *e, int fd, pa_io_event_flags_t f, void *userdata);
+
+static void delete_events(pa_iochannel *io) {
+    pa_assert(io);
+
+    if (io->input_event)
+        io->mainloop->io_free(io->input_event);
+
+    if (io->output_event && io->output_event != io->input_event)
+        io->mainloop->io_free(io->output_event);
+
+    io->input_event = io->output_event = NULL;
+}
+
+static void enable_events(pa_iochannel *io) {
     pa_assert(io);
 
-    if (io->input_event == io->output_event && io->input_event) {
+    if (io->hungup) {
+        delete_events(io);
+        return;
+    }
+
+    if (io->ifd == io->ofd && io->ifd >= 0) {
         pa_io_event_flags_t f = PA_IO_EVENT_NULL;
-        pa_assert(io->input_event);
 
         if (!io->readable)
             f |= PA_IO_EVENT_INPUT;
         if (!io->writable)
             f |= PA_IO_EVENT_OUTPUT;
 
-        io->mainloop->io_enable(io->input_event, f);
+        pa_assert(io->input_event == io->output_event);
+
+        if (f != PA_IO_EVENT_NULL) {
+            if (io->input_event)
+                io->mainloop->io_enable(io->input_event, f);
+            else
+                io->input_event = io->output_event = io->mainloop->io_new(io->mainloop, io->ifd, f, callback, io);
+        } else
+            delete_events(io);
+
     } else {
-        if (io->input_event)
-            io->mainloop->io_enable(io->input_event, io->readable ? PA_IO_EVENT_NULL : PA_IO_EVENT_INPUT);
-        if (io->output_event)
-            io->mainloop->io_enable(io->output_event, io->writable ? PA_IO_EVENT_NULL : PA_IO_EVENT_OUTPUT);
+
+        if (io->ifd >= 0) {
+            if (!io->readable) {
+                if (io->input_event)
+                    io->mainloop->io_enable(io->input_event, PA_IO_EVENT_INPUT);
+                else
+                    io->input_event = io->mainloop->io_new(io->mainloop, io->ifd, PA_IO_EVENT_INPUT, callback, io);
+            } else if (io->input_event) {
+                io->mainloop->io_free(io->input_event);
+                io->input_event = NULL;
+            }
+        }
+
+        if (io->ofd >= 0) {
+            if (!io->writable) {
+                if (io->output_event)
+                    io->mainloop->io_enable(io->output_event, PA_IO_EVENT_OUTPUT);
+                else
+                    io->output_event = io->mainloop->io_new(io->mainloop, io->ofd, PA_IO_EVENT_OUTPUT, callback, io);
+            } else if (io->input_event) {
+                io->mainloop->io_free(io->output_event);
+                io->output_event = NULL;
+            }
+        }
     }
 }
 
@@ -113,7 +159,7 @@ static void callback(pa_mainloop_api* m, pa_io_event *e, int fd, pa_io_event_fla
     }
 
     if (changed) {
-        enable_mainloop_sources(io);
+        enable_events(io);
 
         if (io->callback)
             io->callback(io, io->userdata);
@@ -126,49 +172,25 @@ pa_iochannel* pa_iochannel_new(pa_mainloop_api*m, int ifd, int ofd) {
     pa_assert(m);
     pa_assert(ifd >= 0 || ofd >= 0);
 
-    io = pa_xnew(pa_iochannel, 1);
+    io = pa_xnew0(pa_iochannel, 1);
     io->ifd = ifd;
     io->ofd = ofd;
-    io->ifd_type = io->ofd_type = 0;
     io->mainloop = m;
 
-    io->userdata = NULL;
-    io->callback = NULL;
-    io->readable = FALSE;
-    io->writable = FALSE;
-    io->hungup = FALSE;
-    io->no_close = FALSE;
-
-    io->input_event = io->output_event = NULL;
-
-    if (ifd == ofd) {
-        pa_assert(ifd >= 0);
+    if (io->ifd >= 0)
         pa_make_fd_nonblock(io->ifd);
-        io->input_event = io->output_event = m->io_new(m, ifd, PA_IO_EVENT_INPUT|PA_IO_EVENT_OUTPUT, callback, io);
-    } else {
-
-        if (ifd >= 0) {
-            pa_make_fd_nonblock(io->ifd);
-            io->input_event = m->io_new(m, ifd, PA_IO_EVENT_INPUT, callback, io);
-        }
 
-        if (ofd >= 0) {
-            pa_make_fd_nonblock(io->ofd);
-            io->output_event = m->io_new(m, ofd, PA_IO_EVENT_OUTPUT, callback, io);
-        }
-    }
+    if (io->ofd >= 0 && io->ofd != io->ifd)
+        pa_make_fd_nonblock(io->ofd);
 
+    enable_events(io);
     return io;
 }
 
 void pa_iochannel_free(pa_iochannel*io) {
     pa_assert(io);
 
-    if (io->input_event)
-        io->mainloop->io_free(io->input_event);
-
-    if (io->output_event && (io->output_event != io->input_event))
-        io->mainloop->io_free(io->output_event);
+    delete_events(io);
 
     if (!io->no_close) {
         if (io->ifd >= 0)
@@ -207,8 +229,8 @@ ssize_t pa_iochannel_write(pa_iochannel*io, const void*data, size_t l) {
     pa_assert(io->ofd >= 0);
 
     if ((r = pa_write(io->ofd, data, l, &io->ofd_type)) >= 0) {
-        io->writable = FALSE;
-        enable_mainloop_sources(io);
+        io->writable = io->hungup = FALSE;
+        enable_events(io);
     }
 
     return r;
@@ -222,8 +244,12 @@ ssize_t pa_iochannel_read(pa_iochannel*io, void*data, size_t l) {
     pa_assert(io->ifd >= 0);
 
     if ((r = pa_read(io->ifd, data, l, &io->ifd_type)) >= 0) {
-        io->readable = FALSE;
-        enable_mainloop_sources(io);
+
+        /* We also reset the hangup flag here to ensure that another
+         * IO callback is triggered so that we will again call into
+         * user code */
+        io->readable = io->hungup = FALSE;
+        enable_events(io);
     }
 
     return r;
@@ -296,18 +322,15 @@ ssize_t pa_iochannel_write_with_creds(pa_iochannel*io, const void*data, size_t l
         u->gid = getgid();
     }
 
-    memset(&mh, 0, sizeof(mh));
-    mh.msg_name = NULL;
-    mh.msg_namelen = 0;
+    pa_zero(mh);
     mh.msg_iov = &iov;
     mh.msg_iovlen = 1;
     mh.msg_control = &cmsg;
     mh.msg_controllen = sizeof(cmsg);
-    mh.msg_flags = 0;
 
     if ((r = sendmsg(io->ofd, &mh, MSG_NOSIGNAL)) >= 0) {
-        io->writable = FALSE;
-        enable_mainloop_sources(io);
+        io->writable = io->hungup = FALSE;
+        enable_events(io);
     }
 
     return r;
@@ -363,8 +386,8 @@ ssize_t pa_iochannel_read_with_creds(pa_iochannel*io, void*data, size_t l, pa_cr
             }
         }
 
-        io->readable = FALSE;
-        enable_mainloop_sources(io);
+        io->readable = io->hungup = FALSE;
+        enable_events(io);
     }
 
     return r;
diff --git a/src/pulsecore/iochannel.h b/src/pulsecore/iochannel.h
index 9050df90..180245b8 100644
--- a/src/pulsecore/iochannel.h
+++ b/src/pulsecore/iochannel.h
@@ -38,11 +38,6 @@
    the channel a callback function is called. It is safe to destroy
    the calling iochannel object from the callback */
 
-/* When pa_iochannel_is_readable() returns non-zero, the user has to
- * call this function in a loop until it is no longer set or EOF
- * reached. Otherwise strange things may happen when an EOF is
- * reached. */
-
 typedef struct pa_iochannel pa_iochannel;
 
 /* Create a new IO channel for the specified file descriptors for