Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOH] Since v8.5.0, a change in API behavior causes curl_multi_poll() or curl_multi_perform() to return error code #14414

Closed
luozhaohui opened this issue Aug 6, 2024 · 11 comments
Labels
name lookup DNS and related tech

Comments

@luozhaohui
Copy link

luozhaohui commented Aug 6, 2024

I did this

Note
I'm not entirely sure if this is a bug or an improvement. However, since curl_multi_poll() returns error code 12: Unrecoverable error in select/poll work thread exiting, I can't determine if cURL's internal state is valid and if it should continue working. Therefore, I'm reporting this as an issue for now.

If this isn't a bug, could you please advise on how to proceed when curl_multi_poll() returns error code 12? Thank you very much.

Test Program

#include <cstdio>
#include <exception>
#include <iostream>
#include <iterator>
#include <memory>
#include <ostream>
#include <sstream>
#include <mutex>
#include <future>
#include <functional>
#include <thread>
#include <atomic>
#include <chrono>
#include <string>
#include <utility>
#include <list>
#include <vector>
#include <string.h>
#include <assert.h>
#include <curl/curl.h>

static bool globalInitCalled = false;

void doInit()
{
    static bool initialized = ([] {
        curl_global_init(CURL_GLOBAL_ALL);
        globalInitCalled = true;
        return true;
    })();

    // silence unused warning
    (void)initialized;
}

enum Method
{
    HEAD,
    GET,
    POST,
    PUT,
};

class Session
{
public:
    enum State
    {
        NEW,
        PERFORMING,
        ABORTED,
        HTTP_OK,
        HTTP_ERR,
        OTHER_ERR
    };

    Session(Method method, const std::string& url, const std::string& dnsServer, std::shared_ptr<std::atomic_bool> flag)
        : state_(NEW)
        , statusStr_("new")
        , origUrl_(url)
    {
        doInit();
        curl_ = curl_easy_init();

        // Set URL
        setopt(CURLOPT_URL, url.c_str());

        // Set method
        switch (method)
        {
            case HEAD:
                setopt(CURLOPT_NOBODY, 1L);
                break;
            case GET:
                setopt(CURLOPT_HTTPGET, 1L);
                break;
            case POST:
                setopt(CURLOPT_POST, 1L);
                break;
            case PUT:
                setopt(CURLOPT_CUSTOMREQUEST, "PUT");
                setopt(CURLOPT_UPLOAD, 1L);
                break;
        }

        // encoding
        setopt(CURLOPT_ACCEPT_ENCODING, "");
        setopt(CURLOPT_SSL_VERIFYPEER, 0L);

        if (flag)
        {
            setAbortFlag(flag);
        }

        if (!dnsServer.empty())
        {
            enableDoH(dnsServer);
        }
    }

    ~Session()
    {
        if (curl_ != nullptr)
        {
            curl_easy_cleanup(curl_);
            curl_ = nullptr;
        }
    }

    void setAbortFlag(std::shared_ptr<std::atomic_bool> f)
    {
        assert(state_ == NEW);
        isAborted_ = [f]()
        {
            bool ret = f->load();
            if (ret)
            {
                std::cout << "> abort flag set." << std::endl;
            }

            return f->load();
        };
    }

    void enableDoH(const std::string& v)
    {
        assert(state_ == NEW);

        std::cout << "enableDoH: " << v << std::endl;
        setopt(CURLOPT_DOH_URL, v.c_str());
        setopt(CURLOPT_DOH_SSL_VERIFYPEER, 0);
        setopt(CURLOPT_DOH_SSL_VERIFYHOST, 0);
    }

    bool checkAborted()
    {
        return isAborted_();
    }

    void start()
    {
        assert(state_ == NEW);
        setopt(CURLOPT_PRIVATE, this);
        state_ = PERFORMING;
        statusStr_ = "performing";
    }

