This bug is reported by Chris Houck. In pruthr.c, _PR_NativeDestroyThread(), the thread private data is not disposed of and hence leaked. This memory leak is caught by Purify. Going through the file, I spotted other places where thread private data is not disposed of. We need to do a careful code review. The pthreads version does not seem to have this problem.
Created attachment 2497 [details] [diff] [review] The patch that Chris Houck submitted. Note that this is not the complete fix.
I thoroughly reviewed our TPD code and fixed other problems that I noticed. I'm going to post a patch with comments.
Created attachment 2899 [details] [diff] [review] A patch for the memory leak and other problems in our TPD code.
Comments on the patch (attachment id 2899): 1. primpl.h: the global variable _pr_tpd_destructors does not need to be exported from prtpd.c. 2. ptthread.c: removed unused field 'highwater' from pt_book. Removed commented out code. _pt_thread_death now needs to free thred->privateData because _PR_DestroyThreadPrivate no longer does that. 3. prcthr.c: _PR_CleanupThread should call _PR_DestroyThreadPrivate to free the thread-private data because that function takes care of all the necessary iterations (4) of invoking the destructors. 4. prtpd.c: Removed unused macro _PR_TPD_MODULO and made _pr_tpd_destructors static. PR_NewThreadPrivateIndex should return index starting from 0 as opposed to 1. Fixed several problems in PR_SetThreadPrivate and _PR_DestroyThreadPrivate. Also, _PR_DestroyThreadPrivate now doesn't free self->privateData, so that it can be called by _PR_CleanupThread to prepare a dead thread for recycling. 5. pruthr.c: _PR_NativeDestroyThread should free thread->privateData to plug the memory leak that chouck reported.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
I checked in the fix. /cvsroot/mozilla/nsprpub/pr/include/private/primpl.h, revision 3.27 /cvsroot/mozilla/nsprpub/pr/src/pthreads/ptthread.c, revision 3.28 /cvsroot/mozilla/nsprpub/pr/src/threads/prcthr.c, revision 3.10 /cvsroot/mozilla/nsprpub/pr/src/threads/prtpd.c, revision 3.7 /cvsroot/mozilla/nsprpub/pr/src/threads/combined/pruthr.c, revision 3.16 The fix is also checked into the internal repository. /m/src/ns/nspr20/pr/include/private/primpl.h, revision 2.68 /m/src/ns/nspr20/pr/src/pthreads/ptthread.c, revision 2.56 /m/src/ns/nspr20/pr/src/threads/prcthr.c, revision 2.15 /m/src/ns/nspr20/pr/src/threads/prtpd.c, revision 2.10 /m/src/ns/nspr20/pr/src/threads/combined/pruthr.c, revision 2.33 Chouck, can you verify that the memory leak is gone? Thanks.
After applying your patch Purify is no longer reporting leaked thread private data. Thanks, -Chris
Thanks, Chris. Marked the bug verified.
The patch looks good. In _pt_thread_death, shouldn't thred->errorString, thred->io_cv and thred->privateData be set to NULL?
Setting those fields of 'thred' to NULL is not necessary because 'thred' will be freed.
This isn't a bug, I was suggesting it more as a safety feature, if the threads are recycled in the feature. When resources are freed conditionally, it is better to reset the condition after freeing the resources.
You need to log in before you can comment on or make changes to this bug.