Re: listener thread not exiting cleanly on OSX on trunk

2018-01-10 Thread Jim Jagielski 
Let me take a look... I haven't built trunk for 1-2 weeks

What version of OSX and Xcode are you using?

> On Jan 10, 2018, at 2:13 PM, Eric Covener  wrote:
> 
> I'm new to OSX. For my httpd sandbox, I notice "apachectl stop" has to
> forcibly kill children.  When I caught one with pstack, the listener
> thread was the only thing running (other than the main thread trying
> to join_workers)
> 
> 
>  thread #2
>frame #0: 0x7fffc2baf0c2 libsystem_kernel.dylib`__accept + 10
>frame #1: 0x00010a4c3e8c
> libapr-1.0.dylib`apr_socket_accept(new=0x75116e50,
> sock=0x7fc03e01ad50, connection_context=0x7fc03e053c28) at
> sockets.c:271 [opt]
>frame #2: 0x00010a35a607
> httpd`ap_unixd_accept(accepted=0x75116ea0,
> lr=0x7fc03e01ad08, ptrans=) at unixd.c:302 [opt]
>frame #3: 0x00010a7bd349
> mod_mpm_worker.so`listener_thread(thd=0x7fc0400051d0,
> dummy=) at worker.c:693 [opt]
>frame #4: 0x7fffc2c9a93b libsystem_pthread.dylib`_pthread_body + 180
>frame #5: 0x7fffc2c9a887 libsystem_pthread.dylib`_pthread_start + 286
>frame #6: 0x7fffc2c9a08d libsystem_pthread.dylib`thread_start + 13
> 
> It happens with both 1 or multiple listening sockets.  It doesn't
> happen in my 2.4 sandbox.
> 
> Does anyone else see this (or not?) . The obvious symptom is:
> [Wed Jan 10 19:11:21.204862 2018] [core:warn] [pid 68986:tid
> 140736609248192] AH00045: child process 68988 still did not exit,
> sending a SIGTERM
> 
> -- 
> Eric Covener
> cove...@gmail.com

listener thread not exiting cleanly on OSX on trunk

2018-01-10 Thread Eric Covener 
I'm new to OSX. For my httpd sandbox, I notice "apachectl stop" has to
forcibly kill children.  When I caught one with pstack, the listener
thread was the only thing running (other than the main thread trying
to join_workers)


  thread #2
frame #0: 0x7fffc2baf0c2 libsystem_kernel.dylib`__accept + 10
frame #1: 0x00010a4c3e8c
libapr-1.0.dylib`apr_socket_accept(new=0x75116e50,
sock=0x7fc03e01ad50, connection_context=0x7fc03e053c28) at
sockets.c:271 [opt]
frame #2: 0x00010a35a607
httpd`ap_unixd_accept(accepted=0x75116ea0,
lr=0x7fc03e01ad08, ptrans=) at unixd.c:302 [opt]
frame #3: 0x00010a7bd349
mod_mpm_worker.so`listener_thread(thd=0x7fc0400051d0,
dummy=) at worker.c:693 [opt]
frame #4: 0x7fffc2c9a93b libsystem_pthread.dylib`_pthread_body + 180
frame #5: 0x7fffc2c9a887 libsystem_pthread.dylib`_pthread_start + 286
frame #6: 0x7fffc2c9a08d libsystem_pthread.dylib`thread_start + 13

It happens with both 1 or multiple listening sockets.  It doesn't
happen in my 2.4 sandbox.

Does anyone else see this (or not?) . The obvious symptom is:
[Wed Jan 10 19:11:21.204862 2018] [core:warn] [pid 68986:tid
140736609248192] AH00045: child process 68988 still did not exit,
sending a SIGTERM

-- 
Eric Covener
cove...@gmail.com

httpd 2.2 does not build without APR_HAS_THREADS

2018-01-10 Thread Stefan Sperling 
r1750836 broke the httpd 2.2 build if APR_HAS_THREADS is not defined.

