[PATCH] Fix ETIME handling for Solaris event ports

[Ralph Boehme]

Hi Nathan,

On Tue, Feb 02, 2016 at 10:52:55AM -0700, Nathan Huff wrote:
> 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 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.

good catch!

Are you 100% sure we mustn't apply the same logic to EINTR?

The manpage reads [1]:

  ...
  
  The port_getn() function block until the desired number of events
  are available, the timeout elapses, a signal occurs, a port is
  closed by another thread, or the port is in or enters alert mode.

  On return, the value pointed to by nget is updated to the actual
  number of events retrieved in list.

  ...

The second paragraph stands on its own so to me this implies on
*every* (non-fatal) return we may have some events.

I think a simple test binary could be written that sets up an event
port in a process, forks, and then, in the child at the right moment,
sends a signal to the parent.

-Ralph

[1] <http://docs.oracle.com/cd/E19253-01/816-5168/port-get-3c/index.html>

> diff --git a/lib/tevent/tevent_port.c b/lib/tevent/tevent_port.c
> index 5b487d7..4b06e0e 100644
> --- a/lib/tevent/tevent_port.c
> +++ b/lib/tevent/tevent_port.c
> @@ -484,29 +484,29 @@ 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 (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++) {


-- 
Schönen Gruß
Ralph Böhme

-- 
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