Re: mod_proxy_fcgi and flush

[Helmut K. C. Tessarek]

On 2017-07-07 17:50, Jacob Champion wrote:
> I'm probably not the person you're looking for; I don't know the 
> mod_proxy code very well. You may find that my root cause analysis is
> completely incorrect. But, if you'd like to bounce ideas off me, go
> for it. (As Luca already knows, I can be a little sporadic when it 
> comes to this sort of thing. Fair warning! :) )

I'm gonna have a look at the mod_proxy and mod_proxy_fcgi code. Who
wrote the code in the first place? Maybe I can bug that person with my
questions. ;-)
Let's see, maybe I'm totally out of my depth anyway. Sometimes it's very
hard to chime in, when you haven't been there from the beginning.

> #httpd-dev on Freenode is also a place -- very quiet, usually, but I 
> try to be there when I can.

Yea, not a fan of irc. I always find it very frustrating. Asking
something, waiting forever, not getting any answers. I'm more into IM.
I've noticed that people on IM are more responsive, since they really
seem to be online when the status says they are online.
Anyway, I don't think that real time chat is necessary, at least not
right away.


-- 
regards Helmut K. C. Tessarek
lookup https://sks-keyservers.net/i for KeyID 0xC11F128D

/*
   Thou shalt not follow the NULL pointer for chaos and madness
   await thee at its end.
*/

Re: mod_proxy_fcgi and flush

[Jacob Champion]

On 07/06/2017 08:25 PM, Helmut K. C. Tessarek wrote:

On 2017-07-06 14:54, Jacob Champion wrote:

It probably makes sense to work on a nonblocking architecture for
proxied responses in general.


I'm not familiar with that particular code, but would be interested in
looking into it. Does anybody volunteer as a mentor?


I'm probably not the person you're looking for; I don't know the 
mod_proxy code very well. You may find that my root cause analysis is 
completely incorrect. But, if you'd like to bounce ideas off me, go for 
it. (As Luca already knows, I can be a little sporadic when it comes to 
this sort of thing. Fair warning! :) )


#httpd-dev on Freenode is also a place -- very quiet, usually, but I try 
to be there when I can.


--Jacob

Re: httpd 2.4.26 with apr 1.6 ExtFilterDefine

[William A Rowe Jr]

On Fri, Jul 7, 2017 at 7:13 AM, Jim Jagielski  wrote:
> 2.4.26/27 doesn't *require* APR/APU 1.6.x, but there are
> some features that depend on it. If it's a bug in apr 1.6.x,
> then it's not a httpd bug specifically... imo at least.
>
> any further detail on how the below is actually borken??
> What happens?
>
>> On Jul 7, 2017, at 7:41 AM, Steffen  wrote:
>>
>>
>> I got the following report. Is this known ?
>>
>>
>> Because apache http server all of 2.4.26 (vc11,vc14,vc15)
>> was not work ExtFilterDefine & OutputFilter. Its bug
>> exists in apr 1.6. I thought it need to inform.
>>
>> Apache 2.4.26 changes apr 1.6, but it broke
>> ExtFilterDefine & OutputFilter.
>> (test) copy apache 2.4.25's libapr-1 & libaprutil to
>> apache 2.4.26, they worked correctly like before apaches.

Yes, we need the actual error messages.

I'm about ready to T apr 1.6.1 / apu 1.6.3 for the various fixes already
present, but if we can fix something specific that we are unaware of...

Re: [VOTE] Release httpd-2.2.34

[Jim Jagielski]

> On Jul 6, 2017, at 3:33 PM, William A Rowe Jr  wrote:
> 
> 
> +/-1
> [ ] Release 2.2.34 as legacy GA

+1: macOS 10.12.5

> 
> +/-1
> [ ] Retire the 2.2.x branch from any further maintenance.

+1

Re: [VOTE] Release httpd-2.2.34

[Gregg Smith]

On 7/6/2017 12:33 PM, William A Rowe Jr wrote:

