You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2014/06/09 03:03:39 UTC
svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/proxy_util.c
Author: ylavic
Date: Mon Jun 9 01:03:39 2014
New Revision: 1601291
URL: http://svn.apache.org/r1601291
Log:
mod_proxy: Shutdown (eg. SSL close notify) the backend connection
before closing.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/log-message-tags/next-number
httpd/httpd/trunk/modules/proxy/proxy_util.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1601291&r1=1601290&r2=1601291&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Mon Jun 9 01:03:39 2014
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_proxy: Shutdown (eg. close notify) the backend connection before
+ closing. [Yann Ylavic]
+
*) mpm_event[opt]: Send the SSL close notify alert when the KeepAliveTimeout
expires. PR54998. [Yann Ylavic]
Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1601291&r1=1601290&r2=1601291&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Mon Jun 9 01:03:39 2014
@@ -1 +1 @@
-2642
+2643
Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1601291&r1=1601290&r2=1601291&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Jun 9 01:03:39 2014
@@ -2815,6 +2815,33 @@ PROXY_DECLARE(int) ap_proxy_connect_back
return connected ? OK : DECLINED;
}
+static apr_status_t connection_shutdown(void *theconn)
+{
+ proxy_conn_rec *conn = (proxy_conn_rec *)theconn;
+ conn_rec *c = conn->connection;
+ if (c) {
+ if (!c->aborted) {
+ apr_interval_time_t saved_timeout = 0;
+ apr_socket_timeout_get(conn->sock, &saved_timeout);
+ if (saved_timeout) {
+ apr_socket_timeout_set(conn->sock, 0);
+ }
+
+ (void)ap_shutdown_conn(c, 0);
+ c->aborted = 1;
+
+ if (saved_timeout) {
+ apr_socket_timeout_set(conn->sock, saved_timeout);
+ }
+ }
+
+ ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(02642)
+ "proxy: connection shutdown");
+ }
+ return APR_SUCCESS;
+}
+
+
PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function,
proxy_conn_rec *conn,
conn_rec *c,
@@ -2887,6 +2914,12 @@ PROXY_DECLARE(int) ap_proxy_connection_c
}
apr_socket_timeout_set(conn->sock, current_timeout);
+ /* Shutdown the connection before closing it (eg. SSL connections
+ * need to be close-notify-ed).
+ */
+ apr_pool_cleanup_register(conn->scpool, conn, connection_shutdown,
+ apr_pool_cleanup_null);
+
return OK;
}
Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
Fixed in r1601630.
On Tue, Jun 10, 2014 at 3:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Jun 10, 2014 at 3:02 PM, Plüm, Rüdiger, Vodafone Group
> <ru...@vodafone.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Takashi Sato [mailto:takashi@tks.st]
>>> Sent: Dienstag, 10. Juni 2014 14:47
>>> To: dev@httpd.apache.org
>>> Subject: Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
>>> docs/log-message-tags/next-number modules/proxy/proxy_util.c
>>>
>>> r1601291 causes SEGV.
>>>
>>> # Failed test 2 in t/ssl/proxy.t at line 56
>>> # Failed test 115 in t/ssl/proxy.t at line 56 fail #3
>>> [ error] oh gosh, server dumped core
>>> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
>>> -core /home/st/c99fix/httpd-test/t/core.3649
>>> [ error] oh nuts, server dumped core again
>>> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
>>> -core /home/st/c99fix/httpd-test/t/core.3647
>>> t/ssl/proxy.t .......................
>>> Failed 2/172 subtests
>>>
>>> The new function connection_shutdown is a pool cleanup func.
>>> connection_shutdown calls ap_shutdown_conn,
>>> but ap_shutdown_conn finally calls apr_pool_clear!
>>> (see attached strack trace)
>>
>> Hm, very bad. Actually we are trying to clean a subpool of the connection pool in setaside_remaining_output which
>> has been already destroyed at this point. So I guess we need to have to use apr_pool_pre_cleanup_register instead of
>> apr_pool_cleanup_register for the connection_shutdown.
>
> I'm testing this, will commit when sure it works.
>
> Regards,
> Yann.
Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 10, 2014 at 3:02 PM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
>
>> -----Original Message-----
>> From: Takashi Sato [mailto:takashi@tks.st]
>> Sent: Dienstag, 10. Juni 2014 14:47
>> To: dev@httpd.apache.org
>> Subject: Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
>> docs/log-message-tags/next-number modules/proxy/proxy_util.c
>>
>> r1601291 causes SEGV.
>>
>> # Failed test 2 in t/ssl/proxy.t at line 56
>> # Failed test 115 in t/ssl/proxy.t at line 56 fail #3
>> [ error] oh gosh, server dumped core
>> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
>> -core /home/st/c99fix/httpd-test/t/core.3649
>> [ error] oh nuts, server dumped core again
>> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
>> -core /home/st/c99fix/httpd-test/t/core.3647
>> t/ssl/proxy.t .......................
>> Failed 2/172 subtests
>>
>> The new function connection_shutdown is a pool cleanup func.
>> connection_shutdown calls ap_shutdown_conn,
>> but ap_shutdown_conn finally calls apr_pool_clear!
>> (see attached strack trace)
>
> Hm, very bad. Actually we are trying to clean a subpool of the connection pool in setaside_remaining_output which
> has been already destroyed at this point. So I guess we need to have to use apr_pool_pre_cleanup_register instead of
> apr_pool_cleanup_register for the connection_shutdown.
I'm testing this, will commit when sure it works.
Regards,
Yann.
RE: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/proxy_util.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Original Message-----
> From: Takashi Sato [mailto:takashi@tks.st]
> Sent: Dienstag, 10. Juni 2014 14:47
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
> docs/log-message-tags/next-number modules/proxy/proxy_util.c
>
> r1601291 causes SEGV.
>
> # Failed test 2 in t/ssl/proxy.t at line 56
> # Failed test 115 in t/ssl/proxy.t at line 56 fail #3
> [ error] oh gosh, server dumped core
> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
> -core /home/st/c99fix/httpd-test/t/core.3649
> [ error] oh nuts, server dumped core again
> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
> -core /home/st/c99fix/httpd-test/t/core.3647
> t/ssl/proxy.t .......................
> Failed 2/172 subtests
>
> The new function connection_shutdown is a pool cleanup func.
> connection_shutdown calls ap_shutdown_conn,
> but ap_shutdown_conn finally calls apr_pool_clear!
> (see attached strack trace)
Hm, very bad. Actually we are trying to clean a subpool of the connection pool in setaside_remaining_output which
has been already destroyed at this point. So I guess we need to have to use apr_pool_pre_cleanup_register instead of
apr_pool_cleanup_register for the connection_shutdown.
Regards
Rüdiger
Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Jun 10, 2014 at 2:46 PM, Takashi Sato <ta...@tks.st> wrote:
> r1601291 causes SEGV.
>
> # Failed test 2 in t/ssl/proxy.t at line 56
> # Failed test 115 in t/ssl/proxy.t at line 56 fail #3
> [ error] oh gosh, server dumped core
> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
> -core /home/st/c99fix/httpd-test/t/core.3649
> [ error] oh nuts, server dumped core again
> [ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
> -core /home/st/c99fix/httpd-test/t/core.3647
> t/ssl/proxy.t .......................
> Failed 2/172 subtests
>
> The new function connection_shutdown is a pool cleanup func.
> connection_shutdown calls ap_shutdown_conn,
> but ap_shutdown_conn finally calls apr_pool_clear!
> (see attached strack trace)
Ouch, thanks, deferred_write_pool has to be alive...
Will fix with :
Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c (revision 1601598)
+++ modules/proxy/proxy_util.c (working copy)
@@ -2917,8 +2917,7 @@ PROXY_DECLARE(int) ap_proxy_connection_create(cons
/* Shutdown the connection before closing it (eg. SSL connections
* need to be close-notify-ed).
*/
- apr_pool_cleanup_register(conn->scpool, conn, connection_shutdown,
- apr_pool_cleanup_null);
+ apr_pool_pre_cleanup_register(conn->scpool, conn, connection_shutdown);
return OK;
}
[END]
which was my original version...
Re: svn commit: r1601291 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/proxy_util.c
Posted by Takashi Sato <ta...@tks.st>.
r1601291 causes SEGV.
# Failed test 2 in t/ssl/proxy.t at line 56
# Failed test 115 in t/ssl/proxy.t at line 56 fail #3
[ error] oh gosh, server dumped core
[ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
-core /home/st/c99fix/httpd-test/t/core.3649
[ error] oh nuts, server dumped core again
[ error] for stacktrace, run: gdb /home/st/c99fix/apache/bin/httpd
-core /home/st/c99fix/httpd-test/t/core.3647
t/ssl/proxy.t .......................
Failed 2/172 subtests
The new function connection_shutdown is a pool cleanup func.
connection_shutdown calls ap_shutdown_conn,
but ap_shutdown_conn finally calls apr_pool_clear!
(see attached strack trace)