sagebind / isahc

The practical HTTP client that is fun to use.
https://docs.rs/isahc
MIT License
705 stars 62 forks source link

Memory Leak detected with Valgrind #427

Open tunakasif opened 1 year ago

tunakasif commented 1 year ago

The simple example provided in README.md (also provided below for reference) generates memory leak. The cargo-valgrind output is also provided below. AFAIK, it is common to see still reachable leaks especially in ffi code, but this example results in possibly lost leaks. I don't know if this is expected due to libcurl utilization, but wanted to point it out.

Minimal Cargo.toml dependencies:

[dependencies]
isahc = "1.7.2"

Given minimal src/main.rs use example:

use isahc::prelude::*;

fn main() -> Result<(), isahc::Error> {
    let mut response = isahc::get("https://example.org")?;
    println!("Status: {}", response.status());
    println!("Headers: {:#?}", response.headers());
    print!("{}", response.text()?);
    Ok(())
}

The cargo-valgrind output indicating memory leaks:

Error leaked 24 B in 1 block
        Info at malloc (vg_replace_malloc.c:393)
             at alloc::alloc::Global::alloc_impl (alloc.rs:95)
             at allocate (alloc.rs:237)
             at alloc::alloc::exchange_malloc (alloc.rs:326)
             at alloc::sync::Arc<T>::new (boxed.rs:220)
             at waker_fn::waker_fn (lib.rs:29)
             at isahc::agent::selector::Selector::waker (selector.rs:61)
             at isahc::agent::AgentBuilder::spawn (mod.rs:95)
             at isahc::client::HttpClientBuilder::build (client.rs:459)
             at isahc::client::HttpClient::new (client.rs:625)
             at isahc::client::HttpClient::shared::SHARED::{{closure}} (client.rs:633)
             at core::ops::function::FnOnce::call_once (function.rs:507)
             at core::ops::function::FnOnce::call_once (function.rs:507)
       Error leaked 24 B in 1 block
        Info at malloc (vg_replace_malloc.c:393)
             at alloc::alloc::Global::alloc_impl (alloc.rs:95)
             at allocate (alloc.rs:237)
             at alloc::alloc::exchange_malloc (alloc.rs:326)
             at alloc::sync::Arc<T>::new (boxed.rs:220)
             at waker_fn::waker_fn (lib.rs:29)
             at isahc::agent::selector::Selector::waker (selector.rs:61)
             at isahc::agent::AgentContext::new (mod.rs:324)
             at isahc::agent::AgentBuilder::spawn::{{closure}} (mod.rs:121)
             at std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:121)
             at std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}} (mod.rs:550)
             at <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once (unwind_safe.rs:271)
             at std::panicking::try::do_call (panicking.rs:483)
       Error leaked 24 B in 1 block
        Info at malloc (vg_replace_malloc.c:393)
             at alloc::alloc::Global::alloc_impl (alloc.rs:95)
             at allocate (alloc.rs:237)
             at alloc::alloc::exchange_malloc (alloc.rs:326)
             at alloc::sync::Arc<T>::new (boxed.rs:220)
             at waker_fn::waker_fn (lib.rs:29)
             at futures_lite::future::block_on::parker_and_waker (future.rs:68)
             at futures_lite::future::block_on::CACHE::__init (future.rs:76)
             at futures_lite::future::block_on::CACHE::__getit::{{closure}} (local.rs:354)
             at std::thread::local::lazy::LazyKeyInner<T>::initialize (local.rs:810)
             at std::thread::local::fast::Key<T>::try_initialize (local.rs:988)
             at std::thread::local::fast::Key<T>::get (local.rs:971)
             at futures_lite::future::block_on::CACHE::__getit (local.rs:346)
       Error leaked 116 B in 1 block
        Info at malloc (vg_replace_malloc.c:393)
             at alloc::alloc::Global::alloc_impl (alloc.rs:95)
             at <alloc::alloc::Global as core::alloc::Allocator>::allocate (alloc.rs:237)
             at hashbrown::raw::alloc::inner::do_alloc (alloc.rs:11)
             at hashbrown::raw::RawTableInner<A>::new_uninitialized (mod.rs:1080)
             at hashbrown::raw::RawTableInner<A>::fallible_with_capacity (mod.rs:1109)
             at hashbrown::raw::RawTableInner<A>::prepare_resize (mod.rs:1353)
             at hashbrown::raw::RawTable<T,A>::reserve_rehash (mod.rs:1426)
             at hashbrown::raw::RawTable<T,A>::reserve (mod.rs:646)
             at hashbrown::raw::RawTable<T,A>::insert (mod.rs:725)
             at hashbrown::map::HashMap<K,V,S,A>::insert (map.rs:1679)
             at std::collections::hash::map::HashMap<K,V,S>::insert (map.rs:1104)
       Error leaked 288 B in 1 block
        Info at calloc (vg_replace_malloc.c:1340)
             at UnknownInlinedFun (rtld-malloc.h:44)
             at allocate_dtv (dl-tls.c:375)
             at _dl_allocate_tls (dl-tls.c:634)
             at allocate_stack (allocatestack.c:423)
             at pthread_create@@GLIBC_2.34 (pthread_create.c:650)
             at std::sys::unix::thread::Thread::new (thread.rs:87)
             at std::thread::Builder::spawn_unchecked_ (mod.rs:584)
             at std::thread::Builder::spawn_unchecked (mod.rs:478)
             at std::thread::Builder::spawn (mod.rs:410)
             at isahc::agent::AgentBuilder::spawn (mod.rs:140)
             at isahc::client::HttpClientBuilder::build (client.rs:459)
             at isahc::client::HttpClient::new (client.rs:625)
             at isahc::client::HttpClient::shared::SHARED::{{closure}} (client.rs:633)
     Summary Leaked 476 B total