Your votes on two decisions, please?


[+1] Release 2.2.34 as legacy GA
[+1] Retire the 2.2.x branch from any further maintenance.

Re: [VOTE] Release Apache httpd 2.4.27 as GA

[Stefan Eissing]

> Am 06.07.2017 um 19:45 schrieb Jim Jagielski :
> 
> The pre-release test tarballs for Apache httpd
> version 2.4.27 can be found at the usual place:
> 
>   http://httpd.apache.org/dev/dist/
> 
> I'm calling a VOTE on releasing these as Apache httpd 2.4.27 GA.
> 
> [ ] +1: Good to go
> [ ] +0: meh
> [ ] -1: Danger Will Robinson. And why.
> 
> Vote will last the normal 72 hrs.
> 
> NOTE: The *-deps are only there for convenience.
> 
> Thx!

+1:
 o Ubuntu 16.04.02, 64bit (with dep apr and 1.5.x apr)

Re: httpd 2.4.26 with apr 1.6 ExtFilterDefine

[Jim Jagielski]

2.4.26/27 doesn't *require* APR/APU 1.6.x, but there are
some features that depend on it. If it's a bug in apr 1.6.x,
then it's not a httpd bug specifically... imo at least.

any further detail on how the below is actually borken??
What happens?

> On Jul 7, 2017, at 7:41 AM, Steffen  wrote:
> 
> 
> I got the following report. Is this known ?
> 
> 
> Because apache http server all of 2.4.26 (vc11,vc14,vc15)
> was not work ExtFilterDefine & OutputFilter. Its bug
> exists in apr 1.6. I thought it need to inform.
> 
> Apache 2.4.26 changes apr 1.6, but it broke
> ExtFilterDefine & OutputFilter.
> (test) copy apache 2.4.25's libapr-1 & libaprutil to
> apache 2.4.26, they worked correctly like before apaches.
> 
> 

httpd 2.4.26 with apr 1.6 ExtFilterDefine

[Steffen]

I got the following report. Is this known ?


Because apache http server all of 2.4.26 (vc11,vc14,vc15)
was not work ExtFilterDefine & OutputFilter. Its bug
exists in apr 1.6. I thought it need to inform.

Apache 2.4.26 changes apr 1.6, but it broke
ExtFilterDefine & OutputFilter.
(test) copy apache 2.4.25's libapr-1 & libaprutil to
apache 2.4.26, they worked correctly like before apaches.

Re: [VOTE] Release Apache httpd 2.4.27 as GA

[Jim Jagielski]

+1:
  o CentOS 6.9, 64bit
  o CentOS 7.3, 64bit
  o Ubuntu 15.10, 64bit
> On Jul 6, 2017, at 1:45 PM, Jim Jagielski  wrote:
> 
> The pre-release test tarballs for Apache httpd
> version 2.4.27 can be found at the usual place:
> 
>   http://httpd.apache.org/dev/dist/
> 
> I'm calling a VOTE on releasing these as Apache httpd 2.4.27 GA.
> 
> [ ] +1: Good to go
> [ ] +0: meh
> [ ] -1: Danger Will Robinson. And why.
> 
> Vote will last the normal 72 hrs.
> 
> NOTE: The *-deps are only there for convenience.
> 
> Thx!

[RFC/PATCH] mpm_winnt: Fix several issues in the child process shutdown logic

[Evgeny Kotkov]

Hi all,

Recently we (Ivan Zhakov and I) found several issues in the code responsible
for shutting down a mpm_winnt's child process.

