curl_multi_fdset() returns a write fd instead of read fd

[thanhphamhuu]

I did this

my system makes a request to the center to verify the PIN code via HTTP.
we use
+mc = curl_multi_fdset(multi, &fdread, &fdwrite, &fdexcep, &maxfd);
+ret = select(maxfd + 1, &fdread, &fdwrite, &fdexcep, &timeout);
+CURLMcode mc = curl_multi_perform(multi, &still_running);
in version V7.81.0 our system was fine but in version V8.4.0 the system had a problem.
the problem we encountered was that curl_multi_fdset returned fdwrite instead of fdread
I tested on version from V7.81.0 - V7.86.0 it still returned fdread but from version V7.87.0 our system had an error.

I expected the following

mc = curl_multi_fdset(multi, &fdread, &fdwrite, &fdexcep, &maxfd);
returns fdread on version v8.4.0

curl/libcurl version

curl 8.4.0

operating system

Linux

[CriticalThinking028]

您好，我已经收悉您的邮件，随后将回复您！ 祝您身体健康，工作顺利！

[pkropachev]

@thanhphamhuu , could you provide more details and attach a minimal sample that shows the problem?

I've tried on version 8.9.1-DEV, but cannot reproduce it.

#include <cstdio>
#include <curl/curl.h>

void print_fd(CURLM *multi_handle) {
  int max_fd, i, num_read = 0, num_write = 0, num_exc = 0;
  fd_set read_fd_set, write_fd_set, exc_fd_set;

  FD_ZERO(&read_fd_set);
  FD_ZERO(&write_fd_set);
  FD_ZERO(&exc_fd_set);

  curl_multi_fdset(multi_handle, &read_fd_set, &write_fd_set, &exc_fd_set,
                   &max_fd);

  for (i = 0; i <= max_fd; i++) {
    if (FD_ISSET(i, &read_fd_set))
      num_read++;
    if (FD_ISSET(i, &write_fd_set))
      num_write++;
    if (FD_ISSET(i, &exc_fd_set))
      num_exc++;
  }

  fprintf(stdout, "FDs: read: %d, write: %d, exc: %d\n", num_read, num_write,
          num_exc);
}

int main(int argc, char *argv[]) {
  FILE *output = fopen("/dev/null", "w");
  CURLMsg *msg = NULL;

  curl_global_init(CURL_GLOBAL_DEFAULT);

  CURLM *multi_handle = curl_multi_init();

  curl_multi_setopt(multi_handle, CURLMOPT_PIPELINING, CURLPIPE_NOTHING);

  curl_version_info_data *data = curl_version_info(CURLVERSION_NOW);
  fprintf(stdout, "Version libcurl: %s\n", data->version);

  for (int i = 0; i < 10; i++) {
    CURL *easy_handle = curl_easy_init();
    curl_easy_setopt(easy_handle, CURLOPT_URL, "https://example.com");
    curl_easy_setopt(easy_handle, CURLOPT_WRITEDATA, output);
    curl_multi_add_handle(multi_handle, easy_handle);
  }

  int running_handles = 0;
  do {
    curl_multi_perform(multi_handle, &running_handles);

    int msgs_left = 0;
    while ((msg = curl_multi_info_read(multi_handle, &msgs_left))) {
      if (msg && (msg->msg == CURLMSG_DONE)) {
        long res_status;
        curl_easy_getinfo(msg->easy_handle, CURLINFO_RESPONSE_CODE,
                          &res_status);
        fprintf(stdout, "Request complete (HTTP code: %ld), remaining: %d\n",
                res_status, running_handles);

        print_fd(multi_handle);

        curl_multi_remove_handle(multi_handle, msg->easy_handle);
        curl_easy_cleanup(msg->easy_handle);
      }
    }
  } while (running_handles);

  curl_multi_cleanup(multi_handle);
  curl_global_cleanup();

  return 0;
}

Version libcurl: 8.9.1-DEV
Request complete (HTTP code: 200), remaining: 9
FDs: read: 9, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 8
FDs: read: 8, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 7
FDs: read: 7, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 6
FDs: read: 6, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 5
FDs: read: 5, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 4
FDs: read: 4, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 3
FDs: read: 3, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 2
FDs: read: 2, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 1
FDs: read: 1, write: 0, exc: 0
Request complete (HTTP code: 200), remaining: 0
FDs: read: 0, write: 0, exc: 0

