(boost::)asio and kqueue problem

[Hartmut Brandt]

Hi,

I'm trying to use asio (that's boost::asio without boost) to handle 
listening sockets asynchronuosly. This appears not to work. There are also
some reports on the net about this problem. I was able to reproduce the 
problem with a small C-programm that does the same steps as asio. The 
relevant sequence of system calls is:

kqueue()					 = 3 (0x3)
socket(PF_INET,SOCK_STREAM,6)			 = 4 (0x4)
setsockopt(0x4,0xffff,0x800,0x7fffffffea2c,0x4)	 = 0 (0x0)
kevent(3,{ 4,EVFILT_READ,EV_ADD|EV_CLEAR,0x0,0x0,0x0 4,EVFILT_WRITE,EV_ADD|EV_CLEAR,0x0,0x0,0x0 },2,0x0,0,0x0) = 0 (0x0)
setsockopt(0x4,0xffff,0x4,0x7fffffffea2c,0x4)	 = 0 (0x0)
bind(4,{ AF_INET 0.0.0.0:8080 },16)		 = 0 (0x0)
listen(0x4,0x80)				 = 0 (0x0)
ioctl(4,FIONBIO,0xffffea2c)			 = 0 (0x0)
kevent(3,{ 4,EVFILT_READ,EV_ADD|EV_CLEAR,0x0,0x0,0x0 4,EVFILT_WRITE,EV_ADD|EV_CLEAR,0x0,0x0,0x0 },2,0x0,0,0x0) = 0 (0x0)
kevent(3,0x0,0,0x7fffffffe5a0,32,0x0)		 ERR#4 'Interrupted system call'

The problem here is that asio registers each file descriptor with 
EVFILT_READ and EVFILT_WRITE as soon as it is opened (first kevent call). 
After bringing the socket into the listening state and when async_accept() 
is called it registers the socket a second time. According to the man page 
this is perfectly legal and can be used to modify the registration.

With this sequence of calls kevent() does not return when a connection is 
established successfully.

I tracked down the problem and the reason is in soo_kqfilter(). This is 
called for the first EVFILT_READ registration and decides based on the 
SO_ACCEPTCONN flag which filter operations to use solisten_filtops or 
soread_filtops. In this case it chooses soread_filtops.

The second EVFILT_READ registration does not call soo_kqfilter() again, 
but just updates the filter from the data and fflags field so the 
listening socket ends up with the wrong filter operations.

The attached patch fixes this (kind of) by using the f_touch 
operation (currently used only by the user filter). The filt_sotouch() 
function changes the operation pointer in the knote when the socket is now 
in the listening state. I suppose that the required locking is already 
done in kqueue_register(), but I'm not sure. Asynchronous accepting now works.

A better fix would probably be to change the operation vector on all 
knotes attached to the socket in solisten(), but I fear I don't have the 
necessary understanding of the locking that is required for this.

Could somebody with enough kqueue() knowledge look whether the patch is 
correct lock-wise?

Regards,
harti

Index: kern_event.c
===================================================================
--- kern_event.c	(revision 302977)
+++ kern_event.c	(working copy)
@@ -1350,8 +1350,8 @@
  	KQ_UNLOCK(kq);
  	knl = kn_list_lock(kn);
  	kn->kn_kevent.udata = kev->udata;
-	if (!fops->f_isfd && fops->f_touch != NULL) {
-		fops->f_touch(kn, kev, EVENT_REGISTER);
+	if (kn->kn_fop->f_touch != NULL) {
+		kn->kn_fop->f_touch(kn, kev, EVENT_REGISTER);
  	} else {
  		kn->kn_sfflags = kev->fflags;
  		kn->kn_sdata = kev->data;
Index: uipc_socket.c
===================================================================
--- uipc_socket.c	(revision 302977)
+++ uipc_socket.c	(working copy)
@@ -160,6 +160,7 @@
  static void	filt_sowdetach(struct knote *kn);
  static int	filt_sowrite(struct knote *kn, long hint);
  static int	filt_solisten(struct knote *kn, long hint);
+static void	filt_sotouch(struct knote *kn, struct kevent *kev, u_long type);
  static int inline hhook_run_socket(struct socket *so, void *hctx, int32_t h_id);
  fo_kqfilter_t	soo_kqfilter;

@@ -172,6 +173,7 @@
  	.f_isfd = 1,
  	.f_detach = filt_sordetach,
  	.f_event = filt_soread,
+	.f_touch = filt_sotouch,
  };
  static struct filterops sowrite_filtops = {
  	.f_isfd = 1,
@@ -3091,6 +3093,31 @@
  	return (0);
  }

+static void
+filt_sotouch(struct knote *kn, struct kevent *kev, u_long type)
+{
+	struct socket *so = kn->kn_fp->f_data;
+
+	switch (type) {
+	case EVENT_REGISTER:
+		if (kn->kn_fop == &soread_filtops &&
+		    (so->so_options & SO_ACCEPTCONN))
+			kn->kn_fop = &solisten_filtops;
+
+		kn->kn_sfflags = kev->fflags;
+		kn->kn_sdata = kev->data;
+		break;
+
+        case EVENT_PROCESS:
+		*kev = kn->kn_kevent;
+		break;
+
+	default:
+		panic("filt_sotouch() - invalid type (%ld)", type);
+		break;
+	}
+}
+
  /*
   * Some routines that return EOPNOTSUPP for entry points that are not
   * supported by a protocol.  Fill in as needed.
-------------- next part --------------
#include <sys/socket.h>
#include <sys/types.h>
#include <sys/event.h>
#include <sys/filio.h>

#include <sys/ioctl.h>
#include <netinet/in.h>
#include <stdio.h>
#include <string.h>
#include <err.h>

static void
wait_loop(int kq, int sock)
{
	struct kevent ev[32];
	struct sockaddr_in addr;
	socklen_t socklen;

	for (;;) {
		int nev = kevent(kq, NULL, 0, ev, 32, NULL);
		if (nev < 1)
			err(1, "kevent");
		for (int i = 0; i < nev; ++i) {
			if (ev[i].ident == sock) {
				printf("accept\n");
				int fd = accept(ev[i].ident,
				    (struct sockaddr *)&addr, &socklen);
				if (fd == -1)
					err(1, "accept");
			}
		}
	}
}

int
main()
{
	struct sockaddr_in addr;

	/* open a TCP socket */
	int kq = kqueue();

	int sock = socket(PF_INET, SOCK_STREAM, 0);

	struct kevent ev[2];
	EV_SET(&ev[0], sock, EVFILT_READ, EV_ADD | EV_CLEAR, 0, 0, NULL);
	EV_SET(&ev[1], sock, EVFILT_WRITE, EV_ADD | EV_CLEAR, 0, 0, NULL);

	int opt = 1;
	setsockopt(sock, SOL_SOCKET, SO_NOSIGPIPE, &opt, sizeof(opt));

	if (kevent(kq, ev, 2, NULL, 0, NULL) == -1)
	    err(1, "kevent");

	setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));

	memset(&addr, 0, sizeof(addr));
	addr.sin_port = htons(10000);

	bind(sock, (struct sockaddr *)&addr, sizeof(addr));
	listen(sock, 0x80);

	ioctl(sock, FIONBIO, &opt);

	if (kevent(kq, ev, 2, NULL, 0, NULL) == -1)
		err(1, "kevent");

	wait_loop(kq, sock);
}