Re: (boost::)asio and kqueue problem

heh, nice catch. Would you please file a PR so we don't forget?

Thanks!


-a


On 19 July 2016 at 08:35, Hartmut Brandt <hartmut.brandt_at_dlr.de> wrote:
> 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)
> _at__at_ -1350,8 +1350,8 _at__at_
>         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)
> _at__at_ -160,6 +160,7 _at__at_
>  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;
>
> _at__at_ -172,6 +173,7 _at__at_
>         .f_isfd = 1,
>         .f_detach = filt_sordetach,
>         .f_event = filt_soread,
> +       .f_touch = filt_sotouch,
>  };
>  static struct filterops sowrite_filtops = {
>         .f_isfd = 1,
> _at__at_ -3091,6 +3093,31 _at__at_
>         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.
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"