sagebind commented 1 year ago

Thanks for opening an issue! At first glance it looks like this memory "leak" is just the global Isahc client, which does get allocated on the heap and is not cleaned up. This is because when using the simple isahc::get API, it allocates a global HttpClient under the hood.

This is a busy time for me right now and I don't have a ton of time to look at this yet, but definitely something worth looking in to. If you are feeling adventurous, I would be curious if there are any such leaks reported when only using the HttpClient API directly.

tunakasif commented 1 year ago

@sagebind You were right 🎉, the following snippet first generates a HttpClient and calls .get() with the generated client.

fn main() -> Result<(), isahc::Error> {
    let client = isahc::HttpClient::new()?;
    let response = client.get("https://example.org")?;
    println!("Status: {}", response.status());
    Ok(())
}

It does not generate possibly lost leaks, only still reachable ones (as provided below), which is expected. So, the answer of the question

if are there any such leaks reported when only using the HttpClient API ?

is no. I/You can close the issue if you think this is satisfactory, or keep it open if you think it is necessary to address global HttpClient allocation. Thank you 👍🏻

==399934== Memcheck, a memory error detector
==399934== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==399934== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info
==399934== Command: ./target/debug/memory
==399934==
Status: 200 OK
==399934==
==399934== HEAP SUMMARY:
==399934==     in use at exit: 3,659 bytes in 11 blocks
==399934==   total heap usage: 142,797 allocs, 142,786 frees, 6,467,777 bytes allocated
==399934==
==399934== LEAK SUMMARY:
==399934==    definitely lost: 0 bytes in 0 blocks
==399934==    indirectly lost: 0 bytes in 0 blocks
==399934==      possibly lost: 0 bytes in 0 blocks
==399934==    still reachable: 3,659 bytes in 11 blocks
==399934==         suppressed: 0 bytes in 0 blocks
==399934== Rerun with --leak-check=full to see details of leaked memory
==399934==
==399934== For lists of detected and suppressed errors, rerun with: -s
==399934== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)