Re: apr_proc_other_child_read examined in depth

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Continuing with my example of

/**
 * Check on the specified process.  If it is gone, call the maintenance 
 * function.
 * @param pid The process to check.
 * @param status The status to pass to the maintenance function.
 */
APR_DECLARE(apr_status_t) apr_proc_other_child_read(apr_proc_t *pid, int status);

we have this code on Unix...

APR_DECLARE(apr_status_t) apr_proc_other_child_read(apr_proc_t *pid, int status)
{
    apr_other_child_rec_t *ocr, *nocr;

    for (ocr = other_children; ocr; ocr = nocr) {
        nocr = ocr->next;
        if (ocr->proc->pid != pid->pid)
            continue;

        ocr->proc = NULL;
        (*ocr->maintenance) (APR_OC_REASON_DEATH, ocr->data, status);
        return 0;
    }
    return APR_CHILD_NOTDONE;
} 

So let's look at the use case (chopped down from the prefork MPM);

int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
{
[...]
    restart_pending = shutdown_pending = 0;

    while (!restart_pending && !shutdown_pending) {
	apr_exit_why_e exitwhy;
	int status, processed_status;
	apr_proc_t pid;

	ap_wait_or_timeout(&exitwhy, &status, &pid, pconf);

	if (pid.pid != -1) {
	    processed_status = ap_process_child_status(&pid, exitwhy, status);
	    if (processed_status == APEXIT_CHILDFATAL) {
                return 1;
	    }

	    /* non-fatal death... note that it's gone in the scoreboard. */
	    child_slot = find_child_by_pid(&pid);
	    if (child_slot >= 0) {
	        [...]
#if APR_HAS_OTHER_CHILD
	    }
	    else if (apr_proc_other_child_read(&pid, status) == 0) {
		/* handled */
#endif
	    }
              [...]
	    continue;
	[...]
	}

	perform_idle_server_maintenance(pconf);
	[...]
    }
}

Now how is apr_proc_other_child_read related to its description above?  There is no check
for the current health of the process.  apr_proc_other_child_read appears to really be
apr_proc_other_child_died(), a function we can't support on win32.  Why not?  Because we
don't have 'child processes' that raise exceptions.  Yes - we can have them raise an event,
add them to a poll that signals when the process exits (if we have the apr_proc_t we created
when we spawned the child - that apr_proc_t contains a handle that can be sampled.)

Bill

Re: apr_proc_other_child_read examined in depth

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

At 12:00 AM 1/29/2003, Justin Erenkrantz wrote:
>--On Tuesday, January 28, 2003 11:48 PM -0600 "William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:
>
>>-1 on 0.9.2 till this issue is resolved.
>
>I don't really see why we should hold off on 0.9.2 because the current API is broken.  We're not saying it is 1.0.

But it is worthless until we define an OC that someone can use to develop
apps (not especially httpd, but that is a good case study.)  OC is a fairly
reasonable need within a subset of service-type, robust applications.

>Or, do you plan on holding httpd 2.0.45 back as well?  If you still plan to have 2.0.45 out soon and your offer to co-RM 0.9.2 at the same time holds, then waiting is fine with me.  

My plan is to tag 0.9.2.  Roll it.  Tag 2.0.45 (using 0.9.2) and roll the httpd 
release.  I think that would keep everyone happy.

>Otherwise, we should look at getting someone else (Garrett?) to RM.  =)

Well another APR committer needs to step up, but I think we all benefit
by dealing with this now, even if the release takes a few extra days.

>And, are you going to be able to fix this in a way that doesn't blow up backwards compatibility with httpd's 2.0.42+? 

Damned straight.

Sure, if an old module used OC they might have unexpected results (just
like 2.0.44's access log failures were unexpected.)  I expect we will rename
and deprecate some function names and add some new accessors to make
this really portable.  But existing code won't be 'broken', it will just function
as badly as it did before :-)

Bill

Re: apr_proc_other_child_read examined in depth

[Justin Erenkrantz <je...@apache.org>]

--On Tuesday, January 28, 2003 11:48 PM -0600 "William A. Rowe, Jr." 
<wr...@rowe-clan.net> wrote:

> Just more observations, another pair of eyeballs might come in
> handy.

I can't say I know much about OC logic, and I don't really have time 
to investigate this, but...

> -1 on 0.9.2 till this issue is resolved.

I don't really see why we should hold off on 0.9.2 because the 
current API is broken.  We're not saying it is 1.0.

Or, do you plan on holding httpd 2.0.45 back as well?  If you still 
plan to have 2.0.45 out soon and your offer to co-RM 0.9.2 at the 
same time holds, then waiting is fine with me.  Otherwise, we should 
look at getting someone else (Garrett?) to RM.  =)

And, are you going to be able to fix this in a way that doesn't blow 
up backwards compatibility with httpd's 2.0.42+?  -- justin

Re: apr_proc_other_child_read examined in depth

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

At 02:28 PM 1/28/2003, William A. Rowe, Jr. wrote:
>Now how is apr_proc_other_child_read related to its description above?  There is no check
>for the current health of the process.  apr_proc_other_child_read appears to really be
>apr_proc_other_child_died(), a function we can't support on win32.  Why not?  Because we
>don't have 'child processes' that raise exceptions.  Yes - we can have them raise an event,
>add them to a poll that signals when the process exits (if we have the apr_proc_t we created
>when we spawned the child - that apr_proc_t contains a handle that can be sampled.)

I'm really liking this basic idea... some sort of APR construct to poll processes.
On win32 that would be a WaitForMultipleObjects (possibly, several waits within
several cascading threads because Waitxxx APIs have a limit of 64 objects) and
on Unix that would be the basic signal handling.

Still no concrete solution, since this isn't trivial to design.  Just wanted to update
the status.

BTW - there are two different write exceptions; EWOULDBLOCK/EAGAIN and then
any other really nasty errors from write-to-pipe.  I believe we need a notification fn
after apr_write-to-O_C-pipe that would deal with the non-pipe-full conditions.  If the
pipe handle is *really* broken we should deal with that case.

However; that still isn't enough.  Because the pipe never really breaks.  We keep
the child handle around in the parent so we can respawn the other child.  Without
that handle sibling children wouldn't share the parent side of the pipe.  So it is all
really a catch 22.

Just more observations, another pair of eyeballs might come in handy.

-1 on 0.9.2 till this issue is resolved.

Bill