Skip to content

Commit d58ff6f

Browse files
author
Neha Singla
committed
Fix git subprocess termination issues to prevent process accumulation
- Add timeout to subprocess.communicate() to prevent indefinite hanging - Fix pexpect timeout from None to use actual timeout parameter - Add proper process cleanup with graceful termination → wait → force kill - Improve credential cache daemon cleanup with timeout handling - Add comprehensive error handling for timeout scenarios - Ensure all subprocess exceptions properly clean up processes Fixes issue where git subprocesses could accumulate and consume system resources when commands hang due to network issues or authentication problem
1 parent c5585fd commit d58ff6f

File tree

1 file changed

+50
-3
lines changed

1 file changed

+50
-3
lines changed

jupyterlab_git/git.py

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ async def call_subprocess_with_authentication(
113113
cwd=cwd,
114114
env=env,
115115
encoding="utf-8",
116-
timeout=None,
116+
timeout=timeout,
117117
)
118118

119119
# We expect a prompt from git
@@ -140,6 +140,19 @@ async def call_subprocess_with_authentication(
140140
returncode = p.exitstatus
141141
p.close() # close process
142142
return returncode, "", response
143+
except pexpect.exceptions.TIMEOUT: # Handle timeout
144+
p.terminate(force=True)
145+
p.close()
146+
raise TimeoutError(
147+
f"Git authentication timed out after {timeout} seconds: {' '.join(cmdline)}"
148+
)
149+
except Exception as e:
150+
# Ensure process is always closed on any exception
151+
if p and p.isalive():
152+
p.terminate(force=True)
153+
if p:
154+
p.close()
155+
raise e
143156

144157
def call_subprocess(
145158
cmdline: "List[str]",
@@ -150,7 +163,21 @@ def call_subprocess(
150163
process = subprocess.Popen(
151164
cmdline, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, env=env
152165
)
153-
output, error = process.communicate()
166+
try:
167+
output, error = process.communicate(timeout=timeout)
168+
except subprocess.TimeoutExpired:
169+
# Terminate the process gracefully
170+
process.terminate()
171+
try:
172+
process.wait(timeout=5)
173+
except subprocess.TimeoutExpired:
174+
# Force kill if still alive after 5 seconds
175+
process.kill()
176+
process.wait()
177+
raise TimeoutError(
178+
f"Git command timed out after {timeout} seconds: {' '.join(cmdline)}"
179+
)
180+
154181
if is_binary:
155182
return (
156183
process.returncode,
@@ -198,6 +225,10 @@ def call_subprocess(
198225
get_logger().debug(
199226
"Code: {}\nOutput: {}\nError: {}".format(code, log_output, log_error)
200227
)
228+
except TimeoutError as e:
229+
# Handle our custom timeout errors
230+
code, output, error = -1, "", str(e)
231+
get_logger().warning("Git command timed out: {!s}".format(cmdline))
201232
except BaseException as e:
202233
code, output, error = -1, "", traceback.format_exc()
203234
get_logger().warning("Fail to execute {!s}".format(cmdline), exc_info=True)
@@ -228,8 +259,24 @@ def __init__(self, config=None):
228259
)
229260

230261
def __del__(self):
262+
self._cleanup_processes()
263+
264+
def _cleanup_processes(self):
265+
"""Clean up any running processes managed by this Git instance."""
231266
if self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS:
232-
self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS.terminate()
267+
try:
268+
if self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS.poll() is None:
269+
self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS.terminate()
270+
try:
271+
self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS.wait(timeout=5)
272+
except subprocess.TimeoutExpired:
273+
self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS.kill()
274+
self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS.wait()
275+
get_logger().debug("Git credential cache daemon process cleaned up")
276+
except Exception as e:
277+
get_logger().warning(f"Failed to cleanup credential cache daemon: {e}")
278+
finally:
279+
self._GIT_CREDENTIAL_CACHE_DAEMON_PROCESS = None
233280

234281
async def __execute(
235282
self,

0 commit comments

Comments
 (0)