    void abort()
    {
        assert(state_ == PERFORMING);
        state_ = ABORTED;
        statusStr_ = "aborted";
    }

    State getState() const
    {
        return state_;
    }

    bool inProgress() const
    {
        return state_ == PERFORMING;
    }

    bool failed() const
    {
        return state_ != NEW &&
               state_ != PERFORMING &&
               state_ != HTTP_OK;
    }

    const std::string& getStatusStr() const
    {
        return statusStr_;
    }

    const std::string& url() const
    {
        return origUrl_;
    }

    CURL* handle() const
    {
        return curl_;
    }

    void finish(CURLcode res)
    {
        if (res == CURLE_OK)
        {
            state_ = HTTP_OK;
            statusStr_ = "OK";
        }
        else
        {
            state_ = OTHER_ERR;
            statusStr_ = "error";
        }
    }

private:
    template <typename T>
    void setopt(CURLoption opt, T&& v) const
    {
        curl_easy_setopt(curl_, opt, v);
    }

private:
    State state_;
    std::string statusStr_;
    std::string origUrl_;
    CURL* curl_ = nullptr;

    std::function<bool()> isAborted_ = []() {
        return false;
    };
};

class RequestError: public std::exception
{
public:
    RequestError(std::unique_ptr<Session> session)
        : session_(std::make_shared<std::unique_ptr<Session>>(std::move(session)))
    {
    }
    const char* what() const noexcept override
    {
        return response().getStatusStr().c_str();
    }

    const Session& response() const
    {
        return **session_;
    }

private:
    std::shared_ptr<std::unique_ptr<Session>> session_;
};

inline std::ostream& operator<<(std::ostream& out, const RequestError& err)
{
    return out << err.what();
}

using Continuation = std::function<void(std::future<std::unique_ptr<Session>>)>;

class Pool {
public:
    Pool()
    {
        doInit();
        curlm_ = curl_multi_init();
    }

    ~Pool()
    {
        std::cout << "Pool destruction." << std::endl;
        if (curlm_ != nullptr)
        {
            curl_multi_cleanup(curlm_);
            curlm_ = nullptr;
        }
    }

    std::unique_ptr<Session> create(Method method, const std::string& url, const std::string& dnsServer, std::shared_ptr<std::atomic_bool> flag)
    {
        return std::make_unique<Session>(method, url, dnsServer, flag);
    }

    void add(Session& s)
    {
        s.start();
        check(curl_multi_add_handle(curlm_, s.handle()), "curl_multi_add_handle");
    }

    void remove(Session& s)
    {
        check(curl_multi_remove_handle(curlm_, s.handle()), "curl_multi_remove_handle");
    }

    void abortIfInProgress(Session& s)
    {
        if (s.getState() == Session::PERFORMING)
        {
            std::cout << ">> abort session" << std::endl;
            remove(s);
            s.abort();
        }
    }

    bool wait(int timeoutMs = 20)
    {
        auto ec = curl_multi_poll(curlm_, NULL, 0, timeoutMs, NULL);
        check(ec, "curl_multi_poll");
        return ec == CURLM_OK;
    }

    void wakeup()
    {
        check(curl_multi_wakeup(curlm_), "curl_multi_wakeup");
    }

    void check(CURLMcode ec, const char* msg)
    {
        if (ec != CURLM_OK)
        {
            std::stringstream err;
            err << "[curl error " << ec << "], " << msg << ": " << curl_multi_strerror(ec);
            std::cout << err.str();
        }
    }

    int step()
    {
        int active = 0;
        CURLMcode mc = curl_multi_perform(curlm_, &active);
        if (!mc)
        {
            CURLMsg* msg = 0;

            do {
                int nmsg = 0;
                msg = curl_multi_info_read(curlm_, &nmsg);
                if (msg && (msg->msg == CURLMSG_DONE))
                {
                    Session* session = nullptr;
                    curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE, &session);
                    assert(session);
                    session->finish(msg->data.result);
                    remove(*session);
                }

                std::cout << "-- active: " << active << ", number of message: " << nmsg << std::endl;
            }
            while(msg);
        }
        else
        {
            check(mc, "curl_multi_perform");
        }

