Write function callback is called twice after resume transfer and return CURL_WRITEFUNC_ERROR #13337
Comments
- remember error encountered in invoking write callback and always fail afterwards without further invokes - check behaviour in test_02_17 with h2-pausing client - refs curl#13337
|
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? |
|
Thanks @icing! Now behavior looks correct in the sample and our particular application. But unfortunately we faced with another problem in In our traces it looks like the following for In Sorry for these logs. Just our application is quite complicated (it uses |
|
@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 |
|
@icing, yes, sorry for that logs. Anyway, initially filed problem is fixed for us. Thank you for that! We checked the fix for HTTP 1.1 and 2. Let me describe details of the problem that we found in the |
|
So, we use #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;
}
All looks correct except the third call of callback. But this is due to the fix #13340 is not applied yet. 🙂
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: We see that callback is called again immediately after return If we rollback #13271 and run tests again then we get correct behavior for both tests. Hope it will help. |
|
The log line |
|
Yes, I think so. For example, if I run test with HTTP/2 and fix from #13340 then I get the following output: Test with HTTP/1.1 and fix from #13340: So, the fix #13340 for initial problem works as expected. |
|
Ok, thanks. Just wanted to be certain what I am looking at. |
|
This output in case using HTTP/2 is unexpected for us: We put transfer on pause by returning |
|
The problem seems to be that 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. |
|
It's interesting. I see the same behavior with HTTP/2 in case using |
|
I believe it will always happen on a PAUSE when the full response had been received. If you make a larger download, I'd expect this not to happen. |
|
I updated #13340 to no longer "finish" transfers that have received everything but are paused by the client writing. I believe this should address your issue. Happy to get feedback. |
|
@icing, now the sample works correct for HTTP/1.1 and 2. There are not extra calls of the callback. Thank you! I faced with problem with our application and haven't identified the root cause of the problem yet. I use local Caddy server to download a binary file with size 10M. All looks correct in case use HTTP/2 (HTTPS). I collected "write" traces. After abort the transfer I see In case I use HTTP/1.1 (HTTP instead of HTTPS to force switch to 1.1) then I see that At the moment I don't have any ideas why it happens. So, I need a bit more time to understand why it's happening. Could you please share if there is some place in |
|
Interesting application you have there. I updated #13340 again to take care of the situation where Now, the actual unpaused writes happen during "normal" transfer processing and lead to a failed transfer as they should. Thus, |
- remember error encountered in invoking write callback and always fail afterwards without further invokes - check behaviour in test_02_17 with h2-pausing client - refs curl#13337
|
@pkropachev it would be nice to know how the updated PR works for you. Thanks. |
|
@icing, sorry for the delay with reply. We'll test it today and give feedback. Thanks! |
|
@icing, I can confirm now it works great for the sample and our application. |
|
Thanks for confirming and the help in providing sample code! |
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 registerCURLOPT_WRITEFUNCTIONcallback and pause the transfer by returningCURL_WRITEFUNC_PAUSEfrom this callback. After some delay we resume transfer by callingcurl_easy_pause(..., CURLPAUSE_CONT). Then after our callback is called we stop transfer by returningCURL_WRITEFUNC_ERROR. In this scenario we faced with the problem that our callback is called twice after it had returnedCURL_WRITEFUNC_ERROR.We reproduced the behavior using example of code from the comment, but slightly simplify it.
In the result, we're observing the same behavior like in our use case.
I expected the following
Callback should not be called after return
CURL_WRITEFUNC_ERRORfrom it. Behavior is correct in case we don't put the transfer on pause.Just comment the following block in the sample:
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
The text was updated successfully, but these errors were encountered: