You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Sander Striker <st...@apache.org> on 2002/01/13 12:45:41 UTC

FW: svn_io_run_cmd running out of file handles, APR bug?

Hi,

Anyone a direct idea of what is going on and if it is
correct or not?

Sander

-----Original Message-----
From: Philip Martin [mailto:philip@codematters.co.uk]
Sent: 13 January 2002 05:26
To: dev@subversion.tigris.org
Subject: svn_io_run_cmd running out of file handles, APR bug?


Hi

Writing a new test I have been calling svn_io_run_cmd() in a loop,
after a few hundred calls it fails with

apr_error: #24, src_err 0 : <Too many open files>
  run_cmd_in_directory: error setting /usr/bin/cmp process io attributes

Looking in /proc/nnn/fd I see hundreds of pipes.

As far as I can see:

- svn_io_run_cmd() calls apr_procattr_io_set()

  This calls apr_file_pipe_create() to creates three pipes in the
  apr_procattr_t structure.


- svn_io_run_cmd() calls apr_procattr_child_in_set(),
  apr_procattr_child_out_set() and apr_procattr_child_err_set()

  These call apr_file_dup() and overwrite the pipe descriptors.


The result is that the pipe descriptors don't get closed and so
accumulate until the process runs out of resources.

Now even if svn_io_run_cmd() doesn't call apr_procattr_io_set() to
create the pipes, the problem will still occur because the three
functions apr_procattr_child_err_set() et.al. each start by calling
apr_file_pipe_create() themselves if the pipe doesn't exist.

What are these pipes for? It seems silly to create them simply to
close them again. Is this an APR bug?

-- 
Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org




Re: FW: svn_io_run_cmd running out of file handles, APR bug?

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sun, Jan 13, 2002 at 12:45:41PM +0100, Sander Striker wrote:
> Hi,
> 
> Anyone a direct idea of what is going on and if it is
> correct or not?

I'm thinking that the pipe code should be calling dup2 instead.
dup2 will close the old fd which is what I *believe* we want in
this code.  The pipe code will only cleanup what the apr_file_t
points at - not the original FDs that were called (we're changing
them by calling apr_procattr_child_*_set).

In SVN, I bet we will always be creating the pipes in 
apr_procattr_child_*_set when we enter that function, so the dup2 
*sounds* right - we want to close the old pipes we just created.
With mod_cgi, we call apr_procattr_io_set, so the pipes are
certainly created there first.

Can someone please test this and see if it fixes the problem?
I'm way too tired to test this myself (which also leads me to
believe that I may be 100% wrong...).  -- justin

Index: threadproc/unix/proc.c
===================================================================
RCS file: /home/cvs/apr/threadproc/unix/proc.c,v
retrieving revision 1.53
diff -u -r1.53 proc.c
--- threadproc/unix/proc.c	11 Nov 2001 05:51:00 -0000	1.53
+++ threadproc/unix/proc.c	23 Jan 2002 06:53:57 -0000
@@ -141,10 +141,10 @@
         apr_file_pipe_create(&attr->child_in, &attr->parent_in, attr->cntxt);
 
     if (child_in != NULL)
-        apr_file_dup(&attr->child_in, child_in, attr->cntxt);
+        apr_file_dup2(&attr->child_in, child_in, attr->cntxt);
 
     if (parent_in != NULL)
-        apr_file_dup(&attr->parent_in, parent_in, attr->cntxt);
+        apr_file_dup2(&attr->parent_in, parent_in, attr->cntxt);
 
     return APR_SUCCESS;
 }
@@ -157,10 +157,10 @@
         apr_file_pipe_create(&attr->child_out, &attr->parent_out, attr->cntxt);
 
     if (child_out != NULL)
-        apr_file_dup(&attr->child_out, child_out, attr->cntxt);
+        apr_file_dup2(&attr->child_out, child_out, attr->cntxt);
 
     if (parent_out != NULL)
-        apr_file_dup(&attr->parent_out, parent_out, attr->cntxt);
+        apr_file_dup2(&attr->parent_out, parent_out, attr->cntxt);
 
     return APR_SUCCESS;
 }
@@ -173,10 +173,10 @@
         apr_file_pipe_create(&attr->child_err, &attr->parent_err, attr->cntxt);
 
     if (child_err != NULL)
-        apr_file_dup(&attr->child_err, child_err, attr->cntxt);
+        apr_file_dup2(&attr->child_err, child_err, attr->cntxt);
 
     if (parent_err != NULL)
-        apr_file_dup(&attr->parent_err, parent_err, attr->cntxt);
+        apr_file_dup2(&attr->parent_err, parent_err, attr->cntxt);
 
     return APR_SUCCESS;
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: FW: svn_io_run_cmd running out of file handles, APR bug?

Posted by Jeff Trawick <tr...@attglobal.net>.
"Sander Striker" <st...@apache.org> writes:

> Hi,
> 
> Anyone a direct idea of what is going on and if it is
> correct or not?

of course it isn't correct :)

another example: mod_cgid

mod_cgid currently has a pipe leak.

mod_cgid for a long time has had some needless opening/closing of
pipes.

