[PATCH] Memory leak in pthreadpool

[Jeremy Allison]

On Fri, Mar 10, 2017 at 10:53:46AM +0100, Ralph Böhme wrote:
> Hi!
> 
> When copying large files from the server to the client with aio enabled we
> noticed that smbd kept growing RSS and VSZ.
> 
> valgrind reported:
> 
>     ==2503== 4,093,440 bytes in 6,560 blocks are possibly lost in loss record 460 of 460
>     ==2503==    at 0x4C299CE: calloc (vg_replace_malloc.c:711)
>     ==2503==    by 0x4011C24: _dl_allocate_tls (in /usr/lib64/ld-2.17.so)
>     ==2503==    by 0x4E3C960: pthread_create@@GLIBC_2.2.5 (in /usr/lib64/libpthread-2.17.so)
>     ==2503==    by 0x9B298AE: pthreadpool_add_job (in /usr/lib64/samba/libmessages-dgm-samba4.so)
>     ==2503==    by 0x9B29FDC: pthreadpool_tevent_job_send (in /usr/lib64/samba/libmessages-dgm-samba4.so)
>     ==2503==    by 0x56A78EF: ??? (in /usr/lib64/samba/libsmbd-base-samba4.so)
>     ==2503==    by 0x55D86B7: smb_vfs_call_pread_send (in /usr/lib64/samba/libsmbd-base-samba4.so)
>     ==2503==    by 0x55F7543: schedule_smb2_aio_read (in /usr/lib64/samba/libsmbd-base-samba4.so)
>     ==2503==    by 0x5608F57: smbd_smb2_request_process_read (in /usr/lib64/samba/libsmbd-base-samba4.so)
>     ==2503==    by 0x55FCB6C: smbd_smb2_request_dispatch (in /usr/lib64/samba/libsmbd-base-samba4.so)
>     ==2503==    by 0x55FD7DC: ??? (in /usr/lib64/samba/libsmbd-base-samba4.so)
>     ==2503==    by 0x641B977: ??? (in /usr/lib64/samba/libtevent.so.0.9.31)
> 
> The problem seems to be caused by worked threads that are not properly started
> in detached state and thus their tls is not reclaimed upon thread termination.
> 
> In pthreadpool.c we prepare a pthread attribute with PTHREAD_CREATE_DETACHED,
> but we don't pass it to pthread_create().
> 
> The patch is already reviewed my metze and Volker so I'm going to push later if
> noone objects.

Oh good catch ! RB+.

> From 20de4cea4af36c0323e7c3f03cab5df66e193e33 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 9 Mar 2017 19:49:56 +0100
> Subject: [PATCH] lib/pthreadpool: fix a memory leak
> 
> When copying large files from the server to the client with aio enabled
> we noticed that smbd kept growing RSS and VSZ.
> 
> valgrind was reporting:
> 
> ==2503== 4,093,440 bytes in 6,560 blocks are possibly lost in loss record 460 of 460
> ==2503==    at 0x4C299CE: calloc (vg_replace_malloc.c:711)
> ==2503==    by 0x4011C24: _dl_allocate_tls (in /usr/lib64/ld-2.17.so)
> ==2503==    by 0x4E3C960: pthread_create@@GLIBC_2.2.5 (in /usr/lib64/libpthread-2.17.so)
> ==2503==    by 0x9B298AE: pthreadpool_add_job (in /usr/lib64/samba/libmessages-dgm-samba4.so)
> ==2503==    by 0x9B29FDC: pthreadpool_tevent_job_send (in /usr/lib64/samba/libmessages-dgm-samba4.so)
> ==2503==    by 0x56A78EF: ??? (in /usr/lib64/samba/libsmbd-base-samba4.so)
> ==2503==    by 0x55D86B7: smb_vfs_call_pread_send (in /usr/lib64/samba/libsmbd-base-samba4.so)
> ==2503==    by 0x55F7543: schedule_smb2_aio_read (in /usr/lib64/samba/libsmbd-base-samba4.so)
> ==2503==    by 0x5608F57: smbd_smb2_request_process_read (in /usr/lib64/samba/libsmbd-base-samba4.so)
> ==2503==    by 0x55FCB6C: smbd_smb2_request_dispatch (in /usr/lib64/samba/libsmbd-base-samba4.so)
> ==2503==    by 0x55FD7DC: ??? (in /usr/lib64/samba/libsmbd-base-samba4.so)
> ==2503==    by 0x641B977: ??? (in /usr/lib64/samba/libtevent.so.0.9.31)
> 
> The problem seems to be caused by worked threads that are not properly
> started in detached state and thus their tls is not reclaimed upon
> thread termination.
> 
> In pthreadpool.c we prepare a pthread attribute with
> PTHREAD_CREATE_DETACHED, but we don't pass it to pthread_create().
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12624
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> Reviewed-by: Volker Lendecke <vl at samba.org>
> Reviewed-by: Stefan Metzmacher <metze at samba.org>
> ---
>  lib/pthreadpool/pthreadpool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/pthreadpool/pthreadpool.c b/lib/pthreadpool/pthreadpool.c
> index eaddd44..f97cdcc 100644
> --- a/lib/pthreadpool/pthreadpool.c
> +++ b/lib/pthreadpool/pthreadpool.c
> @@ -534,7 +534,7 @@ int pthreadpool_add_job(struct pthreadpool *pool, int job_id,
>  		return res;
>  	}
>  
> -	res = pthread_create(&thread_id, NULL, pthreadpool_server,
> +	res = pthread_create(&thread_id, &thread_attr, pthreadpool_server,
>  			     (void *)pool);
>  	if (res == 0) {
>  		pool->num_threads += 1;
> -- 
> 2.9.3
>