httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject Re: cvs commit: httpd-2.0/server/mpm/worker worker.c
Date Mon, 28 Jan 2002 01:35:45 GMT

> On Sun, 27 Jan 2002, Bill Stoddard wrote:
>
> > > 1)  Do exactly what I told you to do when I committed the patch
> > > originally.  Let's take an example of SSL.  Your SSL vhost must know
> > > which socket it is on, because it can't be on a plain HTTP socket.  So,
> > > for that socket, you replace the accept_function, and that allows you to
> > > use your own create_conn hook to create the connection and insert your
> > > own filters for the bottom of the stack.  In fact, regardless of what
> > > you are configuring, if you are creating a vhost that doesn't speak
> > > plain HTTP, you must know which socket you are listening on, and you can
> > > do exactly what you want.
> > >
> > An SSL socket is a normal BSD socket for the purposed of accept(). No distinction
is
made,
> > so this suggestion isn't sufficient to solve the problem.
>
> This suggestion can solve the problem, but you would need to add
> information to the ssl_Accept function. Yes, under the covers this would
> look very much like unixd_accept, but it would allow your
> create_connection function to do the right thing.
>
> > > 2)  Ignore this patch, and do what you would do if it wasn't there,
> > > which I presume is to add another filter above the CORE_IN and CORE
> > > filters so that they are never called.  That will work just fine with
> > > this patch in place.
> >
> > Partially true. I was adding the SSL_IN/SSL_OUT filters in pre_connection which
no
longer
> > has access to the socket because it is hidden away in the core_net_rec structure.
Relying
> > on the hack is not a cool way to do this. There were some other things broken with
this as
> > well that I don't have time to get into.  Also, adding core in/out filters in the
> > pre_connection hook is just wrong. I have a suggestion to fix this.
>
> I would prefer to know what else was broken with this.  You are correct
> that you didn't have access to the socket, you shouldn't.
> That was the whole point of the change.

I agree

> If you are using the CORE/CORE_IN filters,
> then you don't need access to the socket.  If you aren't using the
> CORE/CORE_IN filters, then you should also take control of the accept,
> because they are all tied together.

Ryan, that last argument is pedantic and I completely disagree with it. If a module author
is satisfied with the accept logic the MPM proivdes, there is no reason to make the module
author duplicate it in an accept function. Sure, in the documentation, advise against it
on principle, but don't force unnecessary work on module authors just to satisfy pedantic
urges.

Can we compromise? My recommendation captures 95% of the essence of what you are trying to
accomplish yet still provides a bit of additional freedom to module authors. It is MUCH
closer to what you want that what we have now. In fact, I would argue that it is by some
accounts better than your original code (its more intuitive that only one network i/o
filter should ever be installed and it provides the same degree of hiding of the client
socket).

Bill

>
> Ryan
>
> >
> > I think this suggestion should make everyone happy:
> > Leave ap_new_connection() in place, do NOT pass in the csd though (and remove
> > client_socket from conn_rec). Create a new hook that runs immediately before the
> > pre_connection hook (and after ap_update_vhost_given_ip(), I think that is the
> > function...). This new hook is a RUN_FIRST hook that installs the desired network
io
> > filter (CORE_IN, CORE_OUT, SSL_IN, SSL_OUT whatever). Need to pass this hook the
csd
and
> > conn_rec. The difference between this new hook and pre_connection hook is that the
new
> > hook is a RUN_FIRST hook. This will guarantee that one and only one network i/o
filter
> > will ever get installed. The new hook allocates and inits the core_net_rec to maintain
the
> > client socket. This is -very- close to your original patch.
> >
> > Could name the new hook install_networkio or something like that. If you like this
idea, I
> > will implement it tonight.
>
>
>
> _______________________________________________________________________________
> Ryan Bloom                        rbb@apache.org
> 550 Jean St
> Oakland CA 94610
> -------------------------------------------------------------------------------
>
>


Mime
View raw message