        return active;
    }
private:
    CURLM* curlm_ = nullptr;
};

// Client Inteface
class IClient
{
public:
    virtual ~IClient() = default;

    virtual std::unique_ptr<Session> get(const std::string& url, const std::string& dnsServer, std::shared_ptr<std::atomic_bool> flag) = 0;
};

class AsyncClient: public IClient
{
public:
    AsyncClient()
    {
        // create thread
        thread_ = std::thread{[&]()
        {
            try
            {
                std::unique_lock<std::mutex> lock(mutex_);

                while (true)
                {
                    // Add incoming requests
                    for (auto& p : incoming_)
                    {
                        pool_.add(*std::get<0>(p));
                        processing_.push_back(std::move(p));
                    }
                    incoming_.clear();

                    // Check for aborted sessions
                    int numOfInprogressRequests = 0;
                    for (auto& p : processing_)
                    {
                        std::unique_ptr<Session>& session = std::get<0>(p);
                        if (aborted_ || session->checkAborted())
                        {
                            pool_.abortIfInProgress(*session);  // this will remove the handle and set abort state
                        }
                        else
                        {
                            ++numOfInprogressRequests;
                        }
                    }

                    // Do I/O
                    int active = 0;
                    if (numOfInprogressRequests > 0)
                    {
                        active = pool_.step();
                    }

                    // Handle completed requests
                    for (auto& p : processing_)
                    {
                        std::unique_ptr<Session>& session = std::get<0>(p);
                        Continuation& then = std::get<1>(p);

                        if (active < 1)
                        {
                            // abort in progress sessions that are not completed if curl is not active anymore.
                            pool_.abortIfInProgress(*session);
                        }

                        if (!session->inProgress())
                        {
                            std::promise<std::unique_ptr<Session>> prom;
                            if (!session->failed())
                            {
                                prom.set_value(std::move(session));
                            }
                            else
                            {
                                std::cout << " aborted: " << session->url() << ". status: " << session->getStatusStr() << std::endl;
                                prom.set_exception(std::make_exception_ptr(RequestError{std::move(session)}));
                            }

                            // trigger callback
                            then(prom.get_future());
                        }
                    }

                    // Remove handled requests(the session has been moved!)
                    processing_.remove_if([](auto& p)
                    {
                        return !std::get<0>(p);
                    });

                    // It's possible to have extra active requests due to DNS over HTTPS activity
                    assert(processing_.size() <= active);

                    // Exit thread if we're shutting down and all requests are handled
                    if (shutdown_ && active < 1)
                    {
                        assert(processing_.empty());
                        lock.unlock();
                        return;
                    }

                    lock.unlock();
                    if (pool_.wait())
                    {
                        lock.lock();
                    }
                    else
                    {
                        std::cout << "work thread failed to poll." << std::endl;
                        // break;
                        lock.lock();
                    }
                }

                std::cout << "work thread exiting ..." << std::endl;
            }
            catch (const std::exception& err)
            {
                std::cerr << "Uncaught exception in async client's work thread: " <<
                          err.what() << std::endl;
                std::terminate();
            }
        }};
    }

    ~AsyncClient() override
    {
        std::cout << "AsyncClient destruction." << std::endl;
        shutdown();
    }

    std::unique_ptr<Session> get(const std::string& url, const std::string& dnsServer, std::shared_ptr<std::atomic_bool> flag) override
    {
        auto prom = std::make_shared<std::promise<std::unique_ptr<Session>>>();
        auto then = [prom](std::future<std::unique_ptr<Session>> res) {
            try
            {
                prom->set_value(res.get());
            }
            catch (...)
            {
                prom->set_exception(std::current_exception());
            }
        };

        getAsync(url, dnsServer, flag, then);
        return prom->get_future().get();
    }

    void getAsync(const std::string& url, const std::string& dnsServer, std::shared_ptr<std::atomic_bool> flag, Continuation then)
    {

        std::unique_lock<std::mutex> lock(mutex_);
        if (shutdown_)
        {
            std::promise<std::unique_ptr<Session>> prom;
            prom.set_exception(std::make_exception_ptr(
                               std::runtime_error("can't perform request on a shut down client")));
            then(prom.get_future());
            return;
        }

        try
        {
            incoming_.emplace_back(pool_.create(Method::GET, url, dnsServer, flag), then);
        }
        catch (const std::exception& e)
        {
            std::promise<std::unique_ptr<Session>> prom;
            prom.set_exception(std::make_exception_ptr(
                std::runtime_error(std::string("Can't perform request due to error: ") + e.what())));
            then(prom.get_future());
        }
    }

    void shutdown()
    {
        std::unique_lock<std::mutex> lock(mutex_);
        if (!shutdown_)
        {
            // set flags
            shutdown_ = true;
            aborted_ = true;

            pool_.wakeup();
            lock.unlock();

            thread_.join();
        }
    }

private:

    using SessionContinuationPair = std::tuple<std::unique_ptr<Session>, Continuation>;

    std::mutex mutex_;

    std::list<SessionContinuationPair> incoming_;
    std::list<SessionContinuationPair> processing_;
    bool shutdown_ = false;
    bool aborted_ = false;

    std::thread thread_;
    Pool pool_;
};

int main(int argc, char* argv[])
{
    std::cout << "=== curl test ===" << std::endl;

    auto url = "https://httpbin.org/delay/5";
    auto dnsServer = "https://1.1.1.1/dns-query";

    auto test_abort_func = [=](IClient& cli, const std::string& dnsServer, int abortAfterMs) {
        auto abortFlag = std::make_shared<std::atomic_bool>(false);
        auto th = std::thread{[&]()
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(abortAfterMs));
            abortFlag->store(true);
        }};

        std::cout << "\n>> start a request that will be aborted." << std::endl;
        try
        {
            auto session = cli.get(url, dnsServer, abortFlag);
            std::cout << ">> abort test " << abortAfterMs << " ms: " << session->getStatusStr() << std::endl;
        }
        catch(RequestError& error)
        {
            std::cout << ">> abort test " << error.response().url() << ". status: " << error.response().getStatusStr() << std::endl;
        }

        std::cout << "\n>> start another request." << std::endl;
        auto flag = std::make_shared<std::atomic_bool>(false);
        cli.get(url, "", flag);
        th.join();
    };

    // test async client with DOH
    std::unique_ptr<IClient> client(new AsyncClient());
    test_abort_func(*client, dnsServer, 500);

    return 0;
}

I expected the following

Below are test reports using different versions of curl.

v8.3.0 & v8.4.0(EXPECTED)

  • curl version:
curl 8.4.0 (x86_64-apple-darwin23.3.0) libcurl/8.4.0 SecureTransport zlib/1.3.1 c-ares/1.19.0
Release-Date: 2023-10-11
Protocols: dict file ftp ftps http https
Features: AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM SSL threadsafe UnixSockets
  • test result:
    This is the expected test result.
-- active: 3, number of message: 0
-- active: 3, number of message: 0
-- active: 3, number of message: 0
> abort flag set.
>> abort session
 aborted: https://httpbin.org/delay/5. status: aborted
>> abort test https://httpbin.org/delay/5. status: aborted

