unsubscribe maillist for me plz
[email protected] From: [email protected] Date: 2015-10-14 20:00 To: [email protected] Subject: nginx-devel Digest, Vol 72, Issue 10 Send nginx-devel mailing list submissions to [email protected] To subscribe or unsubscribe via the World Wide Web, visit http://mailman.nginx.org/mailman/listinfo/nginx-devel or, via email, send a message with subject or body 'help' to [email protected] You can reach the person managing the list at [email protected] When replying, please edit your Subject line so it is more specific than "Re: Contents of nginx-devel digest..." Today's Topics: 1. Re: Patch for s390x support (Maxim Dounin) 2. Re: Patch for s390x support (Neale Ferguson) 3. Resource leak on error paths in thread pools (Bart Warmerdam) 4. Error path does not close fd (Bart Warmerdam) 5. Re: Resource leak on error paths in thread pools (Ruslan Ermilov) ---------------------------------------------------------------------- Message: 1 Date: Tue, 13 Oct 2015 15:44:08 +0300 From: Maxim Dounin <[email protected]> To: [email protected] Subject: Re: Patch for s390x support Message-ID: <[email protected]> Content-Type: text/plain; charset=utf-8 Hello! On Mon, Oct 12, 2015 at 05:23:04PM +0000, Neale Ferguson wrote: > Hi, > I would like to contribute the a fix to enable the Linux s390x platform. > The fix was built against today?s mercurial master and pertains to the gcc > atomic functions. I note the other architectures use inline assembler > rather than the gcc builtin operations. Is this for historical reasons? GCC builtin atomic operations are used when available, grep NGX_HAVE_GCC_ATOMIC for details. -- Maxim Dounin http://nginx.org/ ------------------------------ Message: 2 Date: Tue, 13 Oct 2015 13:22:58 +0000 From: Neale Ferguson <[email protected]> To: "[email protected]" <[email protected]> Subject: Re: Patch for s390x support Message-ID: <d2427d0b.4508c%[email protected]> Content-Type: text/plain; charset="utf-7" Thanks. I have the non-builtin version of the atomic operations written for s390x, so on the off chance if someone wants to build using gcc < 4.1 then things will build/run. However, given then low probability of that is it worth submitting the patch? On 10/13/15, 8:44 AM, "nginx-devel on behalf of Maxim Dounin" <[email protected] on behalf of [email protected]> wrote: >Hello! > >On Mon, Oct 12, 2015 at 05:23:04PM ?, Neale Ferguson wrote: > >> Hi, >> I would like to contribute the a fix to enable the Linux s390x platform. >> The fix was built against today?s mercurial master and pertains to the >>gcc >> atomic functions. I note the other architectures use inline assembler >> rather than the gcc builtin operations. Is this for historical reasons? > >GCC builtin atomic operations are used when available, grep >NGX_HAVE_GCC_ATOMIC for details. ------------------------------ Message: 3 Date: Tue, 13 Oct 2015 23:21:45 +0200 From: Bart Warmerdam <[email protected]> To: [email protected] Subject: Resource leak on error paths in thread pools Message-ID: <[email protected]> Content-Type: text/plain; charset="us-ascii" It looks like in the error paths the attr variable is not destroyed. Please consider adding this patch to the source base. Regards, B. # HG changeset patch # User [email protected] # 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; 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; ------------------------------ Message: 4 Date: Tue, 13 Oct 2015 23:58:04 +0200 From: Bart Warmerdam <[email protected]> To: [email protected] Subject: Error path does not close fd Message-ID: <[email protected]> Content-Type: text/plain; charset="iso-8859-1" Hello, In the ngx_daemon.c the dup2 result code is checked but the earlier opened /dev/null handle is not closed in case of an error. Please consider this path to add to the source base. Regards, B. # HG changeset patch # User Bart Warmerdam <[email protected]> # Date 1444773372 -7200 #??????Tue Oct 13 23:56:12 2015 +0200 # Branch close_fd_on_error # Node ID e1e25db76cdf7583f1145b91b9dbcdff417d6f16 # Parent??2f34ea503ac4e015cc08f6efbb279b360eda609c Close file handle on error as well diff -r 2f34ea503ac4 -r e1e25db76cdf src/os/unix/ngx_daemon.c --- a/src/os/unix/ngx_daemon.c Wed Oct 07 22:19:42 2015 +0300 +++ b/src/os/unix/ngx_daemon.c Tue Oct 13 23:56:12 2015 +0200 @@ -9,6 +9,20 @@ ?#include <ngx_core.h> ? ? +static ngx_int_t +ngx_close_handle(ngx_log_t *log, int fd) +{ +????if (fd > STDERR_FILENO) { +????????if (close(fd) == -1) { +????????????ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "close() failed"); +????????????return NGX_ERROR; +????????} +????} + +????return NGX_OK; +} + + ?ngx_int_t ?ngx_daemon(ngx_log_t *log) ?{ @@ -44,26 +58,26 @@ ? ?????if (dup2(fd, STDIN_FILENO) == -1) { ?????????ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "dup2(STDIN) failed"); +????????ngx_close_handle(log, fd); ?????????return NGX_ERROR; ?????} ? ?????if (dup2(fd, STDOUT_FILENO) == -1) { ?????????ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "dup2(STDOUT) failed"); +????????ngx_close_handle(log, fd); ?????????return NGX_ERROR; ?????} ? ?#if 0 ?????if (dup2(fd, STDERR_FILENO) == -1) { ?????????ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "dup2(STDERR) failed"); +????????ngx_close_handle(log, fd); ?????????return NGX_ERROR; ?????} ?#endif ? -????if (fd > STDERR_FILENO) { -????????if (close(fd) == -1) { -????????????ngx_log_error(NGX_LOG_EMERG, log, ngx_errno, "close() failed"); -????????????return NGX_ERROR; -????????} +????if (ngx_close_handle(log, fd) == NGX_ERROR) { +????????return NGX_ERROR; ?????} ? ?????return NGX_OK; -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20151013/26bee341/attachment-0001.html> ------------------------------ Message: 5 Date: Wed, 14 Oct 2015 13:38:18 +0300 From: Ruslan Ermilov <[email protected]> To: [email protected] Subject: Re: Resource leak on error paths in thread pools Message-ID: <[email protected]> Content-Type: text/plain; charset=us-ascii 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 [email protected] > # 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(). ------------------------------ Subject: Digest Footer _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel ------------------------------ End of nginx-devel Digest, Vol 72, Issue 10 *******************************************
_______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