Run strace on mod_cgid and you'll see several "close(-1)" calls for
each request.  There is some discrepancy between cleanups and explicit
close logic for some reason.

I don't see any leak in mod_cgi.

No, I don't know what is going on.  I have a feeling that part of this
is due to a broken API (APR doesn't have the information it needs to
know when to create pipes).  The leak is just a bug, perhaps
introduced with recent changes to apr_file_dup[2].

> -----Original Message-----
> From: Philip Martin [mailto:philip@codematters.co.uk]
> Sent: 13 January 2002 05:26
> To: dev@subversion.tigris.org
> Subject: svn_io_run_cmd running out of file handles, APR bug?
> 
> 
> Hi
> 
> Writing a new test I have been calling svn_io_run_cmd() in a loop,
> after a few hundred calls it fails with
> 
> apr_error: #24, src_err 0 : <Too many open files>
>   run_cmd_in_directory: error setting /usr/bin/cmp process io attributes
> 
> Looking in /proc/nnn/fd I see hundreds of pipes.
> 
> As far as I can see:
> 
> - svn_io_run_cmd() calls apr_procattr_io_set()
> 
>   This calls apr_file_pipe_create() to creates three pipes in the
>   apr_procattr_t structure.
> 
> 
> - svn_io_run_cmd() calls apr_procattr_child_in_set(),
>   apr_procattr_child_out_set() and apr_procattr_child_err_set()
> 
>   These call apr_file_dup() and overwrite the pipe descriptors.
> 
> 
> The result is that the pipe descriptors don't get closed and so
> accumulate until the process runs out of resources.
> 
> Now even if svn_io_run_cmd() doesn't call apr_procattr_io_set() to
> create the pipes, the problem will still occur because the three
> functions apr_procattr_child_err_set() et.al. each start by calling
> apr_file_pipe_create() themselves if the pipe doesn't exist.
> 
> What are these pipes for? It seems silly to create them simply to
> close them again. Is this an APR bug?
> 
> -- 
> Philip
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
> 
> 
> 

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Re: FW: svn_io_run_cmd running out of file handles, APR bug?

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Sun, Jan 13, 2002 at 12:45:41PM +0100, Sander Striker wrote:
> Hi,
> 
> Anyone a direct idea of what is going on and if it is
> correct or not?

I'm thinking that the pipe code should be calling dup2 instead.
dup2 will close the old fd which is what I *believe* we want in
this code.  The pipe code will only cleanup what the apr_file_t
points at - not the original FDs that were called (we're changing
them by calling apr_procattr_child_*_set).

In SVN, I bet we will always be creating the pipes in 
apr_procattr_child_*_set when we enter that function, so the dup2 
*sounds* right - we want to close the old pipes we just created.
With mod_cgi, we call apr_procattr_io_set, so the pipes are
certainly created there first.

Can someone please test this and see if it fixes the problem?
I'm way too tired to test this myself (which also leads me to
believe that I may be 100% wrong...).  -- justin

Index: threadproc/unix/proc.c
===================================================================
RCS file: /home/cvs/apr/threadproc/unix/proc.c,v
retrieving revision 1.53
diff -u -r1.53 proc.c
--- threadproc/unix/proc.c	11 Nov 2001 05:51:00 -0000	1.53
+++ threadproc/unix/proc.c	23 Jan 2002 06:53:57 -0000
@@ -141,10 +141,10 @@
         apr_file_pipe_create(&attr->child_in, &attr->parent_in, attr->cntxt);
 
     if (child_in != NULL)
-        apr_file_dup(&attr->child_in, child_in, attr->cntxt);
+        apr_file_dup2(&attr->child_in, child_in, attr->cntxt);
 
     if (parent_in != NULL)
-        apr_file_dup(&attr->parent_in, parent_in, attr->cntxt);
+        apr_file_dup2(&attr->parent_in, parent_in, attr->cntxt);
 
     return APR_SUCCESS;
 }
@@ -157,10 +157,10 @@
         apr_file_pipe_create(&attr->child_out, &attr->parent_out, attr->cntxt);
 
     if (child_out != NULL)
-        apr_file_dup(&attr->child_out, child_out, attr->cntxt);
+        apr_file_dup2(&attr->child_out, child_out, attr->cntxt);
 
     if (parent_out != NULL)
-        apr_file_dup(&attr->parent_out, parent_out, attr->cntxt);
+        apr_file_dup2(&attr->parent_out, parent_out, attr->cntxt);
 
     return APR_SUCCESS;
 }
@@ -173,10 +173,10 @@
         apr_file_pipe_create(&attr->child_err, &attr->parent_err, attr->cntxt);
 
     if (child_err != NULL)
-        apr_file_dup(&attr->child_err, child_err, attr->cntxt);
+        apr_file_dup2(&attr->child_err, child_err, attr->cntxt);
 
     if (parent_err != NULL)
-        apr_file_dup(&attr->parent_err, parent_err, attr->cntxt);
+        apr_file_dup2(&attr->parent_err, parent_err, attr->cntxt);
 
     return APR_SUCCESS;
 }