libcpr / cpr

C++ Requests: Curl for People, a spiritual port of Python Requests.
https://docs.libcpr.org/
Other
6.49k stars 924 forks source link

Crash in cpr worker thread while using Interceptor (in a possibly unintended way) with async calls #871

Closed FranchescaMullin closed 1 year ago

FranchescaMullin commented 1 year ago

Description

We recently switched to using this library, as we really love the simplicity of the API compared with other curl libs. So far it's been great, but now I'm doing something a bit more complicated with it. I might have found a bug, or I might just be using the library in a way that is unintended.

I have to make a large number of API calls that use a bearer token. I am making these API calls as GetAsync and then storing the resulting futures in a vector. To avoid the problem of the bearer token expiring part way through, I am requesting the bearer token inside an Inteceptor. This logic checks if the token is still valid, and requests another one if it has expired (via cpr Post, using it's own cpr::Session object).

A cpr worker thread crashes when I add logic to get the token via an interceptor (exception message: double free or corruption (fasttop): 0x00007fffcc000970 ***). Inside the call stack I can see that the curl object has an error set which says "easy handle already used in multi handle".

I set breakpoints, and I see that one of the cpr worker threads will crash when this request is made, but I can see (from the url in the session object) that the crashed thread is not handling the call with the breakpoint in it, but one of the subsequent GET requests.

Like I said, this might just be an unintended use of the interceptor class, in which case I am wondering if there is a better approach that I can use to solve the original problem with the bearer expiring? Whatever the outcome of this, thanks for making a great library that has vastly simplified my team's code!

Here is the line of code that crashes: image

Here is the backtrace that is produced by the worker thread:

double free or corruption (fasttop): 0x00007fffcc000970 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x81329)[0x7ffff6078329] /home/adm-466360/local/lib64/libcurl.so(+0x5addf)[0x7ffff7e60ddf] /home/adm-466360/local/lib64/libcurl.so(+0x5c9b7)[0x7ffff7e629b7] /home/adm-466360/local/lib64/libcurl.so(curl_easy_setopt+0xb4)[0x7ffff7e6b0ff] /home/adm-466360/local/lib64/libcpr.so.1(_ZN3cpr7Session13prepareCommonEv+0x2e4)[0x7ffff7f6cae6] /home/adm-466360/local/lib64/libcpr.so.1(_ZN3cpr7Session10PrepareGetEv+0xfa)[0x7ffff7f702d6] /home/adm-466360/local/lib64/libcpr.so.1(_ZN3cpr7Session3GetEv+0x1c)[0x7ffff7f6f604] /home/adm-466360/local/lib64/libcpr.so.1(+0x85808)[0x7ffff7f6f808] /home/adm-466360/local/lib64/libcpr.so.1(+0x9e118)[0x7ffff7f88118] /home/adm-466360/local/lib64/libcpr.so.1(+0x9db9a)[0x7ffff7f87b9a] /home/adm-466360/local/lib64/libcpr.so.1(+0x9bef9)[0x7ffff7f85ef9] /home/adm-466360/local/lib64/libcpr.so.1(+0x9aedc)[0x7ffff7f84edc] /home/adm-466360/local/lib64/libcpr.so.1(+0x9921f)[0x7ffff7f8321f] /home/adm-466360/local/lib64/libcpr.so.1(+0x97d0c)[0x7ffff7f81d0c] /home/adm-466360/local/lib64/libcpr.so.1(+0x9689c)[0x7ffff7f8089c] /home/adm-466360/local/lib64/libcpr.so.1(+0x9dbec)[0x7ffff7f87bec] /home/adm-466360/local/lib64/libcpr.so.1(+0x9bf64)[0x7ffff7f85f64] /home/adm-466360/local/lib64/libcpr.so.1(+0x9af8c)[0x7ffff7f84f8c] /home/adm-466360/local/lib64/libcpr.so.1(+0x992a1)[0x7ffff7f832a1] /home/adm-466360/local/lib64/libcpr.so.1(_ZNKSt8functionIFSt10unique_ptrINSt13future_base12_Result_baseENS2_8_DeleterEEvEEclEv+0x3d)[0x7ffff7f8ab4b] /home/adm-466360/local/lib64/libcpr.so.1(_ZNSt13future_base13_State_baseV29_M_do_setEPSt8functionIFSt10unique_ptrINS_12_Result_baseENS3_8_DeleterEEvEEPb+0x27)[0x7ffff7f88d4d] /home/adm-466360/local/lib64/libcpr.so.1(_ZSt13invoke_implIvMNSt13__future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS0_12_Result_baseENS4_8_DeleterEEvEEPbEPS1_JS9_SA_EET_St21invoke_memfun_derefOT0_OT1DpOT2+0x98)[0x7ffff7f8ef4f] /home/adm-466360/local/lib64/libcpr.so.1(_ZSt8invokeIMNSt13future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS0_12_Result_baseENS4_8_DeleterEEvEEPbEJPS1_S9_SA_EENSt15invoke_resultIT_JDpT0_EE4typeEOSFDpOSG+0x67)[0x7ffff7f8cc47] /home/adm-466360/local/lib64/libcpr.so.1(_ZZSt9call_onceIMNSt13__future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS0_12_Result_baseENS4_8_DeleterEEvEEPbEJPS1_S9_SA_EEvRSt9once_flagOT_DpOT0_ENKUlvE_clEv+0x6a)[0x7ffff7f8a7a0] /home/adm-466360/local/lib64/libcpr.so.1(_ZZNSt9once_flag18_Prepare_executionC4IZSt9call_onceIMNSt13future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS3_12_Result_baseENS7_8_DeleterEEvEEPbEJPS4_SC_SD_EEvRS_OT_DpOT0_EUlvE_EERSI_ENKUlvE_clEv+0x27)[0x7ffff7f8cc7b] /home/adm-466360/local/lib64/libcpr.so.1(_ZZNSt9once_flag18_Prepare_executionC4IZSt9call_onceIMNSt13__future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS3_12_Result_baseENS7_8_DeleterEEvEEPbEJPS4_SC_SD_EEvRS_OT_DpOT0_EUlvE_EERSI_ENUlvE_4_FUNEv+0xe)[0x7ffff7f8cc8c] /lib64/libpthread.so.0(+0x620b)[0x7ffff7bc520b] /home/adm-466360/local/lib64/libcpr.so.1(+0x9e2b0)[0x7ffff7f882b0] /home/adm-466360/local/lib64/libcpr.so.1(_ZSt9call_onceIMNSt13future_base13_State_baseV2EFvPSt8functionIFSt10unique_ptrINS0_12_Result_baseENS4_8_DeleterEEvEEPbEJPS1_S9_SA_EEvRSt9once_flagOTDpOT0+0x66)[0x7ffff7f8a812] /home/adm-466360/local/lib64/libcpr.so.1(_ZNSt13future_base13_State_baseV213_M_set_resultESt8functionIFSt10unique_ptrINS_12_Result_baseENS3_8_DeleterEEvEEb+0x83)[0x7ffff7f888db] /home/adm-466360/local/lib64/libcpr.so.1(+0x96901)[0x7ffff7f80901] /home/adm-466360/local/lib64/libcpr.so.1(_ZNSt13packaged_taskIFN3cpr8ResponseEvEEclEv+0x33)[0x7ffff7f90f63] /home/adm-466360/local/lib64/libcpr.so.1(+0x871b0)[0x7ffff7f711b0] /home/adm-466360/local/lib64/libcpr.so.1(+0x8e0a2)[0x7ffff7f780a2] /home/adm-466360/local/lib64/libcpr.so.1(+0x8d188)[0x7ffff7f77188] /home/adm-466360/local/lib64/libcpr.so.1(+0x8c2cb)[0x7ffff7f762cb] /home/adm-466360/local/lib64/libcpr.so.1(_ZNKSt8functionIFvvEEclEv+0x32)[0x7ffff7f9754c] /home/adm-466360/local/lib64/libcpr.so.1(+0xac4ab)[0x7ffff7f964ab] /home/adm-466360/local/lib64/libcpr.so.1(+0xacc68)[0x7ffff7f96c68] /home/adm-466360/local/lib64/libcpr.so.1(+0xacc2b)[0x7ffff7f96c2b] /home/adm-466360/local/lib64/libcpr.so.1(+0xacbd8)[0x7ffff7f96bd8] /home/adm-466360/local/lib64/libcpr.so.1(+0xacbac)[0x7ffff7f96bac] /home/adm-466360/local/lib64/libcpr.so.1(+0xacb90)[0x7ffff7f96b90] /home/adm-466360/local/lib64/libcpr.so.1(+0xbdd84)[0x7ffff7fa7d84] /lib64/libpthread.so.0(+0x7ea5)[0x7ffff7bc6ea5] /lib64/libc.so.6(clone+0x6d)[0x7ffff60f5b0d]

