Write function callback is called twice after resume transfer and return CURL_WRITEFUNC_ERROR

[pkropachev]

I did this

We have the scenario when we put the transfer on pause and resume it after a while and then stop transfer with CURL_WRITEFUNC_ERROR. For that we register CURLOPT_WRITEFUNCTION callback and pause the transfer by returning CURL_WRITEFUNC_PAUSE from this callback. After some delay we resume transfer by calling curl_easy_pause(..., CURLPAUSE_CONT). Then after our callback is called we stop transfer by returning CURL_WRITEFUNC_ERROR. In this scenario we faced with the problem that our callback is called twice after it had returned CURL_WRITEFUNC_ERROR.

We reproduced the behavior using example of code from the comment, but slightly simplify it.

#include <curl/curl.h>
#include <stdbool.h>
#include <stdio.h>
#include <unistd.h>

bool paused;
int call_counter;

static size_t http_body(void *buffer, size_t size, size_t nmemb, void *userp) {
  call_counter++;
  fprintf(stderr, "http_body called at %d time\n", call_counter);

  if (call_counter == 1) {
    paused = true;
    fprintf(stderr, "Return pause\n");
    return CURL_WRITEFUNC_PAUSE;
  }

  fprintf(stderr, "Return abort\n");
  return CURL_WRITEFUNC_ERROR;
}

int main(int argc, char **argv) {
  CURLM *curl_multi_handle;
  CURL *curl;

  const char *url = "http://httpbin.org";
  struct CURLMsg *msg;
  int running_handles;
  int msgs_left;
  call_counter = 0;
  paused = false;

  curl_global_init(CURL_GLOBAL_DEFAULT);
  curl_multi_handle = curl_multi_init();
  curl = curl_easy_init();

  curl_easy_setopt(curl, CURLOPT_URL, url);
  curl_easy_setopt(curl, CURLOPT_BUFFERSIZE, 1024);
  curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
  curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, http_body);
  curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);

  curl_multi_add_handle(curl_multi_handle, curl);

  curl_multi_perform(curl_multi_handle, &running_handles);

  while (running_handles > 0) {
    usleep(100 * 1000);

    /* Resume if transfer is paused */
    if (paused) {
      fprintf(stderr, "Resume transfer\n");
      curl_easy_pause(curl, CURLPAUSE_CONT);
      paused = false;
    }

    curl_multi_perform(curl_multi_handle, &running_handles);
  }

  msg = curl_multi_info_read(curl_multi_handle, &msgs_left);
  fprintf(stderr, "Connection finished: %s\n",
          curl_easy_strerror(msg->data.result));

  curl_multi_remove_handle(curl_multi_handle, curl);
  curl_easy_cleanup(curl);
  curl_multi_cleanup(curl_multi_handle);

  return 0;
}

In the result, we're observing the same behavior like in our use case.

