From 5757d28c2fd9cd78c5570b4e33543fca00184cd6 Mon Sep 17 00:00:00 2001
From: Christophe Monniez <moc@odoo.com>
Date: Wed, 27 May 2026 14:22:30 +0200
Subject: [PATCH] [IMP] runbot: kill hanging git child process

When fetching from GitHub, the git process sometimes hangs for a long
time regardless of whether https or ssh is used. Investigation showed
that the last child process of the git command is always the one
hanging. Killing that subprocess allows git to retry the operation
without data loss.

This commit monitors the git process and kills its hanging child
process on timeout.
---
 runbot/models/repo.py     | 32 +++++++++++++++++++++++------
 runbot/tests/test_repo.py | 42 ++++++++++++++++++++-------------------
 2 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/runbot/models/repo.py b/runbot/models/repo.py
index 77b8c6dd9..51f6a9c8c 100644
--- a/runbot/models/repo.py
+++ b/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)
diff --git a/runbot/tests/test_repo.py b/runbot/tests/test_repo.py
index cb8195a05..d4c40fcae 100644
--- a/runbot/tests/test_repo.py
+++ b/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):