On Tue, 16 Nov 2004, David Phillips wrote:

> This patch addresses bug #948950: excessive amount of file descriptors
> cause crash in libcurl.

Let me first say that I really appreciate you taking on this bug!

> The bug is mainly fixed by creating a new internal function, Curl_select().
> It lives in select.c and select.h. The function is a replacement for simple
> calls to select with a single read and/or write file descriptor event.
> Errors and timeouts are reported, along with read, write or error events.

A very nice approach. Simple yet elegant.

> Things which aren't fixed:
>
> - telnet.c: This requires a read event on two descriptors. This is
> relatively easy to fix with another wrapper function that uses a poll()
> style interface.

Or possibly the only function can take two read descriptors.

> - hostares.c: This requires multiple events. More importantly, it requires
> changes to the ares library.

I agree. But fixing ares (or rather c-ares) is not a problem since I am the
maintainer of that too, and I can't see how we can fix this properly without
doing so.

I believe we need to provide a different function than ares_fds() that returns
a number of file descriptors for reading/writing.

> - curl_multi_fdset() and related issues: In order to make the code
> safe and usable with more than FD_SETSIZE file descriptors, I removed
> the FD_SET() calls in the normal-case code. Did this break break
> curl_multi_fdset()?

I didn't pay attention to the details here, but based on your description here
it sounds like you modified how it works and thus it is bound to break some
app out there.

> A new API function should be introduced that uses a safer method of
> returning file descriptors. For the current method, checks should be put in
> place anytime FD_SET() is called.

Yes. We should add a function that better return the file descriptors in a
more generic manner so that the caller can use them for whatever function it
wants to use. poll() is just one way.

> Please give me feedback on this patch. It currently serves our needs, but I
> would like to get this fix into the main distribution and hopefully into the
> next release.

Me too!

-- 
      Daniel Stenberg -- http://curl.haxx.se -- http://daniel.haxx.se
       Dedicated custom curl help for hire: http://haxx.se/curl.html