#> ./pause-test  
* Host httpbin.org:80 was resolved.
* IPv6: (none)
* IPv4: 3.223.18.102, 3.208.234.136, 35.168.90.70, 18.210.102.60, 18.204.54.143, 34.232.61.91, 54.87.28.144, 52.203.147.106
*   Trying 3.223.18.102:80...
* Connected to httpbin.org (3.223.18.102) port 80
> GET / HTTP/1.1
Host: httpbin.org
Accept: */*

* Request completely sent off
< HTTP/1.1 200 OK
< via: proxy A
< Date: Wed, 10 Apr 2024 10:34:44 GMT
< Server: gunicorn/19.9.0
< Connection: Keep-Alive
< Content-Type: text/html; charset=utf-8
< Content-Length: 9593
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Credentials: true
< 
http_body called at 1 time
Return pause
Resume transfer
http_body called at 2 time
Return abort
* Failure writing output to destination, passed 771 returned 4294967295
http_body called at 3 time                                                       <--- ?
Return abort
* Failure writing output to destination, passed 1795 returned 4294967295
http_body called at 4 time                                                       <--- ?
Return abort
* Failure writing output to destination, passed 1795 returned 4294967295
* Closing connection
Connection finished: Failed writing received data to disk/application

I expected the following

Callback should not be called after return CURL_WRITEFUNC_ERROR from it. Behavior is correct in case we don't put the transfer on pause.

Just comment the following block in the sample:

if (call_counter == 1) {
    paused = true;
    fprintf(stderr, "Return pause\n");
    return CURL_WRITEFUNC_PAUSE;
}

curl/libcurl version

curl 8.7.1
curl 8.7.0-DEV (x86_64-pc-linux-gnu) libcurl/8.7.0-DEV BoringSSL zlib/1.2.11 c-ares/1.27.0 libpsl/0.21.0 nghttp2/1.60.0 quiche/0.20.1
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS HSTS HTTP2 HTTP3 HTTPS-proxy IPv6 Largefile libz NTLM PSL SSL threadsafe UnixSockets

operating system

Linux kwx1252784oftdls 5.13.0-52-generic #59~20.04.1-Ubuntu SMP Thu Jun 16 21:21:28 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

[icing]

Thanks for your excellent description. I could reproduce the problem and adapt one of our test cases to check this. Would you be able to test #13340 if this works for you as well?

[pkropachev]

Thanks @icing! Now behavior looks correct in the sample and our particular application.

But unfortunately we faced with another problem in master branch (currently we use curl_8-7-1 tag). By some reason callback is called twice after put transfer on pause and resume it.

In our traces it looks like the following for curl_8-7-1:

2024-04-10 18:28:04.059,150 [DEBUG] (transfer_handler.cpp:337) write_via_user_callback_data_callback called for request e6235717-422a-4d87-91f5-c34586cff8fe with buffer size 163
2024-04-10 18:28:04.064,465 [DEBUG] (transfer_handler.cpp:374) Pause receiving for request e6235717-422a-4d87-91f5-c34586cff8fe
2024-04-10 18:28:04.084,478 [DEBUG] (transfer_handler.cpp:355) Going to resume receiving for request e6235717-422a-4d87-91f5-c34586cff8fe
2024-04-10 18:28:04.084,647 [DEBUG] (transfer_handler.cpp:610) Resume receiving request e6235717-422a-4d87-91f5-c34586cff8fe
2024-04-10 18:28:04.084,733 [DEBUG] (transfer_handler.cpp:337) write_via_user_callback_data_callback called for request e6235717-422a-4d87-91f5-c34586cff8fe with buffer size 163
2024-04-10 18:28:04.084,831 [WARN ] (transfer_handler.cpp:368) Abort request e6235717-422a-4d87-91f5-c34586cff8fe due to Want to abort

In master branch:

2024-04-10 18:34:28.407,189 [DEBUG] (transfer_handler.cpp:337) write_via_user_callback_data_callback called for request aa74de12-667f-4fba-b7f9-f1a387bfebb7 with buffer size 163
2024-04-10 18:34:28.412,484 [DEBUG] (transfer_handler.cpp:374) Pause receiving for request aa74de12-667f-4fba-b7f9-f1a387bfebb7
2024-04-10 18:34:28.412,589 [DEBUG] (transfer_handler.cpp:337) write_via_user_callback_data_callback called for request aa74de12-667f-4fba-b7f9-f1a387bfebb7 with buffer size 163
2024-04-10 18:34:28.417,917 [DEBUG] (transfer_handler.cpp:374) Pause receiving for request aa74de12-667f-4fba-b7f9-f1a387bfebb7
2024-04-10 18:34:28.433,459 [DEBUG] (transfer_handler.cpp:355) Going to resume receiving for request aa74de12-667f-4fba-b7f9-f1a387bfebb7
2024-04-10 18:34:28.433,660 [DEBUG] (transfer_handler.cpp:610) Resume receiving request aa74de12-667f-4fba-b7f9-f1a387bfebb7
2024-04-10 18:34:28.433,741 [DEBUG] (transfer_handler.cpp:355) Going to resume receiving for request aa74de12-667f-4fba-b7f9-f1a387bfebb7
2024-04-10 18:34:28.433,806 [DEBUG] (transfer_handler.cpp:337) write_via_user_callback_data_callback called for request aa74de12-667f-4fba-b7f9-f1a387bfebb7 with buffer size 163
2024-04-10 18:34:28.433,888 [WARN ] (transfer_handler.cpp:368) Abort request aa74de12-667f-4fba-b7f9-f1a387bfebb7 due to Want to abort

Sorry for these logs. Just our application is quite complicated (it uses epoll and curl_multi_socket_action), at the moment I don't have an idea how to reproduce this problem with short sample.
By the way, after rolled back #13271 (just transfer.c) all looks correct in our application. I don't know how changes in #13271 impacted on this behavior.

[icing]

@pkropachev, it is difficult to understand what the log means, not knowing your application. Can you get a stack trace of the second "pausing" call, where it comes from?

As an aside: if you need identifiers for requests in your application (unique for all in the same multi handle), there is CURLINFO_XFER_ID since curl 8.2.0. But then, you might need the uuids also for other purposes, in which case disregard this advice.

[pkropachev]

@icing, yes, sorry for that logs.
We were able to reproduce the problem that I described in my previous comment with short sample of code. This problem is specific for HTTP version 2 (all is ok for HTTP 1.1). Problem is reproduced with libcurl built from the current master branch. All is ok in case rollback #13271.

Anyway, initially filed problem is fixed for us. Thank you for that! We checked the fix for HTTP 1.1 and 2.
So, I'm not sure if we should continue investigation in scope of this issue or need to raise new one.

Let me describe details of the problem that we found in the master branch in next comment and attach a sample for reproducing.

[pkropachev]

So, we use libcurl built from the current master branch.

#include <curl/curl.h>
#include <stdbool.h>
#include <stdio.h>
#include <unistd.h>

bool paused;
int call_counter;

static size_t http_body(void *buffer, size_t size, size_t nmemb, void *userp) {
  call_counter++;
  fprintf(stderr, "http_body called at %d time. Callback call is %s expected\n",
          call_counter, !paused ? "" : "NOT");
  
  if (call_counter == 1) {
    paused = true;
    fprintf(stderr, "Return pause\n");
    return CURL_WRITEFUNC_PAUSE;
  }
  
  fprintf(stderr, "Return abort\n");
  return CURL_WRITEFUNC_ERROR;
}

int main(int argc, char **argv) {
  CURLM *curl_multi_handle;
  CURL *curl;

  const char *url = "https://httpbin.org/stream/1";
  struct CURLMsg *msg;
  int running_handles;
  int msgs_left;
  call_counter = 0;
  paused = false;

  curl_global_init(CURL_GLOBAL_DEFAULT);
  curl_multi_handle = curl_multi_init();
  curl = curl_easy_init();

  curl_easy_setopt(curl, CURLOPT_URL, url);
  curl_easy_setopt(curl, CURLOPT_BUFFERSIZE, 1024);
  curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
  // curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);
  curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, http_body);
  curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);

  curl_multi_add_handle(curl_multi_handle, curl);

  curl_multi_perform(curl_multi_handle, &running_handles);

  while (running_handles > 0) {
    usleep(100 * 1000);

    /* Resume if transfer is paused */
    if (paused) {
      fprintf(stderr, "Going to resume transfer\n");
      paused = false;
      curl_easy_pause(curl, CURLPAUSE_CONT);
      fprintf(stderr, "Resumed\n");
    }

    curl_multi_perform(curl_multi_handle, &running_handles);
  }

  msg = curl_multi_info_read(curl_multi_handle, &msgs_left);
  fprintf(stderr, "Connection finished: %s\n",
          curl_easy_strerror(msg->data.result));

  curl_multi_remove_handle(curl_multi_handle, curl);
  curl_easy_cleanup(curl);
  curl_multi_cleanup(curl_multi_handle);

  return 0;
}

