httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <>
Subject MPM design abuse (was: cvs commit: httpd-2.0/server/mpm/prefork prefork.c)
Date Tue, 13 Nov 2001 10:00:55 GMT
On Tue, Nov 13, 2001 at 07:15:36AM -0000, wrote:
> rbb         01/11/12 23:15:36
>   Modified:    include  http_connection.h
>                server   connection.c core.c
>                server/mpm/prefork prefork.c
>   Log:
>   This allows modules to add socket descriptors to the pollset.  I have
>   only added this to the perfork MPM, and the others work without it.
>   Tomorrow I will add it to the other MPMs.
>   --- core.c	2001/11/13 02:09:07	1.91
>   +++ core.c	2001/11/13 07:15:36	1.92
>   @@ -3329,6 +3329,19 @@
>        return net->c;
>    }
>   +static int core_add_listeners(apr_pollfd_t *pollset, 
>   +                              apr_socket_t **listensocks, int num_listensocks)
>   +{
>   +    int i;
>   +    ap_listen_rec *lr;
>   +
>   +    for (lr = ap_listeners, i = 0; i < num_listensocks; lr = lr->next, i++)
>   +        listensocks[i] = lr->sd;
>   +        apr_poll_socket_add(pollset, listensocks[i], APR_POLLIN);
>   +    }
>   +    return OK;
>   +}

I think this is *very* wrong, and I'm even tempted to -1 the darn thing. I
think we need a bit more discussion before continuing here.

The problem is that now an MPM must implement ap_listeners for use by
core.c. The MPM must also use a pollset.

That is NOT right. The MPM is the thing that defines how sockets are to be
set up, listened to, and accepted. It then defines the mapping from those
sockets to backend workers.

Think about the above code: the MPM sets up ap_listeners, and then it calls
an external function to process them, to put them into a pollset that it
created. It's like a damned little GOSUB to get some of its own work done.
The core is not adding any value here. It grabs some data from the MPM (the
ap_listeners variable) and puts it right back into an MPM structure (the
passed pollset variable).

And the interface is just whacked anyhow. The ap_listeners is a global, but
the size of it is a passed parameter. If you want to do this right, then
ap_listeners should not be a global (we've got a serious over-reliance on
globals!), but should be a passed parameter. Yet if that is done, then
again: the hook is not adding any value. The MPM passes everything it needs;
the hook just runs a loop for the MPM. Kinda silly...

And with this structure, how is an MPM supposed to set up different sets of
listeners for different threads? Oh... sorry, but the MPM can't define that
any more. I think with a bunch of monkey business, the MPM might be able to
define different sets for child processes. But the MPM is going to be
working *around* the issue when it should be *in charge*.

This change violates the very notion of an MPM being the piece of code
responsible for socket and worker handling.

I like the code changes for reducing the socket usage from Apache proper.
But removing it from the MPM isn't right.

Please explain :-(


Greg Stein,

View raw message