On Wed, 11 Jun 2008, Christopher Palow wrote:

(Mail written top-down, I did realize some further facts later on in the mail
but I left the upper parts for the "greater picture")

> First off while all of my samplings showed the file descriptor for the http
> connection ends up being the same as the DNS connection this isn?t always
> going to be the case (especially if you have lots of other connections
> closing around the same time).

Right, but I don't see how the code would have a problem with the file
descriptor being different. The only reason it doesn't inform about the change
in your cases is exactly because you got the same file descriptor. If you had
gotten a different one instead, libcurl would've told the app about the
removal and the addition.

> > If the DNS resolve is compeleted, the socket is closed and another socket
> > is created (which then is the same file descriptor), why would libcurl
> > need to tell the app about this??
>
> In general I think libcurl should tell the app to stop being interested on a
> socket before calling close() on it.

Well sure, but that would then force us to A) make sure c-ares uses its socket
callbacks that way and B) make sure libcurl uses c-ares' socket callbacks.
Since in the case of the DNS resolve, the socket is owned and controlled by
c-ares and libcurl only polls the socket "action info" from c-ares after the
c-ares functions have returned.

> We can?t assume all sockets the app is using are going to be used just for
> libcurl.

I don't see how we make this assumption anywhere.

> Since Linux gives you the lowest unused fd, oftentimes one you just used,
> for a new fd I suspect you can get some interesting races where the app
> calls epoll_wait/select etc on a fd it still thinks is a libcurl one.

But we're talking about a single thread and linear execution here. Where would
or could anyone else in your process "nick" a file descriptor from between
c-ares closes the socket to the adjacent socket() call libcurl does?

> The patch I proposed actually informs the app it?s not interested in the fd
> anymore after close(). I?m not crazy about but that?s also what currently
> happens on timeouts (at least some of the time).

Yes, as soon as libcurl figures out there's a socket previously being watched
that are no longer "interesting", it tells the app about it. But there's no
need to tell it about multiple changes within the same function call. One
change - the final update - is enough, and then the function returns.

> Don?t think about APIs like select() or poll() where the interested set is
> maintained in userspace

curl_multi_socket() even makes it awkward to use select() or poll() ...

> think instead of epoll (Linux) or kevent (FreeBSD) where the interested set
> is maintained in the kernel. The ones where the state is maintained in the
> kernel will remove the socket from the interested set when close() is
> called.

Aha! This is the key I was missing all the time. I hadn't understood that
close() will have that affect...

Ok, so then we have to do that extra REMOVE update only to add it again just
microseconds later to make sure the app re-adds the "supervision" of that
socket! But of course, this still makes the notification get done _after_ the
close() but is that really gonna matter?

> I can look at redoing my patch so that semantic is obeyed if you agree.

No, I think I now understand exactly why the patch you posted is needed like
that. I'd only like an explanatory comment around it that explains why it is
needed there.

Thanks for your detailed descriptions and patience!

-- 
  / daniel.haxx.se