Fix CFileTree::GetNodeFullPath memory leak #53
Conversation
It also use const char* instead of char* for immutable strings
src/FileTree.cpp
Outdated
| } | ||
| } | ||
|
|
||
| char* CFileTree::GetNodeFullPath(tree_node_<FILE_ITEM>* node) | ||
| std::string CFileTree::GetNodeFullPath(tree_node_<FILE_ITEM>* node) |
There was a problem hiding this comment.
The effective fix is here (returning std::string instead of char*).
|
After more that one day running this fork, #52 didn't occur. The process actually eats up far less memory than it did (actually 52 016K for Private Bytes, and 2 776K for Working Set). |
|
I still need to work on this. In some case, getPathByHandle can return NULL, but it cause a crash because method has to return std::string. I'll add an exception instead of returning NULL, and catch it to return a proper error to NFS client. |
|
@marcharding, I just add a new commit bcf1689 that fixes crashes when an invalid handle (unknown tree item) is requested by the client. It should fix #54. It has occured on my environment in a reproductible manner, so I could debug and fix this issue properly. It also pass std::string by reference. That's the first time I write C++, so I was a bit confused at first with those changes, but I think I now understand the keys of memory management. I attach the binary from HEAD of this branch for people who wants to test. |
Toilal
force-pushed
the
fix-getpath-leak
branch
from
bcf1689 to
633a1c0
Compare
|
Looks good so far, i'll test it a few more days to see if there are any regressions. 👍 👍 👍 |
|
I've been using it for some days now and it's stable for me. The memory consumption stopped to increase and is stabilized around 50MB. |
|
We have been running this on a team of 5 developers since one month, and no more crash too, at least when using UDP protocol. I think it could be merged and released @marcharding please :) |
|
Will do later today 👍 |
It also use const char* instead of char* for immutable strings