>> start another request.
-- active: 1, number of message: 0
-- active: 1, number of message: 0
-- active: 1, number of message: 0
-- active: 1, number of message: 0
...
-- active: 1, number of message: 0
{
  "args": {}, 
  "data": "", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "deflate, gzip", 
    "Host": "httpbin.org", 
    "X-Amzn-Trace-Id": "Root=1-66b1f303-73a84f6232fbd1f84c3bfe66"
  }, 
  "origin": "12.32.72.250", 
  "url": "https://httpbin.org/delay/5"
}
-- active: 0, number of message: 0
-- active: 0, number of message: 0
AsyncClient destruction.
Pool destruction.

v8.5.0 & v8.6.0 &v8.7.1(NOT EXPECTED)

  • curl version:
curl 8.6.0 (x86_64-apple-darwin23.3.0) libcurl/8.6.0 SecureTransport zlib/1.3.1 c-ares/1.19.0
Release-Date: 2024-01-31
Protocols: dict file ftp ftps http https ipfs ipns
Features: AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM SSL threadsafe UnixSockets
  • test result:
    This is NOT the expected test result.
-- active: 3, number of message: 0
-- active: 3, number of message: 0
> abort flag set.
>> abort session
 aborted: https://httpbin.org/delay/5. status: aborted
>> abort test https://httpbin.org/delay/5. status: aborted

>> start another request.
[curl error 2], curl_multi_perform: Invalid easy handle
[curl error 2], curl_multi_perform: Invalid easy handle
[curl error 2], curl_multi_perform: Invalid easy handle
...
Continuously outputs [curl error 2], curl_multi_perform: Invalid easy handle.

v8.8.0 & v8.9.1(NOT EXPECTED)

  • curl version:
curl 8.8.0 (x86_64-apple-darwin23.3.0) libcurl/8.8.0 SecureTransport zlib/1.3.1 c-ares/1.19.0
Release-Date: 2024-05-22
Protocols: dict file ftp ftps http https ipfs ipns
Features: AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM SSL threadsafe UnixSockets
  • test result:
    This is NOT the expected test result.
-- active: 3, number of message: 0
-- active: 3, number of message: 0
> abort flag set.
>> abort session
 aborted: https://httpbin.org/delay/5. status: aborted
>> abort test https://httpbin.org/delay/5. status: aborted
[curl error 12], curl_multi_poll: Unrecoverable error in select/pollwork thread failed to poll.
[curl error 12], curl_multi_poll: Unrecoverable error in select/pollwork thread failed to poll.

>> start another request.
-- active: 1, number of message: 0
-- active: 1, number of message: 0
-- active: 1, number of message: 0
...
-- active: 1, number of message: 0
{
  "args": {}, 
  "data": "", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "deflate, gzip", 
    "Host": "httpbin.org", 
    "X-Amzn-Trace-Id": "Root=1-66b1f5ea-1e6a192e7bcb152b72084718"
  }, 
  "origin": "12.32.72.250", 
  "url": "https://httpbin.org/delay/5"
}
-- active: 0, number of message: 0
-- active: 0, number of message: 0
AsyncClient destruction.
Pool destruction.

curl/libcurl version

  • v8.3.0, v8.4.0: Good.
  • v8.5.0, v8.6.0, v8.7.1: Bad, return [curl error 2], curl_multi_perform: Invalid easy handle.
  • v8.8.0, v8.9.1: Bad, return [curl error 12], curl_multi_poll: Unrecoverable error in select/pollwork thread failed to poll.

operating system

Darwin Kernel Version 23.3.0: Wed Dec 20 21:28:58 PST 2023; root:xnu-10002.81.5~7/RELEASE_X86_64 x86_64

@luozhaohui luozhaohui changed the title [DOH] Since v8.5.0, a change in API behavior causes curl_multi_poll() to return error code [DOH] Since v8.5.0, a change in API behavior causes curl_multi_poll() or curl_multi_perform() to return error code Aug 6, 2024
@vszakats vszakats added the name lookup DNS and related tech label Aug 6, 2024
@bagder
Copy link
Member

bagder commented Aug 6, 2024

