[PATCH] Fix ETIME handling for Solaris event ports

[Nathan Huff]

Turns out EINTR is even more messed up than ETIME.  It is possible to not
get an event and not have nget updated either so you have to actually check
the event structure you get back.  I have attached an updated patch to deal
with that situation as well.

On Wed, Feb 3, 2016 at 11:45 AM, Ralph Boehme <rb at sernet.de> wrote:

> On Wed, Feb 03, 2016 at 11:19:49AM -0700, Nathan Huff wrote:
> > I am not actually sure if you can get events on EINTR or not and I am not
> > sure if anybody else is really sure either.
>
> :)
>
> > I went through and looked what gets handled by other event libraries
> > like libevent and libev.  Some of them only treat ETIME specially
> > and some of them also check EINTR as well.  I have actually seen the
> > ETIME case and could get it to reproduce by running wbinfo -p in a
> > tight loop.  I can update the patch to also check on EINTR which
> > shouldn't actually hurt anything even if we never get events.
>
> yes please! Can you also submit your patch as git format-patch and add
> your signed-off [1]? Thanks!
>
> -Ralph
>
> [1] <https://wiki.samba.org/index.php/CodeReview#commit_message_tags>
>
>
> --
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de,mailto:kontakt@sernet.de
>
-------------- next part --------------
From c7790ac220217e4d6c03750e11eb2c03ace4df83 Mon Sep 17 00:00:00 2001
From: Nathan Huff <nhuff at acm.org>
Date: Wed, 3 Feb 2016 13:06:52 -0700
Subject: [PATCH] Handle ETIME and EINTR handling for solaris event ports.

It is quite possible to get a return of -1 and a errno of ETIME from
port_getn and also get an event returned. The attached patch checks for
the number of events returned and if it isn't 0 it falls through to the
event handling code.

If you get EINTR back it might be possible that you got an event, but it
also is possible that you didn't get an event and nget isn't updated so
you need to check whether the event list was actually filled in. The
patch sets a port source value that isn't valid in the first event so
that we can check if the event structure was actually initialized.

If you don't do this Samba loses file descriptor associations since the
kernel has dissociated the fd from the event port and Samba doesn't process
the event and reassociate it.

Signed-off-by: Nathan Huff <nhuff at acm.org>
---
 lib/tevent/tevent_port.c | 54 ++++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/lib/tevent/tevent_port.c b/lib/tevent/tevent_port.c
index 5b487d7..8f7bb8b 100644
--- a/lib/tevent/tevent_port.c
+++ b/lib/tevent/tevent_port.c
@@ -458,6 +458,14 @@ static int port_event_loop(struct port_event_context *port_ev, struct timeval *t
 	struct timespec ts;
 	struct tevent_context *ev = port_ev->ev;
 
+	/*
+	 * Set to a known non valid value becuase if port_getn returns
+	 * EINTR and nget > 0 it may have returned an event or it may
+	 * have just not updated nget. We need to check if the events
+	 * list was actually filled in
+	 */
+	events[0].portev_source = 0;
+
 	if (tvalp) {
 		ts.tv_sec = tvalp->tv_sec;
 		ts.tv_nsec = tvalp->tv_usec * 1000;
@@ -484,29 +492,31 @@ static int port_event_loop(struct port_event_context *port_ev, struct timeval *t
 	port_errno = errno;
 	tevent_trace_point_callback(ev, TEVENT_TRACE_AFTER_WAIT);
 
-	if (ret == -1 && port_errno == EINTR) {
-		if (ev->signal_events) {
-			tevent_common_check_signal(ev);
-		}
-		/*
-		 * If no signal handlers we got an unsolicited
-		 * signal wakeup. This can happen with epoll
-		 * too. Just return and ignore.
-		 */
-		return 0;
-	}
-
-	if (ret == -1 && port_errno == ETIME && tvalp) {
-		/* we don't care about a possible delay here */
-		tevent_common_loop_timer_delay(ev);
-		return 0;
-	}
-
 	if (ret == -1) {
-		tevent_debug(ev, TEVENT_DEBUG_ERROR,
-				"port_get failed (%s)\n",
-				strerror(errno));
-		return -1;
+		if (port_errno == EINTR) {
+			if (events[0].portev_source == 0) {
+				if (ev->signal_events) {
+					tevent_common_check_signal(ev);
+				}
+				/*
+				 * If no signal handlers we got an unsolicited
+				 * signal wakeup. This can happen with epoll
+				 * too. Just return and ignore.
+				 */
+				return 0;
+			}
+		} else if (port_errno == ETIME) {
+			if (nget == 0 && tvalp) {
+				/* we don't care about a possible delay here */
+				tevent_common_loop_timer_delay(ev);
+				return 0;
+			}
+		} else {
+			tevent_debug(ev, TEVENT_DEBUG_ERROR,
+					"port_get failed (%s)\n",
+					strerror(errno));
+			return -1;
+		}
 	}
 
 	for (i = 0; i < nget; i++) {
-- 
2.5.0