Hi, Ryan.
<[email protected]> wrote on 2016-Jun-29 at 11:22 AM:
This is a Trac with Nginx and fcgi configuration on Debian 8.5, which has
always been problematic but has become very unstable over the past few days. I
searched existing issues (1), but am currently out of ideas. Excerpt from log
below, along with configuration.
Unfortunately I don't know Nginx or fcgi very well. I'm considering moving to
Apache with mod_wsgi. Any hints about what the problem might be would be
appreciated.
- Ryan
After [14419], we use PIPEs for stdin/stdout/stderr of Popen. I guess those
pipes leak and EMFILE is raised due to rendering many git revisions in tickets'
description in custom query. However, the pipes are closed by gc.collect() each
request.
Therefore, I think this issue wouldn't be solved by moving to mod_wsgi.
I attach patch which closes the pipes. Could you please try it?
--
Jun Omae <[email protected]> (大前 潤)
--
You received this message because you are subscribed to the Google Groups "Trac
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.
diff --git a/tracopt/versioncontrol/git/PyGIT.py
b/tracopt/versioncontrol/git/PyGIT.py
index fc61319ed..0e482e19d 100644
--- a/tracopt/versioncontrol/git/PyGIT.py
+++ b/tracopt/versioncontrol/git/PyGIT.py
@@ -86,6 +86,13 @@ def _unquote(path):
return path
+def _close_proc_pipes(proc):
+ if proc:
+ for f in (proc.stdin, proc.stdout, proc.stderr):
+ if f:
+ f.close()
+
+
class GitCore(object):
"""Low-level wrapper around git executable"""
@@ -139,6 +146,7 @@ class GitCore(object):
p = self.__pipe(git_cmd, stdout=PIPE, stderr=PIPE, *cmd_args)
stdout_data, stderr_data = p.communicate()
+ _close_proc_pipes(p)
if self.__log and (p.returncode != 0 or stderr_data):
self.__log.debug('%s exits with %d, dir: %r, args: %s %r, '
'stderr: %r', self.__git_bin, p.returncode,
@@ -417,12 +425,15 @@ class Storage(object):
self.logger.debug("PyGIT.Storage instance for '%s' is constructed",
git_dir)
+ def _cleanup_proc(self, proc):
+ if proc:
+ _close_proc_pipes(proc)
+ terminate(proc)
+ proc.wait()
+
def __del__(self):
with self.__cat_file_pipe_lock:
- if self.__cat_file_pipe is not None:
- self.__cat_file_pipe.stdin.close()
- terminate(self.__cat_file_pipe)
- self.__cat_file_pipe.wait()
+ self._cleanup_proc(self.__cat_file_pipe)
#
# cache handling
@@ -683,9 +694,7 @@ class Storage(object):
# consistent state (Otherwise it happens that next time we
# call cat_file we get payload from previous call)
self.logger.debug("closing cat_file pipe")
- self.__cat_file_pipe.stdin.close()
- terminate(self.__cat_file_pipe)
- self.__cat_file_pipe.wait()
+ self._cleanup_proc(self.__cat_file_pipe)
self.__cat_file_pipe = None
def verifyrev(self, rev):
@@ -923,9 +932,8 @@ class Storage(object):
path, _ = path.rsplit('/', 1)
except ValueError:
break
- f.close()
- terminate(p[0])
- p[0].wait()
+ if p:
+ self._cleanup_proc(p[0])
p[:] = []
while True:
yield None
@@ -943,9 +951,7 @@ class Storage(object):
yield historian
finally:
if p:
- p[0].stdout.close()
- terminate(p[0])
- p[0].wait()
+ self._cleanup_proc(p[0])
def last_change(self, sha, path, historian=None):
if historian is not None:
diff --git a/tracopt/versioncontrol/git/tests/git_fs.py
b/tracopt/versioncontrol/git/tests/git_fs.py
index 500931f42..baf149f33 100644
--- a/tracopt/versioncontrol/git/tests/git_fs.py
+++ b/tracopt/versioncontrol/git/tests/git_fs.py
@@ -64,6 +64,7 @@ class GitCommandMixin(object):
def _git(self, *args, **kwargs):
proc = self._spawn_git(*args, **kwargs)
stdout, stderr = proc.communicate()
+ self._close_proc_pipes(proc)
self.assertEqual(0, proc.returncode,
'git exits with %r, args %r, kwargs %r, stdout %r, '
'stderr %r' %
@@ -75,6 +76,7 @@ class GitCommandMixin(object):
data = data.encode('utf-8')
proc = self._spawn_git('fast-import', stdin=PIPE)
stdout, stderr = proc.communicate(input=data)
+ self._close_proc_pipes(proc)
self.assertEqual(0, proc.returncode,
'git exits with %r, stdout %r, stderr %r' %
(proc.returncode, stdout, stderr))
@@ -96,6 +98,11 @@ class GitCommandMixin(object):
env['GIT_COMMITTER_DATE'] = dt
env['GIT_AUTHOR_DATE'] = dt
+ def _close_proc_pipes(self, proc):
+ for f in (proc.stdin, proc.stdout, proc.stderr):
+ if f:
+ f.close()
+
class BaseTestCase(unittest.TestCase, GitCommandMixin):