Instead of trying to figure out what your code actually does, I decided to write a (simpler) version in plain C and it just works. Any idea what differences there are between them?

#include <stdio.h>
#include <string.h>

/* curl stuff */
#include <curl/curl.h>

/*
 * Simply download an HTTP file.
 */
int main(void)
{
  CURL *http_handle;
  CURLM *multi_handle;
  int still_running = 1; /* keep number of running handles */

  curl_global_init(CURL_GLOBAL_DEFAULT);

  http_handle = curl_easy_init();

  curl_easy_setopt(http_handle, CURLOPT_URL, "https://httpbin.org/delay/5");
  curl_easy_setopt(http_handle, CURLOPT_DOH_URL, "https://1.1.1.1/dns-query");

  /* init a multi stack */
  multi_handle = curl_multi_init();

  /* add the individual transfers */
  curl_multi_add_handle(multi_handle, http_handle);

  do {
    CURLMcode mc = curl_multi_perform(multi_handle, &still_running);

    if(!mc)
      /* wait for activity, timeout or "nothing" */
      mc = curl_multi_poll(multi_handle, NULL, 0, 1000, NULL);

    if(mc) {
      fprintf(stderr, "curl_multi_poll() failed, code %d.\n", (int)mc);
      break;
    }

  } while(still_running);

  curl_multi_remove_handle(multi_handle, http_handle);

  curl_easy_cleanup(http_handle);

  curl_multi_cleanup(multi_handle);

  curl_global_cleanup();

  return 0;
}

@luozhaohui
Copy link
Author

luozhaohui commented Aug 7, 2024

@bagder Thank you for your response and for taking the time to write sample code.

I would like to share more test results. This issue only occurs when DOH is enabled and curl_multi_remove_handle() is called to remove handles during the execution of curl. This problem started appearing from version v8.5.0. The test program is simplified from the product code (which works with versions v7.68.0 to v8.4.0). It primarily implements two features: setting up DOH and canceling requests.

  • The following test code only sets up DOH and does not cancel requests, the issue does not occur.
    auto url = "https://httpbin.org/delay/5";
    auto dnsServer = "https://1.1.1.1/dns-query";

    auto test_abort_func = [=](IClient& cli, const std::string& dnsServer, int abortAfterMs) {
        auto flag = std::make_shared<std::atomic_bool>(false);

        std::cout << "\n>> start a request." << std::endl;
        auto session = cli.get(url, dnsServer, flag);

        std::cout << "\n>> start another request." << std::endl;
        cli.get(url, dnsServer, flag);
    };
  • The following test code only cancels requests and does not set up DOH, the issue does not occur.
    auto url = "https://httpbin.org/delay/5";
    auto dnsServer = "https://1.1.1.1/dns-query";
    dnsServer = ""; // disable DoH

    auto test_abort_func = [=](IClient& cli, const std::string& dnsServer, int abortAfterMs) {
        auto abortFlag = std::make_shared<std::atomic_bool>(false);
        auto th = std::thread{[&]()
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(abortAfterMs));
            abortFlag->store(true);
        }};

        std::cout << "\n>> start a request that will be aborted." << std::endl;
        try
        {
            auto session = cli.get(url, dnsServer, abortFlag);
            std::cout << ">> abort test " << abortAfterMs << " ms: " << session->getStatusStr() << std::endl;
        }
        catch(RequestError& error)
        {
            std::cout << ">> abort test " << error.response().url() << ". status: " << error.response().getStatusStr() << std::endl;
        }

        std::cout << "\n>> start another request." << std::endl;
        auto flag = std::make_shared<std::atomic_bool>(false);
        cli.get(url, "", flag);
        th.join();
    };

Based on the program run and version test results, I speculate that this is likely related to some changes in DoH in v8.5.0. I hope the information provided will be helpful.

@icing
Copy link
Contributor

icing commented Aug 7, 2024

