Problem with OpenSSL memory leaks

[BlehPoster]

Description

I am experiencing memory leaks when using yhirose/cpp-httplib with OpenSSL, particularly when integrating with Boost IO services. While I have implemented a workaround for some of the issues, there are still persistent memory leaks that I would like to bring to your attention.

Environment

• httplib version: 0.18.5
• OpenSSL version: 3.3.2
• Boost version: 1.83
• Operating System: Win 10

Steps to Reproduce

1. Integrate httplib with Boost IO services or just regular threads.
2. Invoke httplib functionalities in multi-threaded configurations.
3. Observe memory usage with tools like VLD, Valgrind or similar to detect leaks.

Expected Behavior

No memory leaks should occur when using httplib with OpenSSL and Boost IO services.

Sample Code

Here is a reproducible example:

#define CPPHTTPLIB_OPENSSL_SUPPORT
#include <cpp-httplib/httplib.h>
#include <boost/asio/io_service.hpp>
#include <memory>
#include <vld.h>

class IO_Service_Handler
{
public:
    IO_Service_Handler()
    {
        m_ioservice = std::make_unique<decltype(m_ioservice)::element_type>();
        m_ioservice_work = std::make_unique<decltype(m_ioservice_work)::element_type>(*m_ioservice);
        m_io_thread = std::thread([this]() {
            m_ioservice->run();
        });
    }
    void stop()
    {
        m_ioservice_work.reset();
        if (m_io_thread.joinable())
        {
            m_io_thread.join();
        }
    }
    ~IO_Service_Handler()
    {
        stop();
    }

    boost::asio::io_service* get_io_service() { return m_ioservice.get(); }

private:
    std::thread m_io_thread;
    std::unique_ptr<boost::asio::io_service> m_ioservice;
    std::unique_ptr<boost::asio::io_service::work> m_ioservice_work;
};

struct SSLClient_Factory {
    using SSLClient_Ptr = std::shared_ptr<httplib::SSLClient>;
    struct SSLClient_Ptr_Deleter {
        void operator()(auto* client) const {
            delete client;
            OPENSSL_cleanup(); // Necessary to avoid memory leaks
        };
    };

    static auto create(const std::string& url) {
        return std::unique_ptr<httplib::SSLClient, SSLClient_Ptr_Deleter>(new httplib::SSLClient(std::string("https://") + url));
    };
};

void get(const std::string& url, boost::asio::io_service& io_service) {
    auto work = [url]() mutable {
        auto client = SSLClient_Factory::create(url);
        client->Get("/");
    };

    io_service.post(work);
}

int WINAPI WinMain(HINSTANCE /*hInstance*/, HINSTANCE /*hPrevInstance*/, LPSTR /*lpCmdLine*/, int /*nCmdShow*/) {
    auto h = "echo.free.beeceptor.com";

    IO_Service_Handler io_service_handler_1;
    IO_Service_Handler io_service_handler_2;

    for (auto i = 0; i < 5000; ++i) {
        io_service_handler_1.get_io_service()->post([&]() mutable {
            get(h, *io_service_handler_2.get_io_service());
        });
        get(h, *io_service_handler_2.get_io_service());
    }

    std::this_thread::sleep_for(std::chrono::seconds(25));
    return 0;
}

