Re: Question/proposal about function returning pollfds from multi handle

From: Timothe Litt <litt_at_acm.org>
Date: Mon, 18 Dec 2023 11:57:54 -0500

On 18-Dec-23 10:40, Daniel Stenberg via curl-library wrote:
> On Thu, 14 Dec 2023, Dmitry Karpov via curl-library wrote:
>
>> Probably, it would make sense to also return the count of
>> descriptors, like:
>>
>> CURLMcode curl_multi_pollfds(struct curl_waitfd *ufds, unsigned int
>> size, unsigned int* fd_count);
>>
>> The fd_count will tell the client code how many descriptors the
>> multi-handle actually has.
>
> How about this tweak:
>
>  CURLMcode
>  curl_multi_pollfds(struct Curl_multi *multi,
>                     struct curl_waitfd **ufds,
>                     unsigned int *fd_count);
>
> It returns a pointer to an array of descriptors and fd_count is the
> number of entries in that array. The array is allocated by libcurl and
> is stored associated with the multi handle so it is freed with the
> handle. Calling this function several times will then just overwrite
> the same array so it does not have to get reallocated, unless it needs
> to grow, during a program's lifetime.
>
> It seems less complicated than having the caller pass in an array of a
> size it can't know how big it needs to be. The downside is of course
> that the data in the array will not survive calling the function again.
>
Is there a thread safety issue with this?  E.g.  one thread calls,
buffer changes due to allocating more FDs in second thread, first thread
now has cached pointer to freed memory (use after free).

In any case, the traditional way for the user to get the buffer size
required is to return the needed size with a non-fatal error code.  You
could use the proposed signature but have the caller initialize *ufds &
count.  If *ufds is NULL or count is less than required, just return the
count (required size) in *fd_count & and error code
(CURL_BUFFER_TOO_SMALL).  Caller (re-)alloc's a buffer of at least that
size (sizeof struct *curl_waitfd)* *fd_count and retries.  This leaves
the caller responsible for both allocating and deallocating the
buffer.   It's not data used by libcurl, so making libcurl take
responsibility for it seems odd.

This also raises the question of how the caller knows that it's
necessary to get an updated list of active fds.  Presumably, libcurl can
allocate/free fds for its internal use.  There may not be a simple
mapping between a caller's actions on handles and the fds that need to
be polled.  You will get an EBADF from poll if an fd is closed, or
confused if it's been recycled.

Another approach would be for libcurl to register a callback triggered
when an fd is opened/closed, and provide the user with a curl_waitfd &
open/close flag.  (Model on CURLOPT_{OPEN,CLOSE}SOCKETFUNCTION) The user
has more bookkeeping to do, but libcurl doesn't have to worry about
freeing this user buffer.  And the user knows exactly what fds are active.


Timothe Litt
ACM Distinguished Engineer
--------------------------
This communication may not represent the ACM or my employer's views,
if any, on the matters discussed.

-- 
Unsubscribe: https://lists.haxx.se/mailman/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Received on 2023-12-18