On Thu, Mar 29, 2018 at 15:09 +0200, Lukas Larsson wrote:
> Hello,
>
> I've been re-writing the polling mechanisms in the Erlang VM and stumbled
> across
> something that might be a bug in the OpenBSD implementation of kqueue.
>
> When using EV_DISPATCH, the event is never triggered again after the EV_EOF
> flag has been delivered, even though there is more data to be read from the
> socket.
>
> I've attached a smallish program that shows the problem.
>
> The shortened ktrace output looks like this on OpenBSD 6.2:
>
> 29672 a.out 0.012883 CALL kevent(4,0x7f7ffffe8220,1,0,0,0)
> 29672 a.out 0.012888 STRU struct kevent { ident=5, filter=EVFILT_READ,
> flags=0x81<EV_ADD|EV_DISPATCH>, fflags=0<>, data=0, udata=0x0 }
> 29672 a.out 0.012895 RET kevent 0
> 29672 a.out 0.012904 CALL kevent(4,0,0,0x7f7ffffe7cf0,32,0)
> 29672 a.out 0.013408 STRU struct kevent { ident=5, filter=EVFILT_READ,
> flags=0x81<EV_ADD|EV_DISPATCH>, fflags=0<>, data=6, udata=0x0 }
> 29672 a.out 0.013493 RET kevent 1
> 29672 a.out 0.013548 CALL read(5,0x7f7ffffe8286,0x2)
> 29672 a.out 0.013562 RET read 2
> 29672 a.out 0.013590 CALL kevent(4,0x7f7ffffe8220,1,0,0,0)
> 29672 a.out 0.013594 STRU struct kevent { ident=5, filter=EVFILT_READ,
> flags=0x84<EV_ENABLE|EV_DISPATCH>, fflags=0<>, data=0, udata=0x0 }
> 29672 a.out 0.013608 RET kevent 0
> 29672 a.out 1.022228 CALL kevent(4,0,0,0x7f7ffffe7cf0,32,0)
> 29672 a.out 1.022537 STRU struct kevent { ident=5, filter=EVFILT_READ,
> flags=0x8081<EV_ADD|EV_DISPATCH|EV_EOF>, fflags=0<>, data=4, udata=0x0 }
> 29672 a.out 1.022572 RET kevent 1
> 29672 a.out 1.022663 CALL read(5,0x7f7ffffe8286,0x2)
> 29672 a.out 1.022707 RET read 2
> 29672 a.out 1.022816 CALL kevent(4,0x7f7ffffe8220,1,0,0,0)
> 29672 a.out 1.022822 STRU struct kevent { ident=5, filter=EVFILT_READ,
> flags=0x84<EV_ENABLE|EV_DISPATCH>, fflags=0<>, data=0, udata=0x0 }
> 29672 a.out 1.022835 RET kevent 0
> 29672 a.out 2.032238 CALL kevent(4,0,0,0x7f7ffffe7cf0,32,0)
> 29672 a.out 5.277194 PSIG SIGINT SIG_DFL
>
> In this example I would have expected the last kevent call to return with
> EV_EOF and
> data set to 2, but it does not trigger again. If I don't use EV_DISPATCH,
> the event is
> triggered again and the program terminates.
>
> Does anyone know if this is the expected behavior or a bug?
>
> I've worked around this issue by using EV_ONESHOT instead of EV_DISPATCH on
> OpenBSD for now, but would like to use EV_DISPATCH in the future as I've
> found
> that it aligns better with the abstractions that I use, and could possibly
> be a little bit
> more performant.
>
> Lukas
>
> PS. If relevant, it seems like FreeBSD does behave the way that I expected,
> i.e.
> it triggers again for EV_DISPATCH after EV_EOF has been shown. DS.
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <stdint.h>
> #include <err.h>
> #include <sys/event.h>
>
> #include <sys/socket.h>
> #include <sys/un.h>
> #include <netdb.h>
> #include <stdio.h>
> #include <stdarg.h>
> #include <string.h>
>
> #define USE_DISPATCH 1
>
> int main() {
> struct addrinfo *addr;
> struct addrinfo hints;
> int kq, listen_s, fd = -1;
> struct kevent evSet;
> struct kevent evList[32];
>
> /* open a TCP socket */
> memset(&hints, 0, sizeof hints);
> hints.ai_family = PF_UNSPEC; /* any supported protocol */
> hints.ai_flags = AI_PASSIVE; /* result for bind() */
> hints.ai_socktype = SOCK_STREAM; /* TCP */
> int error = getaddrinfo ("127.0.0.1", "8080", &hints, &addr);
> if (error)
> errx(1, "getaddrinfo failed: %s", gai_strerror(error));
> listen_s = socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
> if (setsockopt(listen_s, SOL_SOCKET, SO_REUSEADDR, &(int){ 1 },
> sizeof(int)) < 0)
> errx(1, "setsockopt(SO_REUSEADDR) failed");
> bind(listen_s, addr->ai_addr, addr->ai_addrlen);
> listen(listen_s, 5);
>
> kq = kqueue();
>
> system("echo -n abcdef | nc -v -w 1 127.0.0.1 8080 &");
>
> EV_SET(&evSet, listen_s, EVFILT_READ, EV_ADD, 0, 0, NULL);
> if (kevent(kq, &evSet, 1, NULL, 0, NULL) == -1)
> err(1, "kevent");
>
> while(1) {
> int i;
> int nev = kevent(kq, NULL, 0, evList, 32, NULL);
> for (i = 0; i < nev; i++) {
> if (evList[i].ident == listen_s) {
> struct sockaddr_storage addr;
> socklen_t socklen = sizeof(addr);
> if (fd != -1)
> close(fd);
> fd = accept(evList[i].ident, (struct sockaddr *)&addr,
> &socklen);
> printf("accepted %d\n", fd);
> #if USE_DISPATCH
> EV_SET(&evSet, fd, EVFILT_READ, EV_ADD|EV_DISPATCH, 0, 0,
> NULL);
> #else
> EV_SET(&evSet, fd, EVFILT_READ, EV_ADD, 0, 0, NULL);
> #endif
> if (kevent(kq, &evSet, 1, NULL, 0, NULL) == -1)
> err(1, "kevent");
> } else {
> if (evList[i].flags & EV_EOF && evList[i].data == 0) {
> printf("closing %d\n", fd);
> close(fd);
> fd = -1;
> exit(0);
> } else if (evList[i].filter == EVFILT_READ) {
> char buff[2];
> read(fd, buff, 2);
> if (evList[i].flags & EV_EOF) printf("EOF: ");
> printf("read %c%c from %d\n", buff[0], buff[1], fd);
> #if USE_DISPATCH
> EV_SET(&evSet, fd, EVFILT_READ, EV_ENABLE|EV_DISPATCH, 0,
> 0, NULL);
> if (kevent(kq, &evSet, 1, NULL, 0, NULL) == -1)
> err(1, "kevent");
> #endif
> sleep(1);
> }
> }
> }
> }
> }
Hi,
This appears to be an issue with reactivating disabled event sources
in kqueue_register. Something along the lines of FreeBSD commits:
https://svnweb.freebsd.org/base?view=revision&revision=274560 and
https://reviews.freebsd.org/rS295786 where parent differential review
https://reviews.freebsd.org/D5307 has some additional comments.
In any case, by either porting their code (#else branch) or slightly
adjusting our own (I think that should be enough), I can no longer
reproduce the issue you've reported. Please test and report back if
that solves your original issue. Either variants will require
rigorous testing and a thorough review.
Cheers,
Mike
diff --git sys/kern/kern_event.c sys/kern/kern_event.c
index fb9cad360b1..8a999b24923 100644
--- sys/kern/kern_event.c
+++ sys/kern/kern_event.c
@@ -661,25 +661,39 @@ kqueue_register(struct kqueue *kq, struct kevent *kev,
struct proc *p)
kn->kn_fop->f_detach(kn);
knote_drop(kn, p, p->p_fd);
goto done;
}
+#if 1
if ((kev->flags & EV_DISABLE) &&
((kn->kn_status & KN_DISABLED) == 0)) {
s = splhigh();
kn->kn_status |= KN_DISABLED;
splx(s);
}
if ((kev->flags & EV_ENABLE) && (kn->kn_status & KN_DISABLED)) {
s = splhigh();
kn->kn_status &= ~KN_DISABLED;
- if ((kn->kn_status & KN_ACTIVE) &&
- ((kn->kn_status & KN_QUEUED) == 0))
- knote_enqueue(kn);
+ if (kn->kn_fop->f_event(kn, 0))
+ KNOTE_ACTIVATE(kn);
splx(s);
}
+#else
+ s = splhigh();
+ if ((kev->flags & EV_ENABLE) != 0)
+ kn->kn_status &= ~KN_DISABLED;
+ else if ((kev->flags & EV_DISABLE) != 0)
+ kn->kn_status |= KN_DISABLED;
+
+ if (!(kn->kn_status & KN_DISABLED) && kn->kn_fop->f_event(kn, 0))
+ kn->kn_status |= KN_ACTIVE;
+ if ((kn->kn_status & (KN_ACTIVE | KN_DISABLED | KN_QUEUED)) ==
+ KN_ACTIVE)
+ knote_enqueue(kn);
+ splx(s);
+#endif
done:
if (fp != NULL)
FRELE(fp, p);
return (error);
