Merge pull request #654 from meeseeksmachine/auto-backport-of-pr-564-on-0.11.x

Backport PR #564 on branch 0.11.x (Fix race condition issues on slower FS)

Author: fcollonval

jupyterlab_git/git.py

@@ -8,6 +8,8 @@
 
 import pexpect
 import tornado
+import tornado.locks
+import datetime
 
 
 # Git configuration options exposed through the REST API
@@ -16,11 +18,18 @@
 # See https://git-scm.com/docs/git-config#_syntax for git var syntax
 CONFIG_PATTERN = re.compile(r"(?:^|\n)([\w\-\.]+)\=")
 DEFAULT_REMOTE_NAME = "origin"
+# How long to wait to be executed or finished your execution before timing out
+MAX_WAIT_FOR_EXECUTE_S = 20
+# Ensure on NFS or similar, that we give the .git/index.lock time to be removed
+MAX_WAIT_FOR_LOCK_S = 5
+# How often should we check for the lock above to be free? This comes up more on things like NFS
+CHECK_LOCK_INTERVAL_S = 0.1
 
+execution_lock = tornado.locks.Lock()
 
 async def execute(
     cmdline: "List[str]",
-    cwd: "Optional[str]" = None,
+    cwd: "str",
     env: "Optional[Dict[str, str]]" = None,
     username: "Optional[str]" = None,
     password: "Optional[str]" = None,
@@ -85,19 +94,36 @@ def call_subprocess(
         output, error = process.communicate()
         return (process.returncode, output.decode("utf-8"), error.decode("utf-8"))
 
-    if username is not None and password is not None:
-        code, output, error = await call_subprocess_with_authentication(
-            cmdline,
-            username,
-            password,
-            cwd,
-            env,
-        )
-    else:
-        current_loop = tornado.ioloop.IOLoop.current()
-        code, output, error = await current_loop.run_in_executor(
-            None, call_subprocess, cmdline, cwd, env
-        )
+    try:
+        await execution_lock.acquire(timeout=datetime.timedelta(seconds=MAX_WAIT_FOR_EXECUTE_S))
+    except  tornado.util.TimeoutError:
+        return (1, "", "Unable to get the lock on the directory")
+
+    try:
+        # Ensure our execution operation will succeed by first checking and waiting for the lock to be removed
+        time_slept = 0
+        lockfile = os.path.join(cwd, '.git', 'index.lock')
+        while os.path.exists(lockfile) and time_slept < MAX_WAIT_FOR_LOCK_S:
+            await tornado.gen.sleep(CHECK_LOCK_INTERVAL_S)
+            time_slept += CHECK_LOCK_INTERVAL_S
+
+        # If the lock still exists at this point, we will likely fail anyway, but let's try anyway
+
+        if username is not None and password is not None:
+            code, output, error = await call_subprocess_with_authentication(
+                cmdline,
+                username,
+                password,
+                cwd,
+                env,
+            )
+        else:
+            current_loop = tornado.ioloop.IOLoop.current()
+            code, output, error = await current_loop.run_in_executor(
+                None, call_subprocess, cmdline, cwd, env
+            )
+    finally:
+        execution_lock.release()
 
     return code, output, error
 
@@ -1003,7 +1029,7 @@ async def diff_content(self, filename, prev_ref, curr_ref, top_repo_path):
                 is_binary = await self._is_binary(filename, "INDEX", top_repo_path)
                 if is_binary:
                     raise tornado.web.HTTPError(log_message="Error occurred while executing command to retrieve plaintext diff as file is not UTF-8.")
-                    
+
                 curr_content = await self.show(filename, "", top_repo_path)
             else:
                 raise tornado.web.HTTPError(

jupyterlab_git/tests/test_execute.py

@@ -0,0 +1,27 @@
+import pytest
+from unittest.mock import patch
+
+from jupyterlab_git.git import execute, execution_lock
+
+
+@pytest.mark.asyncio
+async def test_execute_waits_on_index_lock(tmp_path):
+    lock_file = tmp_path / ".git/index.lock"
+    lock_file.parent.mkdir(parents=True, exist_ok=True)
+    lock_file.write_text("")
+
+    async def remove_lock_file(*args):
+        assert "unlocked" not in repr(execution_lock)  # Check that the lock is working
+        lock_file.unlink()  # Raise an error for missing file
+        
+    with patch("tornado.gen.sleep") as sleep:
+        sleep.side_effect = remove_lock_file   # Remove the lock file instead of sleeping
+        
+        assert "unlock" in repr(execution_lock)
+        cmd = ["git", "dummy"]
+        kwargs = {"cwd": f"{tmp_path!s}"}
+        await execute(cmd, **kwargs)
+        assert "unlock" in repr(execution_lock)
+        
+        assert not lock_file.exists()
+        sleep.assert_called_once()