Remaining memory leaks

  Call Stack (TID 20604):
    ntdll.dll!RtlAllocateHeap()
    minkernel\crts\ucrt\src\appcrt\heap\malloc.cpp (27): t.exe!malloc()
    crypto\mem.c (202): t.exe!CRYPTO_malloc() + 0xA bytes
    crypto\mem.c (222): t.exe!CRYPTO_zalloc() + 0x14 bytes
    crypto\err\err_save.c (24): t.exe!OSSL_ERR_STATE_new()
    crypto\err\err.c (691): t.exe!ossl_err_get_state_int() + 0x5 bytes
    crypto\err\err_mark.c (19): t.exe!ERR_set_mark() + 0x5 bytes
    crypto\conf\conf_mod.c (190): t.exe!CONF_modules_load_file_ex()
    crypto\conf\conf_sap.c (70): t.exe!ossl_config_int() + 0x1C bytes
    crypto\init.c (258): t.exe!ossl_init_config() + 0x7 bytes
    crypto\init.c (256): t.exe!ossl_init_config_ossl_() + 0x12 bytes
    crypto\threads_win.c (492): t.exe!CRYPTO_THREAD_run_once()
    crypto\init.c (601): t.exe!OPENSSL_init_crypto() + 0x13 bytes
    ssl\ssl_init.c (115): t.exe!OPENSSL_init_ssl() + 0xF bytes
    cpp-httplib\httplib.h (9049): t.exe!httplib::detail::SSLInit::SSLInit() + 0xC bytes
    cpp-httplib\httplib.h (9153): t.exe!httplib::detail::`dynamic initializer for 'sslinit_''() + 0x12 bytes
    minkernel\crts\ucrt\src\appcrt\startup\initterm.cpp (22): t.exe!_initterm()
    vctools\crt\vcstartup\src\startup\exe_common.inl (258): t.exe!__scrt_common_main_seh()
    vctools\crt\vcstartup\src\startup\exe_common.inl (331): t.exe!__scrt_common_main()
    vctools\crt\vcstartup\src\startup\exe_winmain.cpp (17): t.exe!WinMainCRTStartup()
    KERNEL32.DLL!BaseThreadInitThunk() + 0x14 bytes
    ntdll.dll!RtlUserThreadStart() + 0x21 bytes

  Call Stack (TID 20604):
    ntdll.dll!RtlReAllocateHeap()
    minkernel\crts\ucrt\src\appcrt\heap\realloc_base.cpp (45): t.exe!_realloc_base() + 0x19 bytes
    minkernel\crts\ucrt\src\appcrt\heap\debug_heap.cpp (758): t.exe!_realloc_dbg() + 0x26 bytes
    minkernel\crts\ucrt\src\appcrt\heap\realloc.cpp (41): t.exe!realloc()
    crypto\mem.c (245): t.exe!CRYPTO_realloc()
    crypto\err\err_blocks.c (103): t.exe!ERR_vset_error() + 0x22 bytes
    crypto\err\err_blocks.c (46): t.exe!ERR_set_error()
    crypto\bio\bss_file.c (72): t.exe!BIO_new_file()
    crypto\conf\conf_def.c (175): t.exe!def_load() + 0x11 bytes
    crypto\conf\conf_lib.c (258): t.exe!NCONF_load()
    crypto\conf\conf_mod.c (207): t.exe!CONF_modules_load_file_ex() + 0x12 bytes
    crypto\conf\conf_sap.c (70): t.exe!ossl_config_int() + 0x1C bytes
    crypto\init.c (258): t.exe!ossl_init_config() + 0x7 bytes
    crypto\init.c (256): t.exe!ossl_init_config_ossl_() + 0x12 bytes
    crypto\threads_win.c (492): t.exe!CRYPTO_THREAD_run_once()
    crypto\init.c (601): t.exe!OPENSSL_init_crypto() + 0x13 bytes
    ssl\ssl_init.c (115): t.exe!OPENSSL_init_ssl() + 0xF bytes
    cpp-httplib\httplib.h (9049): t.exe!httplib::detail::SSLInit::SSLInit() + 0xC bytes
    cpp-httplib\httplib.h (9153): t.exe!httplib::detail::`dynamic initializer for 'sslinit_''() + 0x12 bytes
    minkernel\crts\ucrt\src\appcrt\startup\initterm.cpp (22): t.exe!_initterm()
    vctools\crt\vcstartup\src\startup\exe_common.inl (258): t.exe!__scrt_common_main_seh()
    vctools\crt\vcstartup\src\startup\exe_common.inl (331): t.exe!__scrt_common_main()
    vctools\crt\vcstartup\src\startup\exe_winmain.cpp (17): t.exe!WinMainCRTStartup()
    KERNEL32.DLL!BaseThreadInitThunk() + 0x14 bytes
    ntdll.dll!RtlUserThreadStart() + 0x21 bytes

[yhirose]

@BlehPoster thanks for the feedback, but I am not really sure that this is memory leak...

[yhirose]

@BlehPoster could you try with v0.18.3 and v0.18.4 to see if there is any difference? Thanks a lot!

[BlehPoster]

@yhirose I tested different versions, but they had no effect on the outcome.

