[PATCH] Fix ETIME handling for Solaris event ports

[Nathan Huff]

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.

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

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