1. First test with HTTP/1.1. In the result we get the following output:

http_body called at 1 time. Callback call is  expected   <--- OK for pause
Return pause
Going to resume transfer                                         
http_body called at 2 time. Callback call is  expected   <--- OK for resume
Return abort
* Failure writing output to destination, passed 205 returned 4294967295
Resumed
http_body called at 3 time. Callback call is  expected
Return abort
* Failure writing output to destination, passed 205 returned 4294967295
* Connection #0 to host httpbin.org left intact
Connection finished: Failed writing received data to disk/application

All looks correct except the third call of callback. But this is due to the fix #13340 is not applied yet. 🙂

2. Second test with HTTP/2.

You need to comment:

curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);

and uncomment:

curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);

In the result we get the following output:

http_body called at 1 time. Callback call is  expected            <--- OK for pause
Return pause
http_body called at 2 time. Callback call is NOT expected     <--- Why is this called again? We don't see "Going to resume transfer" before it
Return abort
* Failure writing output to destination, passed 205 returned 4294967295
http_body called at 3 time. Callback call is NOT expected
Return abort
* Failure writing output to destination, passed 205 returned 4294967295
* Connection #0 to host httpbin.org left intact
Connection finished: Failed writing received data to disk/application

We see that callback is called again immediately after return CURL_WRITEFUNC_PAUSE from it.

If we rollback #13271 and run tests again then we get correct behavior for both tests. Hope it will help.
Let me know if any additional information is required.

[icing]

The log line * Failure writing output to destination, passed 205 returned 4294967295 indicates that this was running without the fix in #13340 ???

[pkropachev]

Yes, I think so. For example, if I run test with HTTP/2 and fix from #13340 then I get the following output:

http_body called at 1 time. Callback call is  expected
Return pause
http_body called at 2 time. Callback call is NOT expected     <--- This is second problem for HTTP/2 in master branch
Return abort
* client returned ERROR on write of 205 bytes
* Connection #0 to host httpbin.org left intact
Connection finished: Failed writing received data to disk/application

Test with HTTP/1.1 and fix from #13340:

http_body called at 1 time. Callback call is  expected
Return pause
Going to resume transfer
http_body called at 2 time. Callback call is  expected
Return abort
* client returned ERROR on write of 205 bytes
Resumed
* Connection #0 to host httpbin.org left intact
Connection finished: Failed writing received data to disk/application

So, the fix #13340 for initial problem works as expected.

[icing]

Ok, thanks. Just wanted to be certain what I am looking at.

[pkropachev]

This output in case using HTTP/2 is unexpected for us:

http_body called at 1 time. Callback call is  expected
Return pause
http_body called at 2 time. Callback call is NOT expected  <--- ?

We put transfer on pause by returning CURL_WRITEFUNC_PAUSE from callback. After that the callback is called again immediately after return CURL_WRITEFUNC_PAUSE from it.

[icing]

The problem seems to be that https://httpbin.org/stream/1 delivers a 502 error with a short response content of 205 bytes. Those bytes get paused by your callback, then the transfer is done and finished and tries to get rid of the buffered data one last time.

Now, the discussion is: what should curl really do here for a PAUSED transfer that is done. It seems that earlier versions just discarded the data and did not try to flush it. Not sure myself what the correct behaviour is here.

[pkropachev]

It's interesting. I see the same behavior with HTTP/2 in case using https://curl.se.