I suppose this won't be fixed because 2.2 is EOL.
I just wanted to mention it in case somebody cares.

modules/proxy/proxy_util.c:1705: undefined reference to `socket_cleanup'
modules/proxy/.libs/libmod_proxy.a(proxy_util.o): In function 
`ap_proxy_ssl_connection_cleanup':
modules/proxy/proxy_util.c:1743: undefined reference to `socket_cleanup'
modules/proxy/.libs/libmod_proxy.a(proxy_util.o): In function 
`ap_proxy_determine_connection':
modules/proxy/proxy_util.c:2235: undefined reference to `socket_cleanup'
modules/proxy/proxy_util.c:2281: undefined reference to `socket_cleanup'
modules/proxy/proxy_util.c:2337: undefined reference to `socket_cleanup'
modules/proxy/.libs/libmod_proxy.a(proxy_util.o):modules/proxy/proxy_util.c:2542:
 more undefined references to `socket_cleanup' follow



r1750836 | wrowe | 2016-06-30 19:07:29 +0200 (Thu, 30 Jun 2016) | 8 lines

mod_proxy: don't recyle backend announced "Connection: close" connections
to avoid reusing it should the close be effective after some new request
is ready to be sent.

Backports: r1678763, r1703807, r1703813, r1678763
Submitted by: ylavic
Reviewed by: rpluem, wrowe


Index: httpd/httpd/branches/2.2.x/STATUS
===
--- httpd/httpd/branches/2.2.x/STATUS   (revision 1750835)
+++ httpd/httpd/branches/2.2.x/STATUS   (revision 1750836)
@@ -138,18 +138,7 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
  2.2.x patch: trunk works, modulo CHANGES
  +1: rjung, wrowe, ylavic
 
-  *) mod_proxy: don't recyle backend announced "Connection: close" connections
- to avoid reusing it should the close be effective after some new request
- is ready to be sent.
- trunk patch: http://svn.apache.org/r1678763
-  http://svn.apache.org/r1703807
-  http://svn.apache.org/r1703813
- 2.2.x patch: 
http://home.apache.org/~ylavic/patches/httpd-2.2.x-mod_proxy-connection_close.patch
- +1: ylavic, rpluem, wrowe
- ylavic: while at it, I also included r1678763 which is only an
- optimization, but allows to keep code in sync with 2.4/trunk.
 
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
 
Index: httpd/httpd/branches/2.2.x/CHANGES
===
--- httpd/httpd/branches/2.2.x/CHANGES  (revision 1750835)
+++ httpd/httpd/branches/2.2.x/CHANGES  (revision 1750836)
@@ -1,6 +1,10 @@
  -*- coding: utf-8 -*-
 Changes with Apache 2.2.32
 
+  *) mod_proxy: don't recyle backend announced "Connection: close" connections
+ to avoid reusing it should the close be effective after some new request
+ is ready to be sent.  [Yann Ylavic]
+
   *) mod_substitute: Allow to configure the patterns merge order with the new
  SubstituteInheritBefore on|off directive.  PR 57641
  [Marc.Stern , Yann Ylavic, William Rowe]
Index: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
===
--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c   (revision 
1750835)
+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c   (revision 
1750836)
@@ -1399,6 +1399,14 @@ PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(
 }
 
 #if APR_HAS_THREADS
