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.

diff --git a/src/pulsecore/iochannel.c b/src/pulsecore/iochannel.c
index b40c9815643db5a524b14c3714eda2d875aeda65..483300795941661ecd1d1f19587e65a85fe9cfab 100644 (file)
--- 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 9050df90f6fef1a3de8f1fae72e0aa50d662d49c..180245b8b7c08d992ddf29f5c5624519c2158f4c 100644 (file)
--- 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