-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
Fix big memory leak on client disconnect #574
Conversation
A pthread needs to be joined or detached to prevent memory leaks
From the man page of pthread_join; NOTES top
|
Hi, thanks for the PR and sorry for the delay! 🙇 I think I understood the basic problem and what this PR fixes, I just have a few questions, marked in the PR's code. |
|
||
#ifdef LIBVNCSERVER_HAVE_LIBPTHREAD | ||
// We can't join this thread, detach it here so the associated memory will be feed | ||
pthread_detach(cl->client_thread); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two thoughts here:
- Maybe rather
pthread_detach(pthread_self());
for more clarity? - What about
LIBVNCSERVER_HAVE_WIN32THREADS
? That might need a fix as well, the original code only did a_beginthread()
and that was it. Only found https://stackoverflow.com/questions/12744324/how-to-detach-a-thread-on-windows-c that mentions something...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I am unsure this is more clear. But I think it will work. However it would be best to test first (as the other code was tested). For me it is at the moment not so easy to test,
- Sadly I have no knowledge of windows threads. A similar fix might be needed. The pull request however only fixes the pthread memory leak.
So just to give some background. I was working on a system, using libsvnserver in the threaded variation, and I was loosing memory (memory leak). So I started testing and noticed with htop on the server that, each time the client disconnects, I was loosing memory. So I focussed on libsvnserver and after a long time testing found that the memory leak didn't happen when I was using libsvnserver without threading. So I focussed on threading, disabling code to find where the leak was. And in the end found the leak happened at thread ending. At that time I noticed that the leak happened at the end of the thread. So I investigated the thread and found that it wasn't joined at the end. Only when the server is stopped the, at that time, still running thread was joined. At this point I knew what the problem was but I couldn't find a good place to join the client thread. So I decided it would be easier to detach the thread as a detached thread frees it's own memory (no join needed). |
A pthread needs to be joined or detached to prevent memory leaks. Only when starting the server with the runInBackground set to true and using pthreads.