The attached patch should fix these issues:

 (Please note that the changes are combined into a single patch to make the
  review easier, but I'll commit them independently.)

 (1) As a part of the process of shutting down worker threads, the code
 around child.c:1170 currently posts an amount of I/O completion packets
 equal to the amount of the threads blocked on the I/O completion port.
 Then it sleeps until all these threads "acknowledge" the completion
 packets by decrementing the global amount of blocked threads.

 This can be improved to avoid unnecessary Sleep()'s, and make the
 shutdown faster.  There is no need to block until the threads actually
 receive the completion, as the shutdown process includes a separate step
 that waits until the threads exit.  Instead of synchronizing on the
 amount of the threads blocked on the I/O completion port, we can send
 the number of IOCP_SHUTDOWN completion packets equal to the total
 amount of threads and immediately proceed to the next step.

  (2) If the shutdown routine hits a timeout while waiting for the worker
  threads to exit, it uses TerminateThread() to terminate the remaining
  threads.

  Using TerminateThread() can have dangerous consequences such as
  deadlocks — say, if the the thread is terminated while holding a lock
  or a heap lock in the middle of HeapAlloc(), as these locks would not
  be released.  Or it can corrupt the application state and cause a crash.
  See https://msdn.microsoft.com/en-us/library/windows/desktop/ms686717

  Presumably, a much better alternative here would be to leave the cleanup
  to the operating system by calling TerminateProcess().

  (3) Assuming (2) is in place, the code around child.c:1170 that waits for
  multiple thread handles in batches can be simplified.  With (2), there
  is no difference between ending the wait with one or multiple remaining
  threads.  (Since we terminate the process if at least one thread is
  still active when we hit a timeout.)

  Therefore, instead of making an effort to evenly distribute and batch
  the handles with WaitForMultipleObjects(), we could just start from
  one end, and wait for one thread handle at a time.

Note that apart from this, the code around child.c:1146 that shuts down the
listeners, waits, and then proceeds to shutting down the worker threads is
prone to a subtle race.  Since the wait interval is hardcoded to 1 second,
there could still be an accepted connection after it.  And, as the worker
threads are being shut down, it is feasible that this accepted connection
won't ever find a suitable worker thread.  (Eventually, it is going to be
ungracefully closed).  I think that this can be fixed by properly waiting for
the listener threads to exit, and in the meanwhile, this would avoid having
the Sleep(1000) call altogether.  But this is something that I have now left
for future work.

I would greatly appreciate if someone could review or comment on the proposed
changes.  If anyone has an additional insight on the design or planned, but
unaccomplished changes to the mpm_winnt module, I would appreciate hearing
them as well.

Thanks!


Regards,
Evgeny Kotkov
Index: server/mpm/winnt/child.c
===
--- server/mpm/winnt/child.c(revision 1801135)
+++ server/mpm/winnt/child.c(working copy)
@@ -827,18 +827,6 @@ static DWORD __stdcall worker_main(void *thread_nu
 }
 
 
-static void cleanup_thread(HANDLE *handles, int *thread_cnt,
-   int thread_to_clean)
-{
-int i;
-
-CloseHandle(handles[thread_to_clean]);
-for (i = thread_to_clean; i < ((*thread_cnt) - 1); i++)
-handles[i] = handles[i + 1];
-(*thread_cnt)--;
-}
-
-
 /*
  * child_main()
  * Entry point for the main control thread for the child process.
@@ -890,7 +878,6 @@ void child_main(apr_pool_t *pconf, DWORD parent_pi
 HANDLE *child_handles;
 int listener_started = 0;
 int threads_created = 0;
-int watch_thread;
 int time_remains;
 int cld;
 DWORD tid;
@@ -1162,16 +1149,16 @@ void child_main(apr_pool_t *pconf, DWORD parent_pi
 /* Shutdown the worker threads
  * Post worker threads blocked on the ThreadDispatch IOCompletion port
  */
-while (g_blocked_threads > 0) {
+if (g_blocked_threads > 0) {
 ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, ap_server_conf, 
APLOGNO(00361)
  "Child: %d threads blocked on the completion port",
  g_blocked_threads);
-for (i=g_blocked_threads; i > 0; i--) {
-PostQueuedCompletionStatus(ThreadDispatchIOCP, 0,
-   IOCP_SHUTDOWN, NULL);
-}
-Sleep(1000);
 }
+
+