pistache: Memory leaks (general)
Pistache (as of master/head, right now), has some memory leaks in it. It leaks on every bind() (minor problem) and on every processed request (major problem). These are several open bugs reporting memory leaks in various parts of pistache, however, most of those bugs were filed before the recent major rounds of code changes. In time, I plan to close those as duplicates against this issue, and track memory leaks here.
Once we fix all of the memory leaks, I intend to add memory leak checking (via valgrind) to the travis-ci, and auto-reject all PRs that introduce memory leaks.
IMHO, fixing crashes and memory leaks should be our first priority, followed by test flakiness as second priority.
One can observe the leaks by simply running valgrind against the compiled test binaries:
#!/bin/bash
DIR="."
# Ensure that we are at a pistache git root
[[ -d "${DIR}/examples" ]] || exit $?
[[ -f "${DIR}/src/common/transport.cc" ]] || exit $?
cd "./${DIR}" || exit $?
function do_check {
CC=$1
BUILD_DIR="${DIR}/build-${CC}"
for TEST in ${BUILD_DIR}/tests/run_*; do
LOG="${TEST}.xml"
valgrind -q --xml-file=${LOG} --xml=yes --error-exitcode=1 ${TEST}
done
}
# do_check clang
do_check gcc
$ grep "Leak_DefinitelyLost" -l build-gcc/tests/*.xml
build-gcc/tests/run_async_test.xml
build-gcc/tests/run_cookie_test_3.xml
build-gcc/tests/run_http_client_test.xml
build-gcc/tests/run_http_server_test.xml
build-gcc/tests/run_listener_test.xml
build-gcc/tests/run_payload_test.xml
build-gcc/tests/run_rest_server_test.xml
build-gcc/tests/run_streaming_test.xml
About this issue
- Original URL
- State: open
- Created 6 years ago
- Reactions: 1
- Comments: 40 (31 by maintainers)
Commits related to this issue
- Merge pull request #425 from AdrianCX/master https://github.com/oktal/pistache/issues/399 — committed to pistacheio/pistache by dennisjenkins75 5 years ago
Hi,
Thanks for merging the fix. As FYI, this is how to catch this in the future:
Compile with debug symbols Use flag: -DCMAKE_BUILD_TYPE=Debug - or just set up ‘-g’ in the cmake files, or both…
Run valgrind with full leak check
Then we trigger a new allocation - In another terminal generate a connection and close it:
We have code:
Disable pretty printer - we want to figure what the address of the refcount is
Figure out where the refcount is:
Bingo: peer._M_refcount._M_pi._M_use_count
where code is:
So, it leaks when pushed to peersQueue. peersQueue doesn’t seem to decrease it. With a bit of looking over code we see that peersQueue is a lock free queue and the shared_ptr is stored via inplace new in a local storage and that’s never cleared.
Hope it helps, Project looks fun, I’ll see if I can help with other bugs.
Regards, Adrian Cruceru
I did not see any remaining in unit tests or my testing.
If you spot one with valgrind you can post it here and I/someone else can help fix it.
Current status:
Well, this is unfortunate: “remote: Permission to oktal/pistache.git denied to AdrianCX”
How can I get access? 😃
In the meantime, here are the fixes: memleakfixes.diff.gz
If anyone is interested how I debugged/traced them I can add a follow up comment next few days.
Here’s a possible root cause for the leak:
peer.h:
Notice the “void”. When that is hash table is destroyed, the shared_ptrs will not destroy objects.
putData does:
If peer dies with data in the hashTable then that data is leaked. I don’t see any destructor in peer that would clean the hash table.
To test: Apply fix: https://github.com/arthurafarias/pistache/commit/90b7e324a141ffb48617421a3e57e9e106a5186a
Disable SIGINT signal handler (we want to close valgrind with CTRL-C)
Do some requests (telnet to 9080 and close it without sending an HTTP request) Close application.
@kiplingw Thanks, that’s good info, I will try and update the unit test / fix any issues I see while doing that.
Looking over Http::Client there is some weird logic involving multi-threading - doesn’t seem very safe to me so your note is valid - not really ready for production code.
bocse: Unfortunately, no. If I had a quick work around, I would have fixed the bug outright. That last I spoke with knoweldge4igor@ and hydratim@, they were working (independently?) on rewriting the problematic code. I could be mistaken though.
I’ve spent a few hours digging into the leak myself. I suspect that there is a circular smart pointer reference. A destructor is never executing.
For my own project which uses Pistache, the leak is not severe enough to cause a problem until I’ve had 100k’s of requests. Based on that, I suspect that the only thing leaking is the Peer object itself, but not the request or response buffers. But its been a few weeks since I dug into the problem.
I think so. “include/pistache/peer.h” contains a few member variables, but the “data_” is of particular interest. It is the buffered data from the TCP socket.