You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2001/07/13 22:45:33 UTC

Inherited Handles and APR

After a respectable lunch at Boudin's, Ryan and I think we have the general answers
to child handles.

The apr_foo_open/create calls need an APR_INHERIT flag bit to mark resources as
inheritable.  This offers two advantages;

  1. the app doesn't need to worry about fork/exec cleanups v.s. inherit bits for
     createprocess() based Win32 (and other non-forks.)

  2. mainline code is more readable, as the user no longer registers their own
     cleanup_for_exec handlers to simply close handles.

The patch, for apr_file_open, follows.  Rbb is working on a similar for Sockets, and
calls that generate 'handles' will need to be reviewed as well...

I obviously have the much larger job of applying the APR_INHERIT flag judiciously
throughout the entire server and removing child cleanups where appropriate ...

Bill

Re: Inherited Handles and APR

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Anyone care to apply this to unix and hack it a bit?  I only found four cases where
we (probably) should be calling apr_file_open() to create an inheritable file.

As I mention, sockets are still an issue, so are child processes, shared memory
handles, and a list of others.  But if we knock these off one bit at a time, we will
probably knock down the child handle abuse.

Serious (?) bug I noticed, if a buffered file has pending data to write, and a child
is spawned, both the parent _AND_ child will attempt to flush the buffer.  It used
to happen when the child pool was destroyed, with this patch it will occur when the
process is spawned.  It appears we need a pre-fork() registered action, to flush
buffers and the like.  That's only partially effective though, unless we do some
heavy mutex protection :(

I'm still traveling till Sunday morning, so I can't revisit for a bit.



----- Original Message ----- 
From: "William A. Rowe, Jr." <wr...@rowe-clan.net>
To: <de...@apr.apache.org>
Sent: Friday, July 13, 2001 3:45 PM
Subject: Inherited Handles and APR


> After a respectable lunch at Boudin's, Ryan and I think we have the general answers
> to child handles.
> 
> The apr_foo_open/create calls need an APR_INHERIT flag bit to mark resources as
> inheritable.  This offers two advantages;
> 
>   1. the app doesn't need to worry about fork/exec cleanups v.s. inherit bits for
>      createprocess() based Win32 (and other non-forks.)
> 
>   2. mainline code is more readable, as the user no longer registers their own
>      cleanup_for_exec handlers to simply close handles.
> 
> The patch, for apr_file_open, follows.  Rbb is working on a similar for Sockets, and
> calls that generate 'handles' will need to be reviewed as well...
> 
> I obviously have the much larger job of applying the APR_INHERIT flag judiciously
> throughout the entire server and removing child cleanups where appropriate ...
> 
> Bill
> 

Re: Inherited Handles and APR

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Anyone care to apply this to unix and hack it a bit?  I only found four cases where
we (probably) should be calling apr_file_open() to create an inheritable file.

As I mention, sockets are still an issue, so are child processes, shared memory
handles, and a list of others.  But if we knock these off one bit at a time, we will
probably knock down the child handle abuse.

Serious (?) bug I noticed, if a buffered file has pending data to write, and a child
is spawned, both the parent _AND_ child will attempt to flush the buffer.  It used
to happen when the child pool was destroyed, with this patch it will occur when the
process is spawned.  It appears we need a pre-fork() registered action, to flush
buffers and the like.  That's only partially effective though, unless we do some
heavy mutex protection :(

I'm still traveling till Sunday morning, so I can't revisit for a bit.



----- Original Message ----- 
From: "William A. Rowe, Jr." <wr...@rowe-clan.net>
To: <de...@apr.apache.org>
Sent: Friday, July 13, 2001 3:45 PM
Subject: Inherited Handles and APR


> After a respectable lunch at Boudin's, Ryan and I think we have the general answers
> to child handles.
> 
> The apr_foo_open/create calls need an APR_INHERIT flag bit to mark resources as
> inheritable.  This offers two advantages;
> 
>   1. the app doesn't need to worry about fork/exec cleanups v.s. inherit bits for
>      createprocess() based Win32 (and other non-forks.)
> 
>   2. mainline code is more readable, as the user no longer registers their own
>      cleanup_for_exec handlers to simply close handles.
> 
> The patch, for apr_file_open, follows.  Rbb is working on a similar for Sockets, and
> calls that generate 'handles' will need to be reviewed as well...
> 
> I obviously have the much larger job of applying the APR_INHERIT flag judiciously
> throughout the entire server and removing child cleanups where appropriate ...
> 
> Bill
> 

Re: Inherited Handles and APR

Posted by rb...@covalent.net.
On Tue, 17 Jul 2001, dean gaudet wrote:

> +1 from me too (although i don't know that i necessarily have a vote in
> APR :)

You do, and just to show that we all agree... +1 from me as well.

Ryan

>
> -dean
>
> On Tue, 17 Jul 2001, Justin Erenkrantz wrote:
>
> > On Tue, Jul 17, 2001 at 08:32:47PM -0500, William A. Rowe, Jr. wrote:
> > > Then, I'm +1 on _not_ passing this as an argument to the creation fn, rather assuming
> > > that the inherit flag is _not_ set, and force the user to call apr_socket_inherit_set
> > > or apr_file_inherit_set.
> > >
> > > Does this work for _everyone_?
> >
> > +1.  Much nicer.  -- justin
> >
> >
>
>
>


_____________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
-----------------------------------------------------------------------------


Re: Inherited Handles and APR

Posted by dean gaudet <de...@arctic.org>.
+1 from me too (although i don't know that i necessarily have a vote in
APR :)

-dean

On Tue, 17 Jul 2001, Justin Erenkrantz wrote:

> On Tue, Jul 17, 2001 at 08:32:47PM -0500, William A. Rowe, Jr. wrote:
> > Then, I'm +1 on _not_ passing this as an argument to the creation fn, rather assuming
> > that the inherit flag is _not_ set, and force the user to call apr_socket_inherit_set
> > or apr_file_inherit_set.
> >
> > Does this work for _everyone_?
>
> +1.  Much nicer.  -- justin
>
>


Re: Inherited Handles and APR

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jul 17, 2001 at 08:32:47PM -0500, William A. Rowe, Jr. wrote:
> Then, I'm +1 on _not_ passing this as an argument to the creation fn, rather assuming
> that the inherit flag is _not_ set, and force the user to call apr_socket_inherit_set
> or apr_file_inherit_set.
> 
> Does this work for _everyone_?

+1.  Much nicer.  -- justin


Re: Inherited Handles and APR

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "dean gaudet" <de...@arctic.org>
Sent: Tuesday, July 17, 2001 7:52 PM


> On Tue, 17 Jul 2001, William A. Rowe, Jr. wrote:
> 
> > We agree.  APR_INHERIT is the 'option', APR_NON_INHERIT is a zero (usual case) value.
> 
> my complaint is that you're modifying all the handle creation APIs with
> another parameter...

Then, I'm +1 on _not_ passing this as an argument to the creation fn, rather assuming
that the inherit flag is _not_ set, and force the user to call apr_socket_inherit_set
or apr_file_inherit_set.

Does this work for _everyone_?


Re: Inherited Handles and APR

Posted by dean gaudet <de...@arctic.org>.
On Tue, 17 Jul 2001, William A. Rowe, Jr. wrote:

> We agree.  APR_INHERIT is the 'option', APR_NON_INHERIT is a zero (usual case) value.

my complaint is that you're modifying all the handle creation APIs with
another parameter...

-dean


Re: Inherited Handles and APR

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "dean gaudet" <de...@arctic.org>
Sent: Tuesday, July 17, 2001 7:14 PM


> On Fri, 13 Jul 2001, William A. Rowe, Jr. wrote:
> 
> > After a respectable lunch at Boudin's, Ryan and I think we have the general answers
> > to child handles.
> >
> > The apr_foo_open/create calls need an APR_INHERIT flag bit to mark resources as
> > inheritable.  This offers two advantages;
> 
> this sounds like the wrong approach.
> 
> the default should be to assume that all file handles are not inherited.
> this is the 99% case for handle creation.

We agree.  APR_INHERIT is the 'option', APR_NON_INHERIT is a zero (usual case) value.

> then as a "new process attribute" you list the handles which should be
> inherited.  (in general these would be just the 3 standard ones.)

Hmmm... I'm thinking of cooking up _something_ along those lines, esp for Win32.

> it's possible to write a linux clone()-based fork() which inherits nothing
> by default without having to do lame-ass stuff such as setting O_CLOEXEC
> on every bloody handle.  then you just open("/proc/parent/fd/n") to get
> the handful of descriptors you wanted to inherit (or use other fd passing
> methods).  this should be a faster fork() off a large multithreaded, lots
> of fds server.

AHHH... didn't pay attention that you were going to play with a custom tweak here.
Since it isn't cross-platform though, I believe the code now committed still makes
sense, and should be overriden by platform in arch/someplat/inherit.h or the
some_api/some_plat/apicode.c module.  I'm just noticing the rather redundant checks
could be cleaned up with another macro in inherit.h to further introduce some
consistency here.

> and it's even simpler on windows... your API would seem to be worse for
> windows than it needs to be.

Actually, the APR_INHERIT is implemented on Win32, as an inheritable HANDLE, but
that's about it.  It will certainly need to become a little more sophisticated.

Sort of comes back to needing a fork() on Win32, for certain cases :)  For create
process, we need some mechansim to pass up all the 'other' things we offered to
inherit, and for those I don't have a good answer at this moment.

Bill


Re: Inherited Handles and APR

Posted by dean gaudet <de...@arctic.org>.
On Fri, 13 Jul 2001, William A. Rowe, Jr. wrote:

> After a respectable lunch at Boudin's, Ryan and I think we have the general answers
> to child handles.
>
> The apr_foo_open/create calls need an APR_INHERIT flag bit to mark resources as
> inheritable.  This offers two advantages;

this sounds like the wrong approach.

the default should be to assume that all file handles are not inherited.
this is the 99% case for handle creation.

then as a "new process attribute" you list the handles which should be
inherited.  (in general these would be just the 3 standard ones.)

it's possible to write a linux clone()-based fork() which inherits nothing
by default without having to do lame-ass stuff such as setting O_CLOEXEC
on every bloody handle.  then you just open("/proc/parent/fd/n") to get
the handful of descriptors you wanted to inherit (or use other fd passing
methods).  this should be a faster fork() off a large multithreaded, lots
of fds server.

and it's even simpler on windows... your API would seem to be worse for
windows than it needs to be.

-dean