Resource leak on error paths in thread pools

[Ruslan Ermilov]

On Tue, Oct 13, 2015 at 11:21:45PM +0200, Bart Warmerdam wrote:
> It looks like in the error paths the attr variable is not destroyed.
> Please consider adding this patch to the source base.

No thanks (please see below).

> # HG changeset patch
> # User bartw at xs4all.nl
> # Date 1444770783 -7200
> #      Tue Oct 13 23:13:03 2015 +0200
> # Branch thread_unrelease_attr
> # Node ID c2ae7364ec3f2251b8d734f3be7b62ea413dc36f
> # Parent  2f34ea503ac4e015cc08f6efbb279b360eda609c
> Release attr variable on exit
> 
> diff -r 2f34ea503ac4 -r c2ae7364ec3f src/core/ngx_thread_pool.c
> --- a/src/core/ngx_thread_pool.c	Wed Oct 07 22:19:42 2015 +0300
> +++ b/src/core/ngx_thread_pool.c	Tue Oct 13 23:13:03 2015 +0200
> @@ -140,6 +140,7 @@
>  #if 0
>      err = pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN);
>      if (err) {
> +        (void) pthread_attr_destroy(&attr);
>          ngx_log_error(NGX_LOG_ALERT, log, err,
>                        "pthread_attr_setstacksize() failed");
>          return NGX_ERROR;
> @@ -149,6 +150,7 @@
>      for (n = 0; n < tp->threads; n++) {
>          err = pthread_create(&tid, &attr, ngx_thread_pool_cycle, tp);
>          if (err) {
> +            (void) pthread_attr_destroy(&attr);
>              ngx_log_error(NGX_LOG_ALERT, log, err,
>                            "pthread_create() failed");
>              return NGX_ERROR;

There's no leak here because if ngx_thread_pool_init() fails, the
worker process will exit with an error, effectively releasing all
resources.

We similarly don't care about destroying successfully created
threads if we fail creating the next thread.

> diff -r 2f34ea503ac4 -r c2ae7364ec3f src/os/unix/ngx_thread_mutex.c
> --- a/src/os/unix/ngx_thread_mutex.c	Wed Oct 07 22:19:42 2015
> +0300
> +++ b/src/os/unix/ngx_thread_mutex.c	Tue Oct 13 23:13:03 2015
> +0200
> @@ -89,6 +89,7 @@
>  
>      err = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK);
>      if (err != 0) {
> +        (void) pthread_attr_destroy(&attr);
>          ngx_log_error(NGX_LOG_EMERG, log, err,
>                        "pthread_mutexattr_settype"
>                        "(PTHREAD_MUTEX_ERRORCHECK) failed");
> @@ -97,6 +98,7 @@
>  
>      err = pthread_mutex_init(mtx, &attr);
>      if (err != 0) {
> +        (void) pthread_attr_destroy(&attr);
>          ngx_log_error(NGX_LOG_EMERG, log, err,
>                        "pthread_mutex_init() failed");
>          return NGX_ERROR;

Ditto, but you also misspelled pthread_mutexattr_destroy()
as pthread_attr_destroy().