Hi,
I discovered what caused the strange event loss in syslogd during
the hackaton. I had mixed EV_READ and EV_WRITE events on the ev_read
and ev_write event structures. The correct way is to use each event
for its read and write purpose and instead switch the handler. Then
libevent is no longer confused.
ok?
bluhm
Index: usr.sbin/syslogd/evbuffer_tls.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/evbuffer_tls.c,v
retrieving revision 1.7
diff -u -p -r1.7 evbuffer_tls.c
--- usr.sbin/syslogd/evbuffer_tls.c 10 Sep 2015 18:32:06 -0000 1.7
+++ usr.sbin/syslogd/evbuffer_tls.c 18 Sep 2015 22:53:53 -0000
@@ -44,6 +44,9 @@
/* prototypes */
void bufferevent_read_pressure_cb(struct evbuffer *, size_t, size_t, void *);
+static void buffertls_readcb(int, short, void *);
+static void buffertls_writecb(int, short, void *);
+static void buffertls_handshakecb(int, short, void *);
int evtls_read(struct evbuffer *, int, int, struct tls *);
int evtls_write(struct evbuffer *, int, struct tls *);
@@ -96,29 +99,30 @@ buffertls_readcb(int fd, short event, vo
res = evtls_read(bufev->input, fd, howmuch, ctx);
switch (res) {
case TLS_WANT_POLLIN:
- event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb,
- buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ return;
case TLS_WANT_POLLOUT:
- event_set(&bufev->ev_read, fd, EV_WRITE, buffertls_readcb,
+ event_del(&bufev->ev_write);
+ event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_readcb,
buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
+ return;
case -1:
- if (errno == EAGAIN || errno == EINTR)
- goto reschedule;
- /* error case */
what |= EVBUFFER_ERROR;
break;
case 0:
- /* eof case */
what |= EVBUFFER_EOF;
break;
}
if (res <= 0)
goto error;
- event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
- bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ event_del(&bufev->ev_write);
+ event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
+ if (bufev->enabled & EV_READ)
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled & EV_WRITE)
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
/* See if this callbacks meets the water marks */
len = EVBUFFER_LENGTH(bufev->input);
@@ -137,10 +141,6 @@ buffertls_readcb(int fd, short event, vo
(*bufev->readcb)(bufev, bufev->cbarg);
return;
- reschedule:
- bufferevent_add(&bufev->ev_read, bufev->timeout_read);
- return;
-
error:
(*bufev->errorcb)(bufev, what, bufev->cbarg);
}
@@ -163,22 +163,18 @@ buffertls_writecb(int fd, short event, v
res = evtls_write(bufev->output, fd, ctx);
switch (res) {
case TLS_WANT_POLLIN:
- event_set(&bufev->ev_write, fd, EV_READ,
+ event_del(&bufev->ev_read);
+ event_set(&bufev->ev_read, fd, EV_READ,
buffertls_writecb, buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ return;
case TLS_WANT_POLLOUT:
- event_set(&bufev->ev_write, fd, EV_WRITE,
- buffertls_writecb, buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
+ return;
case -1:
- if (errno == EAGAIN || errno == EINTR ||
- errno == EINPROGRESS)
- goto reschedule;
- /* error case */
what |= EVBUFFER_ERROR;
break;
case 0:
- /* eof case */
what |= EVBUFFER_EOF;
break;
}
@@ -186,8 +182,11 @@ buffertls_writecb(int fd, short event, v
goto error;
}
- event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
- if (EVBUFFER_LENGTH(bufev->output) != 0)
+ event_del(&bufev->ev_read);
+ event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
+ if (bufev->enabled & EV_READ)
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled & EV_WRITE)
bufferevent_add(&bufev->ev_write, bufev->timeout_write);
/*
@@ -200,11 +199,6 @@ buffertls_writecb(int fd, short event, v
return;
- reschedule:
- if (EVBUFFER_LENGTH(bufev->output) != 0)
- bufferevent_add(&bufev->ev_write, bufev->timeout_write);
- return;
-
error:
(*bufev->errorcb)(bufev, what, bufev->cbarg);
}
@@ -226,41 +220,30 @@ buffertls_handshakecb(int fd, short even
res = tls_handshake(ctx);
switch (res) {
case TLS_WANT_POLLIN:
- event_set(&bufev->ev_write, fd, EV_READ,
- buffertls_handshakecb, buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ return;
case TLS_WANT_POLLOUT:
- event_set(&bufev->ev_write, fd, EV_WRITE,
- buffertls_handshakecb, buftls);
- goto reschedule;
+ bufferevent_add(&bufev->ev_write, bufev->timeout_write);
+ return;
case -1:
- if (errno == EAGAIN || errno == EINTR ||
- errno == EINPROGRESS)
- goto reschedule;
- /* error case */
what |= EVBUFFER_ERROR;
break;
}
if (res < 0)
goto error;
- /*
- * There might be data available in the tls layer. Try
- * an read operation and setup the callbacks. Call the read
- * callback after enabling the write callback to give the
- * read error handler a chance to disable the write event.
- */
+ /* Handshake was successful, change to read and write callback. */
+ event_del(&bufev->ev_read);
+ event_del(&bufev->ev_write);
+ event_set(&bufev->ev_read, fd, EV_READ, buffertls_readcb, buftls);
event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_writecb, buftls);
- if (EVBUFFER_LENGTH(bufev->output) != 0)
+ if (bufev->enabled & EV_READ)
+ bufferevent_add(&bufev->ev_read, bufev->timeout_read);
+ if (EVBUFFER_LENGTH(bufev->output) != 0 && bufev->enabled & EV_WRITE)
bufferevent_add(&bufev->ev_write, bufev->timeout_write);
- buffertls_readcb(fd, 0, buftls);
return;
- reschedule:
- bufferevent_add(&bufev->ev_write, bufev->timeout_write);
- return;
-
error:
(*bufev->errorcb)(bufev, what, bufev->cbarg);
}
@@ -283,7 +266,7 @@ buffertls_connect(struct buffertls *buft
event_del(&bufev->ev_read);
event_del(&bufev->ev_write);
-
+ event_set(&bufev->ev_read, fd, EV_READ, buffertls_handshakecb, buftls);
event_set(&bufev->ev_write, fd, EV_WRITE, buffertls_handshakecb,
buftls);
bufferevent_add(&bufev->ev_write, bufev->timeout_write);