Example/How to Reproduce

  1. First create an interceptor that makes a curl request (POST), and then modifies the current session
  2. Create a vector of futures to hold the response futures
  3. In a loop:
  4. Create a cpr::Session
  5. Set the interceptor of that session to the one previously created
  6. Call the GetAsync(), and add the future returned to the vector of futures
  7. One of the worker threads inside cpr will crash as soon as the interceptor code makes the POST request.

Possible Fix

No response

Where did you get it from?

GitHub (branch e.g. master)

Additional Context/Your Environment

COM8 commented 1 year ago

@FranchescaMullin Thanks for reporting!

Sorry for the delayed response, we are currently on vacation until March 01, 2023.

This looks like a bug in libcurl. Have you tried using more recent versions of libcurl? This is done by setting the CPR_USE_SYSTEM_CURL CMake variable to OFF (e.g. when configuring cmake .. -DCPR_USE_SYSTEM_CURL=OFF) Then cpr will use your system provided version of libcurl.

Could you please provide a minimal example project reproduces this issue so we can debug it faster?

For Further Reference

MALLOCCHECK This allows us to track down where the issue occurs when using glibc.

We can have a look at it once we are back, or do you have time to look into this in the meantime?

FranchescaMullin commented 1 year ago

Thanks for the response!

Hope you are having a great holiday!