I did some fixes in regard to DoH handle cleanup in #14212. Maybe there is more lurking?

@luozhaohui
Copy link
Author

luozhaohui commented Aug 7, 2024

@icing Good to hear from you again.
I thought the commit in that PR was already included in v8.9.1. If not, I'll cherry-pick it and give it a try.


Updates:
Bad luck. v8.9.1 already includes those changes, see: d8696dc, so there may be more hidden issues.

@icing
Copy link
Contributor

icing commented Aug 7, 2024

The commit should be in 8.9.1. Maybe there is a code path that it has not covered?

The error indicates that there is an easy handle found that is no longer valid, e.g. was released. The question is what kind of handle it is. If you build libcurl yourself, maybe you could extend the GOOD_EASY_HANDLE check to give some more information if the memory had not been overwritten yet.

@luozhaohui
Copy link
Author

luozhaohui commented Aug 7, 2024

I enabled both enable-curldebug and enable-debug options but had no luck. The assert() checks on the handle were successful. By the way, error code 12 means CURLM_UNRECOVERABLE_POLL. It only returns from the following code. This issue is consistently reproducible since v8.5.0, and if you could run the test program and look into this, I would greatly appreciate it.

File: multi.c

    int pollrc;
#ifdef USE_WINSOCK
    if(cpfds.n)         /* just pre-check with WinSock */
      pollrc = Curl_poll(cpfds.pfds, cpfds.n, 0);
    else
      pollrc = 0;
#else
    pollrc = Curl_poll(cpfds.pfds, cpfds.n, timeout_ms); /* wait... */
#endif
    if(pollrc < 0) {
      result = CURLM_UNRECOVERABLE_POLL;
      goto out;
    }

Please note the different test results for the different versions.

  • v8.3.0, v8.4.0: Good.
  • v8.5.0, v8.6.0, v8.7.1: Bad, return [curl error 2], curl_multi_perform: Invalid easy handle.
  • v8.8.0, v8.9.1: Bad, return [curl error 12], curl_multi_poll: Unrecoverable error in select/poll.

@icing
Copy link
Contributor

icing commented Aug 7, 2024

So, my current thinking is that you app does abort transfers which have DoH request ongoing and the cleanup of those is not handled properly:

  • without DoH, everything works fine, as I understand
  • v8.5.0 introduced a change that did not cleanup DoH handles, leading to "Invalid easy handle." errors
  • v8.8.0 somehow fixed that, but the - probably closed - sockets of the DoH requests ended up in the Curl_poll() and cause that to fail.

Sounds reasonable?

@luozhaohui
Copy link
Author

@icing I think your reasoning makes sense. This issue only happens when canceling an ongoing DoH request.

@icing
Copy link
Contributor

icing commented Aug 12, 2024

@luozhaohui thanks for your patience. I made #14499 to manage the dependencies between a transfer and its DoH requests better. I am not sure this will fix the issue you observer, but I would be interested if this changes anything in your setup.

@luozhaohui
Copy link
Author

@icing Thank you for the update. I tested your branch(multi-id), and the issue has been resolved, with no side effects observed so far.

@icing
Copy link
Contributor

icing commented Aug 13, 2024

I am very happy to hear that!

bagder pushed a commit that referenced this issue Aug 14, 2024
`data->id` is unique in *most* situations, but not in all. If a libcurl
application uses more than one connection cache, they will overlap. This
is a rare situations, but libcurl apps do crazy things. However, for
informative things, like tracing, `data->id` is superior, since it
assigns new ids in curl's serial curl_easy_perform() use.

Introduce `data->mid` which is a unique identifer inside one multi
instance, assigned on multi_add_handle() and cleared on
multi_remove_handle().

Use the `mid` in DoH operations and also in h2/h3 stream hashes.

Reported-by: 罗朝辉
Fixes #14414
Closes #14499
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
name lookup DNS and related tech
Development

No branches or pull requests

4 participants