[thanhphamhuu]

@bagder, sorry, i am late reply
I have sample as follows:

#include <stdio.h>
#include <string.h> 
/* somewhat unix-specific */
#include <sys/time.h>
#include <unistd.h> 
/* curl stuff */
#include <curl/curl.h>  
#define HANDLECOUNT 3 /* Number of simultaneous transfers */
#define HTTP_HANDLE 0 /* Index for the HTTP transfer */
#define FTP_HANDLE 1  /* Index for the FTP transfer */ 
int main(void)
{
    CURL *handles[HANDLECOUNT];
    CURLM *multi_handle; 
    int still_running = 0; /* keep number of running handles */
    int i; 
    CURLMsg *msg;  /* for picking up messages with the transfer status */
    int msgs_left; /* how many messages are left */
    curl_global_init(CURL_GLOBAL_DEFAULT);
    /* Allocate one CURL handle per transfer */
    for (i = 0; i < HANDLECOUNT; i++){
        handles[i] = curl_easy_init();
    }
    /* set the options (I left out a few, you will get the point anyway) */
    curl_easy_setopt(handles[HTTP_HANDLE], CURLOPT_URL, "https://example.com");
    curl_easy_setopt(handles[FTP_HANDLE], CURLOPT_URL, "ftp://example.com");
    curl_easy_setopt(handles[FTP_HANDLE], CURLOPT_UPLOAD, 1L);
    /* init a multi stack */
    multi_handle = curl_multi_init();
    if (multi_handle != NULL)
    {
        printf("multi_handle ok\n");
    }
    /* add the individual transfers */
    for (i = 0; i < HANDLECOUNT; i++){
        curl_multi_add_handle(multi_handle, handles[i]);
    }
    /* we start some action by calling perform right away */
    curl_multi_perform(multi_handle, &still_running); 
    while (true)
    {
        //struct timeval timeout;
        int ret;       /* select() return code */
        CURLMcode mc; /* curl_multi_fdset() return code */
 
        fd_set fdread;
        fd_set fdwrite;
        fd_set fdexcep;
        int maxfd = -1; 
        FD_ZERO(&fdread);
        FD_ZERO(&fdwrite);
        FD_ZERO(&fdexcep); 
        /* get file descriptors from the transfers */
        for (i = 0; i < HANDLECOUNT; i++)
        {
            mc = curl_multi_fdset(multi_handle, &fdread, &fdwrite, &fdexcep, &maxfd);
            if (mc != CURLM_OK)
            {
                fprintf(stderr, "curl_multi_fdset() failed, code %d.\n", mc);
                break;
            }
        } 
        ret = select(maxfd + 1, &fdread, &fdwrite, &fdexcep, NULL);
        if (-1 == ret)
        {
            printf("select() failed. errno:%d", errno);
            continue;
        }
 
        for (int index = 0; index < HANDLECOUNT; index++)
        {
            if (FD_ISSET(maxfd, &fdread)) // bug -> remove this condition, I can see that the function can get the information after 2 or 3 loop
            {
                const char *szUrl = NULL;
                size_t headerSize = 0;
                char *location = NULL;
 
                int still_running = 0;
                CURLMcode mc = curl_multi_perform(m_pHttpContext->curlContext.hMulti, &still_running);
                printf("curl_multi_perform. CURLMcode:%d still_running:%d", mc, still_running);
                if(CURLM_OK != mc)
                {
                    return;
                }
                if(1 == still_running)
                {
                    return;
                }
 
                // Get HTTP status code
                //curl_easy_getinfo(handles[HTTP_HANDLE], CURLINFO_RESPONSE_CODE, &m);
                curl_easy_getinfo(handles[FTP_HANDLE], CURLINFO_PRIVATE, &szUrl);
                curl_easy_getinfo(handles[FTP_HANDLE], CURLINFO_HEADER_SIZE, &headerSize);
                ....
                curl_easy_getinfo(.....);
                printf(...)
            }
        }
    } 
    curl_multi_cleanup(multi_handle);
    for (i = 0; i < HANDLECOUNT; i++)
    {
        curl_easy_cleanup(handles[i]);
    }
    curl_global_cleanup();
    return 0;
}

