Welcome! Log In Create A New Profile

Advanced

help

Runfeng.Wang@netbraintech.com
October 14, 2015 09:14PM
unsubscribe maillist for me plz



Runfeng.Wang@netbraintech.com

From: nginx-devel-request@nginx.org
Date: 2015-10-14 20:00
To: nginx-devel@nginx.org
Subject: nginx-devel Digest, Vol 72, Issue 10
Send nginx-devel mailing list submissions to
nginx-devel@nginx.org

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
nginx-devel-request@nginx.org

You can reach the person managing the list at
nginx-devel-owner@nginx.org

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 <mdounin@mdounin.ru>
To: nginx-devel@nginx.org
Subject: Re: Patch for s390x support
Message-ID: <20151013124408.GY30105@mdounin.ru>
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 <neale@sinenomine.net>
To: "nginx-devel@nginx.org" <nginx-devel@nginx.org>
Subject: Re: Patch for s390x support
Message-ID: <D2427D0B.4508C%neale@sinenomine.net>
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"
<nginx-devel-bounces@nginx.org on behalf of mdounin@mdounin.ru> 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 <bartw@xs4all.nl>
To: nginx-devel@nginx.org
Subject: Resource leak on error paths in thread pools
Message-ID: <1444771305.1669.2.camel@xs4all.nl>
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 bartw@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;
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 <bartw@xs4all.nl>
To: nginx-devel@nginx.org
Subject: Error path does not close fd
Message-ID: <1444773484.19364.4.camel@xs4all.nl>
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 <bartw@xs4all.nl>
# 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 <ru@nginx.com>
To: nginx-devel@nginx.org
Subject: Re: Resource leak on error paths in thread pools
Message-ID: <20151014103818.GA75964@lo0.su>
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 bartw@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().



------------------------------

Subject: Digest Footer

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

------------------------------

End of nginx-devel Digest, Vol 72, Issue 10
*******************************************
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

help

Runfeng.Wang@netbraintech.com 443 October 14, 2015 09:14PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 243
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready