On Thu, Mar 10, 2011 at 06:02:19PM -0600, Allex, Paul M (USA E D EA STS) wrote:
Sorry for posting my follow up incorrectly.
Anyways, I believe I have located the memory leak. In the pthread version of create_client (sthreads.c), there is the following code:
int create_client(int ls, int s, CLI *arg, void *(*cli)(void *)) { pthread_attr_t pth_attr; ... pthread_attr_init(&pth_attr); ... if(pthread_create(&thread, &pth_attr, cli, arg)) { ... return 0; }
The issue here is that pthread_attr_destroy is not being called for pth_attr after pthread_create is done. This leaks memory allocated into pth_attr by pthread_attr_init.
http://www.kernel.org/doc/man-pages/online/pages/man3/pthread_attr_init. 3.html
I corrected the code locally and tested, and it appears to no longer leak memory for the test I described earlier.
Patch:
*** sthreads.orig.c Thu Mar 10 17:50:03 2011 --- sthreads.c Thu Mar 10 17:50:43 2011 *************** *** 313,318 **** --- 313,319 ---- if(pthread_create(&thread, &pth_attr, cli, arg)) { #ifdef HAVE_PTHREAD_SIGMASK pthread_sigmask(SIG_SETMASK, &oldmask, NULL); /* restore the mask */ + pthread_attr_destroy(&pth_attr); #endif /* HAVE_PTHREAD_SIGMASK */ if(arg) free(arg); *************** *** 322,327 **** --- 323,329 ---- } #ifdef HAVE_PTHREAD_SIGMASK pthread_sigmask(SIG_SETMASK, &oldmask, NULL); /* restore the mask */ + pthread_attr_destroy(&pth_attr); #endif /* HAVE_PTHREAD_SIGMASK */ return 0; }
Thanks,
Paul Allex
Hmmm, doesn't this point to another little buglet? Shouldn't both your new pthread_attr_destroy() and the pthread_attr_create() that is in the current stunnel code be *outside* the #ifdefs? Michal, what do you think of the two attached patches? The first one moves pthread_attr_create() out of the ifdefs, and the second one is the modified memory leak patch of Paul's with the destroying happening outside of the #ifdefs. G'luck, Peter -- Peter Pentchev roam@ringlet.net roam@FreeBSD.org peter@packetscale.com PGP key: http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 I've heard that this sentence is a rumor.