+static void socket_cleanup(proxy_conn_rec *conn)
+{
+conn->sock = NULL;
+conn->connection = NULL;
+conn->ssl_hostname = NULL;
+apr_pool_clear(conn->scpool);
+}
+
 static apr_status_t conn_pool_cleanup(void *theworker)
 {
 proxy_worker *worker = (proxy_worker *)theworker;
@@ -1681,7 +1689,8 @@ static apr_status_t connection_cleanup(void *theco
 #endif
 
 /* determine if the connection need to be closed */
-if (!ap_proxy_connection_reusable(conn)) {
+if (!worker->is_address_reusable || worker->disablereuse
+|| conn->close_on_recycle) {
 apr_pool_t *p = conn->pool;
 apr_pool_clear(p);
 conn = apr_pcalloc(p, sizeof(proxy_conn_rec));
@@ -1690,6 +1699,12 @@ static apr_status_t connection_cleanup(void *theco
 apr_pool_create(&(conn->scpool), p);
 apr_pool_tag(conn->scpool, "proxy_conn_scpool");
 }
+else if (conn->close
+|| (conn->connection
+&& conn->connection->keepalive == AP_CONN_CLOSE)) {
+socket_cleanup(conn);
+conn->close = 0;
+}
 #if APR_HAS_THREADS
 if (worker->hmax && worker->cp->res) {
 conn->inreslist = 1;
@@ -1705,14 +1720,6 @@ static apr_status_t connection_cleanup(void *theco
 return APR_SUCCESS;
 }
 
-static void socket_cleanup(proxy_conn_rec *conn)
-{
-conn->sock = NULL;
-conn->connection = NULL;
-conn->ssl_hostname = NULL;
-apr_pool_clear(conn->scpool);
-}
-

Re: svn commit: r1820715 - in /httpd/httpd/trunk: CHANGES modules/metadata/mod_headers.c

2018-01-10 Thread Yann Ylavic 
On Wed, Jan 10, 2018 at 2:48 PM, Eric Covener  wrote:
> On Wed, Jan 10, 2018 at 8:27 AM, Eric Covener  wrote:
>> On Wed, Jan 10, 2018 at 7:13 AM, Yann Ylavic  wrote:
>>> On Wed, Jan 10, 2018 at 1:52 AM,   wrote:
 Author: covener
 Date: Wed Jan 10 00:52:25 2018
 New Revision: 1820715

 URL: http://svn.apache.org/viewvc?rev=1820715=rev
 Log:
 PR 61983: "Header unset Content-Type" doesn't work


 Submitted By: Hank Ibell 
 Committed By: covener
>>> []

 Modified: httpd/httpd/trunk/modules/metadata/mod_headers.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_headers.c?rev=1820715=1820714=1820715=diff
 ==
 --- httpd/httpd/trunk/modules/metadata/mod_headers.c (original)
 +++ httpd/httpd/trunk/modules/metadata/mod_headers.c Wed Jan 10 00:52:25 
 2018
 @@ -806,6 +806,9 @@ static int do_headers_fixup(request_rec
  break;
  case hdr_unset:
  apr_table_unset(headers, hdr->header);
 +if (!ap_cstr_casecmp(hdr->header, "Content-Type")) {
 +ap_set_content_type(r, NULL);
 +}
>>>
>>> Don't we need to also check for "headers != r->headers_in &&
>>> !ap_cstr_casecmp(...)" here?
>>
>> I see what you mean, this would be in fixups (after type checker) and
>> we could clobber the outbound side. Thanks.
>
> Hoping I followed in 1820750, thanks again!

Looks good, I didn't figure out we were doing this at several places.
Thanks Eric.

Re: svn commit: r1820715 - in /httpd/httpd/trunk: CHANGES modules/metadata/mod_headers.c

2018-01-10 Thread Eric Covener 
On Wed, Jan 10, 2018 at 8:27 AM, Eric Covener  wrote:
> On Wed, Jan 10, 2018 at 7:13 AM, Yann Ylavic  wrote:
>> On Wed, Jan 10, 2018 at 1:52 AM,   wrote:
>>> Author: covener
>>> Date: Wed Jan 10 00:52:25 2018
>>> New Revision: 1820715
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1820715=rev
>>> Log:
>>> PR 61983: "Header unset Content-Type" doesn't work
>>>
>>>
>>> Submitted By: Hank Ibell 
>>> Committed By: covener
>> []
>>>
>>> Modified: httpd/httpd/trunk/modules/metadata/mod_headers.c
>>> URL: 
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_headers.c?rev=1820715=1820714=1820715=diff
>>> ==
>>> --- httpd/httpd/trunk/modules/metadata/mod_headers.c (original)
>>> +++ httpd/httpd/trunk/modules/metadata/mod_headers.c Wed Jan 10 00:52:25 
>>> 2018
>>> @@ -806,6 +806,9 @@ static int do_headers_fixup(request_rec
>>>  break;
>>>  case hdr_unset:
>>>  apr_table_unset(headers, hdr->header);
>>> +if (!ap_cstr_casecmp(hdr->header, "Content-Type")) {
>>> +ap_set_content_type(r, NULL);
>>> +}
>>
>> Don't we need to also check for "headers != r->headers_in &&
>> !ap_cstr_casecmp(...)" here?
>
> I see what you mean, this would be in fixups (after type checker) and
> we could clobber the outbound side. Thanks.

Hoping I followed in 1820750, thanks again!

Re: svn commit: r1820715 - in /httpd/httpd/trunk: CHANGES modules/metadata/mod_headers.c

2018-01-10 Thread Eric Covener 
On Wed, Jan 10, 2018 at 7:13 AM, Yann Ylavic  wrote:
> On Wed, Jan 10, 2018 at 1:52 AM,   wrote:
>> Author: covener
>> Date: Wed Jan 10 00:52:25 2018
>> New Revision: 1820715
>>
>> URL: http://svn.apache.org/viewvc?rev=1820715=rev
>> Log:
>> PR 61983: "Header unset Content-Type" doesn't work
>>
>>
>> Submitted By: Hank Ibell 
>> Committed By: covener
> []
>>
>> Modified: httpd/httpd/trunk/modules/metadata/mod_headers.c
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_headers.c?rev=1820715=1820714=1820715=diff
>> ==
>> --- httpd/httpd/trunk/modules/metadata/mod_headers.c (original)
>> +++ httpd/httpd/trunk/modules/metadata/mod_headers.c Wed Jan 10 00:52:25 2018
>> @@ -806,6 +806,9 @@ static int do_headers_fixup(request_rec
>>  break;
>>  case hdr_unset:
>>  apr_table_unset(headers, hdr->header);
>> +if (!ap_cstr_casecmp(hdr->header, "Content-Type")) {
>> +ap_set_content_type(r, NULL);
>> +}
>
> Don't we need to also check for "headers != r->headers_in &&
> !ap_cstr_casecmp(...)" here?

I see what you mean, this would be in fixups (after type checker) and
we could clobber the outbound side. Thanks.

Re: svn commit: r1820715 - in /httpd/httpd/trunk: CHANGES modules/metadata/mod_headers.c

2018-01-10 Thread Yann Ylavic 
On Wed, Jan 10, 2018 at 1:52 AM,   wrote:
> Author: covener
> Date: Wed Jan 10 00:52:25 2018
> New Revision: 1820715
>
> URL: http://svn.apache.org/viewvc?rev=1820715=rev
> Log:
> PR 61983: "Header unset Content-Type" doesn't work
>
>
> Submitted By: Hank Ibell 
> Committed By: covener
[]
>
> Modified: httpd/httpd/trunk/modules/metadata/mod_headers.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_headers.c?rev=1820715=1820714=1820715=diff
> ==
> --- httpd/httpd/trunk/modules/metadata/mod_headers.c (original)
> +++ httpd/httpd/trunk/modules/metadata/mod_headers.c Wed Jan 10 00:52:25 2018
> @@ -806,6 +806,9 @@ static int do_headers_fixup(request_rec
>  break;
>  case hdr_unset:
>  apr_table_unset(headers, hdr->header);
> +if (!ap_cstr_casecmp(hdr->header, "Content-Type")) {
> +ap_set_content_type(r, NULL);
> +}

Don't we need to also check for "headers != r->headers_in &&
!ap_cstr_casecmp(...)" here?

>  break;
>  case hdr_echo:
>  v.r = r;