Issue:

• In version v7.81.0 with sample on our system still runs stably. We tested and found that the first time curl_multi_perform() -> curl_multi_fdset() -> returned fdread
• when we upgraded to v8.4.0 we encountered an error.

• We tested and found that the first time curl_multi_perform() -> curl_multi_fdset() -> returned fdwrite. Therefore, "if (FD_ISSET(maxfd, &fdread))" this condition is not satisfied -> the next block of commands is not executed -> our system does not receive data.
• When we remove that condition and after 2->3 loops, the system receives data.

Q&A:

• Please tell me what is the difference when I use the two versions above V7.81.0 and V8.4.0?
• What do we need to do to fix the above situation when upgrading to the new version

?

[bagder]

• Your application calls curl_multi_fdset multiple times each loop instead of just once.
• I recommend using curl_multi_poll() instead for an easier API

We tested and found that the first time curl_multi_perform() -> curl_multi_fdset() -> returned fdwrite.

Why do you consider this is an error?

[thanhphamhuu]

@bagder , I don't consider it a bug but since it returns us fdwrite so not satisfying this condition causes my system to loop infinitely in while(true) loop on version 8.4.0 which is not the case on version 7.81.0

[icing]

Your assumption that a transfer only progresses on being in fdread is wrong. Protocols like HTTP/2, for example, require sending while receiving a response. The old HTTP/1.x way of only sending the request and then only receiving the response no longer holds.

[bagder]

not satisfying this condition causes my system to loop infinitely

That assumption is your error.

[thanhphamhuu]

@bagder

That assumption is your error.
Yes, but I still don't understand why V7.81.0 is fine, and when upgrading to V8.4.0, a bug occurs.

[thanhphamhuu]

@icing

Your assumption that a transfer only progresses on being in fdread is wrong. Protocols like HTTP/2, for example, require sending while receiving a response. The old HTTP/1.x way of only sending the request and then only receiving the response no longer holds.

can you explain more? i am handling it by calling curl_multi_perform multiple times. Is it ok?

[bagder]

Yes, but I still don't understand why V7.81.0 is fine, and when upgrading to V8.4.0, a bug occurs.

libcurl exports knowledge about what to wait for in this function - it does not say that it will provide those sockets and bits in a specific order or that it will ask for a readable socket first. Presumably, we refactored something between those two versions that changes how it behaves for you. Possibly your old version could still end up in that situation, just under different conditions. It could also be because you built the two different versions differently.

[icing]

Your assumption that a transfer only progresses on being in fdread is wrong. Protocols like HTTP/2, for example, require sending while receiving a response. The old HTTP/1.x way of only sending the request and then only receiving the response no longer holds.
can you explain more? i am handling it by calling curl_multi_perform multiple times. Is it ok?

Your code checks that maxfd, the one socket added last to the set, is readable. What about the, possibly, other sockets? They seem to be never checked. If your 3 transfers all use the same connection, this might work, unless the transfers need to send something before receiving.

In newer curl versions, the settings in fdread and fdwrite have been implemented more correctly. Older curl versions will have set bits in fdread where it really was not correct. So, what you see with 7.81 compared to 8.4 differs.

The correct way to use the curl API is to run curl_multi_perform() whenever one of the 1...maxfd sockets signals something, be it read or write.

[bagder]

The correct way to use the curl API is to run curl_multi_perform() whenever one of the 1...maxfd sockets signals something, be it read or write.

And only call it once, independently of how many sockets that are readable and writable. Then loop.

[bagder]

The multi-double example shows how this can be done a little simpler.

[thanhphamhuu]

The correct way to use the curl API is to run curl_multi_perform() whenever one of the 1...maxfd sockets signals something, be it read or write.

And only call it once, independently of how many sockets that are readable and writable. Then loop.

@bagder @icing , so what should we do to best fix this bug. can we use curl_multi_wait or curl_multi_poll ? to fix this error?

[bagder]

1. Stop assuming that you'll get a read activity to wait for first
2. Run the perform call independently of what socket activity there was (as documented)
3. Only run perform once when one (or more) sockets indicate activity

Maybe take another loop over the documentation and existing examples that show this.