The example I provided already works around many of the existing memory leaks. However, the following code demonstrates a scenario that creates more than 40 leaking data blocks:

#define CPPHTTPLIB_OPENSSL_SUPPORT
#include <cpp-httplib/httplib.h>
#include <vld.h>
#include <thread>

int WINAPI WinMain(HINSTANCE, HINSTANCE, LPSTR, int) {

    auto url = "echo.free.beeceptor.com";
    std::thread([&]() {
        auto client = httplib::SSLClient(std::string("https://") + url);
        client.Get("/");
    }).join();

    return 0;
}

[yhirose]

@BlehPoster I found a bug in your code. The string in httplib::SSLClient constructor shouldn't take a scheme string https://.
Could you try with either httplib::SSCLient(url) or httplib::Client(std::string("https://") + url) to see if the memory leak still happens?

You can see all the correct usages with Client and SSLClient here:
https://github.com/yhirose/cpp-httplib?tab=readme-ov-file#ssl-support

[BlehPoster]

I apologize for the mistake. During testing and debugging, I repeatedly modified the code. I have now retested the following code, and the issue still occurs.
I updated the example according to the instructions in your README file:

#define CPPHTTPLIB_OPENSSL_SUPPORT
#include <cpp-httplib/httplib.h>

#include <vld.h>
#include <thread>

int WINAPI WinMain(HINSTANCE, HINSTANCE, LPSTR, int) {
    /* From simple example*/
    /* Does not leak but get a connection error (assert failed) */
    std::thread([&]() {
        httplib::Client cli("https://yhirose.github.io/hi");
        auto res = cli.Get("/hi");
        assert(res); // failure
    }).join();

    /* From client example, but using yhirose.github.io */
    /* Does leak, but receive a valid reply */
    std::thread([&]() {
        httplib::Client cli("https://yhirose.github.io");
        auto res = cli.Get("/hi");
        assert(res); // ok
    }).join();

    return 0;
}

I debugged the client creation to investigate why httplib::Client cli("https://yhirose.github.io/hi"); results in a non-functional client. I discovered that the regex does not match, leading to the creation of a ClientImpl instead of a SSLClient.

For testing, I updated the regex to:

const static std::regex re(
    R"((?:([a-z]+):\/\/)?(?:\[([a-fA-F\d:]+)\]|([^:/?#]+))(?::(\d+))?(\/[^\s]*)?)");

With this updated regex, httplib::Client now internally creates a SSLClient instead of a ClientImpl. However, I still encountered memory leaks during testing.

[yhirose]

@BlehPoster sorry that my recent change in README isn't correct and I just updated it.

        httplib::Client cli("https://yhirose.github.io/hi");

should be

        httplib::Client cli("https://yhirose.github.io");

[BlehPoster]

@yhirose I finally figured it out. I recreated the SSL code in a separate, smaller project and reduced it to its simplest form while still reproducing the memory leak. Unfortunately, I should have approached the problem from the other direction, as creating an SSL context itself leads to significant memory leaks when done in a separate thread.

std::thread([]() {
    auto ctx_ = SSL_CTX_new(TLS_client_method());
    SSL_CTX_free(ctx_);
}).join();

However, using OPENSSL_thread_stop() resolves all memory leaks. Unfortunately, I can't see an easy way to determine if an HTTP client is running in a separate worker thread that will terminate after the call. This means the library user would need to either call OPENSSL_thread_stop() manually or notify the library to cleanup.

A possible solution could be adding a flag that ensures OPENSSL_thread_stop() is called during the destruction of the SSLClient. But I am not sure about that though.

For now, my personal workaround is to explicitly call OPENSSL_thread_stop() before shutting down the io_service worker thread.

[yhirose]

@BlehPoster thanks for the research. Is it the same as openssl/openssl#22831?

[BlehPoster]

@yhirose yes, the two problems are quite similar. the fact that openssl expects the user to manually free the memory for each thread is a bit worrying. it also creates significant problems for me as i don't always have direct access to all threads in my production environment (boost asio). But yes, this is not a bug on your part, but a feature of openssl.

[yhirose]

I'll leave it with 'information' tag.
https://docs.openssl.org/master/man3/OPENSSL_init_crypto/#notes