[IMP] runbot: kill hanging git child process

runbot/models/repo.py

@@ -6,6 +6,7 @@
 import re
 import subprocess
 import time
+import psutil
 import requests
 import markupsafe
 import shlex
@@ -512,12 +513,31 @@ def _git(self, cmd, errors='strict', quiet=False, input_data=None, raw=False):
         cmd = self._get_git_command(cmd, errors)
         if not quiet:
             _logger.info("git command: %s", shlex.join(cmd))
-        kwargs = {'stderr': subprocess.STDOUT}
-        if input_data is not None:
-            if isinstance(input_data, str):
-                input_data = input_data.encode('utf-8')
-            kwargs['input'] = input_data
-        output = subprocess.check_output(cmd, **kwargs)
+
+        if input_data is not None and isinstance(input_data, str):
+            input_data = input_data.encode('utf-8')
+
+        stdin = subprocess.PIPE if input_data is not None else None
+        process = subprocess.Popen(cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+
+        fetch_timeout = int(self.env['ir.config_parameter'].get_param('runbot.runbot_fetch_timeout', default=30))
+        while True:
+            try:
+                output, _ = process.communicate(input=input_data, timeout=fetch_timeout)
+                break
+            except subprocess.TimeoutExpired:
+                try:
+                    parent = psutil.Process(process.pid)
+                    for child in parent.children(recursive=True):
+                        if 'git-upload-pack' in str(child.cmdline()) and child.status() == psutil.STATUS_SLEEPING:
+                            child.kill()
+                            _logger.info("Killed sleeping git subprocess (pid: %s)", child.pid)
+                except psutil.NoSuchProcess:
+                    pass
+
+        if process.returncode:
+            raise subprocess.CalledProcessError(process.returncode, cmd, output=output)
+
         if raw:
             return output
         return output.decode(errors=errors)

runbot/tests/test_repo.py

@@ -383,26 +383,28 @@ def mock_git_helper(self, repo, cmd, input_data=None, raw=False):
 
 class TestIdentityFile(RunbotCase):
 
-        def check_output_helper(self):
-            """Helper that returns a mock for repo._git()"""
-            def mock_check_output(cmd, *args, **kwargs):
-                expected_option = r'-c core.sshCommand=ssh -i \/.+\/\.ssh\/fake_identity'
-                git_cmd = ' '.join(cmd)
-                self.assertTrue(re.search(expected_option, git_cmd), '%s did not match %s' % (git_cmd, expected_option))
-                return Mock()
-
-            return mock_check_output
-
-        def test_identity_file(self):
-            """test that the identity file is used in git command"""
-
-            self.patcher_objects['git_patcher'].stop()
-            self.start_patcher('check_output_patcher', 'odoo.addons.runbot.models.repo.subprocess.check_output', new=self.check_output_helper())
-
-            self.repo_odoo.identity_file = 'fake_identity'
-
-            with mute_logger("odoo.addons.runbot.models.repo"):
-                self.repo_odoo._update_fetch_cmd()
+    def popen_helper(self):
+        """Helper that returns a mock for repo._git()"""
+        def mock_popen(cmd, *args, **kwargs):
+            expected_option = r'-c core.sshCommand=ssh -i \/.+\/\.ssh\/fake_identity'
+            git_cmd = ' '.join(cmd)
+            self.assertTrue(re.search(expected_option, git_cmd), '%s did not match %s' % (git_cmd, expected_option))
+            popen_mock = Mock()
+            attrs = {"communicate.return_value": (b"", b"")}
+            popen_mock.configure_mock(**attrs)
+            return popen_mock
+
+        return mock_popen
+
+    def test_identity_file(self):
+        """test that the identity file is used in git command"""
+
+        self.patcher_objects['git_patcher'].stop()
+        self.start_patcher('popen_patcher', 'odoo.addons.runbot.models.repo.subprocess.Popen', new=self.popen_helper())
+        self.repo_odoo.identity_file = 'fake_identity'
+
+        with mute_logger("odoo.addons.runbot.models.repo"):
+            self.repo_odoo._update_fetch_cmd()
 
 
 class TestRepoScheduler(RunbotCase):