FranchescaMullin commented 1 year ago

Hi,

I had a bit more of a dig into this issue, and I'm beginning to suspect that the interceptor is not the issue. The code below, with the code to set the interceptor commented out, still shows that curl handle error in the response object:

image

Code to replicate is just using the same session repeatedly, with async calls:

    // Setup the session
    std::shared_ptr<cpr::Session> sessionPtr = std::make_shared<cpr::Session>();
    sessionPtr->SetUrl("http://www.httpbin.org/get");

    // Add an interceptor to the session
    //sessionPtr->AddInterceptor(std::make_shared<MyInterceptor>());

    auto future = sessionPtr->GetAsync();
    auto future2 = sessionPtr->GetAsync();
    auto future3 = sessionPtr->GetAsync();

    cpr::Response response = future.get();
    std::cout << "main request, content-type:[" << response.header["content-type"] << "]" << std::endl;

    cpr::Response response2 = future2.get();
    std::cout << "second request, content-type:[" << response2.header["content-type"] << "]" << std::endl;

    cpr::Response response3 = future3.get();
    std::cout << "second request, content-type:[" << response3.header["content-type"] << "]" << std::endl;

Perhaps it's just not good to combine session reuse with GetAsync?

I did get my application working the way I needed in the end. I used std::async with a lambda that wraps the token request and the curl get request, which is also reusing cpr sessions (I made a session pool).

COM8 commented 1 year ago

Just to confirm the session reuse you are doing, with using one cpr::Session object and performing multiple GetAsync() in "parallel" on it, is not supported. It is not strictly a cpr limitation, but a curl limitation since one cpr::Session object uses one curl handle, and this curl handle does not support such a parallel interaction.

To solve this, replace:

auto future = sessionPtr->GetAsync();
auto future2 = sessionPtr->GetAsync();
auto future3 = sessionPtr->GetAsync();

with

std::vector<AsyncResponseC> resps{MultiGetAsync(std::tuple{url}, std::tuple{url}, std::tuple{url})};

Example

using AsyncResponseC = AsyncWrapper<Response, true>;

TEST(AsyncTests, HandleAlreadyUsedInMultiHandleTests) {
    // Setup the session
    std::shared_ptr<cpr::Session> sessionPtr = std::make_shared<cpr::Session>();
    Url url{server->GetBaseUrl() + "/hello.html"};
    sessionPtr->SetUrl(url);

    // Add an interceptor to the session
    // sessionPtr->AddInterceptor(std::make_shared<MyInterceptor>());

    std::vector<AsyncResponseC> resps{MultiGetAsync(std::tuple{url}, std::tuple{url}, std::tuple{url})};

    cpr::Response response = resps[0].get();
    std::cout << "main request, content-type:[" << response.header["content-type"] << "]" << std::endl;
    EXPECT_EQ(200, response.status_code);
    EXPECT_EQ(cpr::ErrorCode::OK, response.error.code);

    response = resps[1].get();
    std::cout << "second request, content-type:[" << response.header["content-type"] << "]" << std::endl;
    EXPECT_EQ(200, response.status_code);
    EXPECT_EQ(cpr::ErrorCode::OK, response.error.code);

    response = resps[2].get();
    std::cout << "second request, content-type:[" << response.header["content-type"] << "]" << std::endl;
    EXPECT_EQ(200, response.status_code);
    EXPECT_EQ(cpr::ErrorCode::OK, response.error.code);
}