Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11973 closed defect (fixed)

Historian leaves the git log instance open

Reported by: spam.martin.schulze@… Owned by: Jun Omae
Priority: normal Milestone: 1.0.5
Component: plugin/git Version: 1.0.4
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fix git process running forever when breaking GitNode.get_entries() iteration.

API Changes:
Internal Changes:

Description

How to reproduce

Setup a Trac instance with bitten on Apache. Add a git repository to Trac and a Build Configuration for it to Bitten.

After some succesful builds and enough commits to the repository each visit of the bitten-slave should leave open several git log processes in the system.

System information

  • Arch Linux x64
  • Python 2.7.9

I already posted this error on the Bitten bug tracker (http://bitten.edgewall.org/ticket/836) but the problem is not in Bitten.

When the bitten-slave visits my Trac instance the function get_historian in PyGIT gets called.

A commenter suggested to add --no-pager to the git command which solved the problem for him and for me as well.

I would suggest to add this change to the PyGIT module.

Attachments (0)

Change History (8)

comment:1 by Jun Omae, 6 years ago

Strange. Trac uses a pipe for stdout of git command. The git command doesn't spawn pager if the stdout is not a tty. Adding --no-pager option must have no effect.

    def log_pipe(self, *cmd_args):
        return self.__pipe('log', stdout=PIPE, *cmd_args)

comment:2 by Jun Omae, 6 years ago

Component: version controlplugin/git

comment:3 by Martin Schulze <spam.martin.schulze@…>, 6 years ago

Ok, I take back that --no-pager fixes the problem. I added the option at the wrong place and git errored out which solved the problem of non-closing git processes but broke the functionality (without me noticing).

This means the problem still persists.

comment:4 by Jun Omae, 6 years ago

Could you please try the following patch?

  • tracopt/versioncontrol/git/PyGIT.py

    diff --git a/tracopt/versioncontrol/git/PyGIT.py b/tracopt/versioncontrol/git/PyGIT.py
    index 0898732..20405ef 100644
    a b class Storage(object):  
    963963            except KeyError:
    964964                next_path[:] = [path]
    965965                return gen.next()
    966         yield historian
    967966
    968         if p:
    969             p[0].stdout.close()
    970             terminate(p[0])
    971             p[0].wait()
     967        try:
     968            yield historian
     969        finally:
     970            if p:
     971                p[0].stdout.close()
     972                terminate(p[0])
     973                p[0].wait()
    972974
    973975    def last_change(self, sha, path, historian=None):
    974976        if historian is not None:

comment:5 by Jun Omae, 6 years ago

Milestone: 1.0.6
Owner: set to Jun Omae
Status: newassigned

Reproduced with the following (from bitten:source:trunk/bitten/queue.py@:76-80#L74):

>>> from trac.env import open_environment
>>> env = open_environment(...)
>>> repos = env.get_repository('pygit2')
>>> node = repos.get_node('/')
>>> for entry in node.get_entries():
...   break
...
>>>

After the above, run ps xf:

$ ps xf
  PID TTY      STAT   TIME COMMAND
 2073 ?        S      1:08 sshd: jun66j5@pts/1
 2074 pts/1    Ss     0:05  \_ -bash
 1752 pts/1    Tl     0:23      \_ vim
26846 pts/1    S+     0:00      \_ make python=25-1.0 clean Trac.egg-info start-python
27083 pts/1    S+     0:00          \_ python
27090 pts/1    S+     0:00              \_ git --git-dir=/home/jun66j5/var/git/pygit2 log --pretty=format:%n%H --name-status 324a8d5....
 1772 ?        S      0:35 sshd: jun66j5@pts/0
 1773 pts/0    Ss     0:03  \_ -bash
19523 pts/0    Tl     1:01      \_ vim
27102 pts/0    R+     0:00      \_ ps xf
Last edited 6 years ago by Jun Omae (previous) (diff)

comment:6 by Martin Schulze <spam.martin.schulze@…>, 6 years ago

The patch seems to solve the problem. I can find no accumulating git processes after applying it.

comment:7 by Jun Omae, 6 years ago

Milestone: 1.0.61.0.5
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed in [13844] and merged to trunk in [13845].

in reply to:  3 comment:8 by anders@…, 6 years ago

I was the one who suggested —no-pager and I did exactly this mistake myself, —no-pager in the wrong place. Everything became 45 years old (1970-01-01) because the plugin was broken.

Back-porting the patch suggested above into my setup with Trac 0.12 and the Git plugin apparently fixes it for me too. Very good, thanks!

Last edited 6 years ago by Ryan J Ollos (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.