From 19dddedff6d6b1b986b606fdcd939dc9f59fb83f Mon Sep 17 00:00:00 2001
From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com>
Date: Fri, 6 Nov 2026 08:53:26 +0100
Subject: [PATCH] fix: a data lock wait names its holder

A refresh blocked on a data directory lock printed only that another
run held it, then waited with no limit. The line now names the lock
file and the lsof command that shows its holder, and for a refresh,
the generate_pack --no-refresh-data and --offline runs that read the
cache without waiting.
---
 scripts/refresh_data_dirs.py    | 20 +++++++++++++++++++-
 tests/test_refresh_data_dirs.py | 29 +++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/scripts/refresh_data_dirs.py b/scripts/refresh_data_dirs.py
index 7b74f7be1..31def57cc 100644
--- a/scripts/refresh_data_dirs.py
+++ b/scripts/refresh_data_dirs.py
@@ -71,6 +71,24 @@ def cache_lock(
     return _announced_lock(cache_dir.with_name(f".{cache_dir.name}.lock"), shared, cache_dir)
 
 
+def _wait_message(lock_path: Path, shared: bool, cache_dir: Path) -> str:
+    """Who to wait for, and, for a refresh, how not to.
+
+    The bare "another run holds it" named neither the lock nor its holder,
+    and the build waited with no limit.
+    """
+    message = (
+        f"waiting for {cache_dir}: another run holds {lock_path} "
+        f"(lsof {lock_path} names it)"
+    )
+    if not shared:
+        message += (
+            ". generate_pack.py --no-refresh-data, or --offline there and in"
+            " pipeline.py, builds from the cache as it is without waiting"
+        )
+    return message
+
+
 @contextlib.contextmanager
 def _announced_lock(lock_path: Path, shared: bool, cache_dir: Path) -> Iterator[None]:
     """file_lock, saying so when it has to wait.
@@ -90,7 +108,7 @@ def _announced_lock(lock_path: Path, shared: bool, cache_dir: Path) -> Iterator[
         try:
             fcntl.flock(handle, mode | fcntl.LOCK_NB)
         except BlockingIOError:
-            log.warning("waiting for %s: another run holds it", cache_dir)
+            log.warning(_wait_message(lock_path, shared, cache_dir))
             fcntl.flock(handle, mode)
         try:
             yield
diff --git a/tests/test_refresh_data_dirs.py b/tests/test_refresh_data_dirs.py
index 42f21d122..dc38143ac 100644
--- a/tests/test_refresh_data_dirs.py
+++ b/tests/test_refresh_data_dirs.py
@@ -118,6 +118,35 @@ class RefreshConcurrency(unittest.TestCase):
         self.assertTrue(finished, "the refresh waited for a reader")
         self.assertEqual(result, [False])
 
+    def test_a_waiting_refresh_names_the_lock_and_the_way_around(self):
+        """"waiting for data/dolphin-sys: another run holds it" named
+        neither the lock nor its holder, and the build waited unbounded."""
+        import fcntl  # noqa: PLC0415
+
+        self.cache.mkdir(parents=True)
+        lock_path = self.cache.with_name(".pak.lock")
+        entered = threading.Event()
+
+        def refresh() -> None:
+            with rdd.cache_lock(self.cache):
+                entered.set()
+
+        with open(lock_path, "a") as reader, self.assertLogs(rdd.log, "WARNING") as logs:
+            fcntl.flock(reader, fcntl.LOCK_SH)
+            worker = threading.Thread(target=refresh, daemon=True)
+            worker.start()
+            for _ in range(200):
+                if logs.records:
+                    break
+                worker.join(timeout=0.05)
+            self.assertFalse(entered.is_set())
+            fcntl.flock(reader, fcntl.LOCK_UN)
+            worker.join(timeout=20)
+        self.assertTrue(entered.is_set())
+        message = logs.records[0].getMessage()
+        self.assertIn(f"lsof {lock_path}", message)
+        self.assertIn("--no-refresh-data", message)
+
     def test_version_file_is_never_left_truncated(self):
         Path(self.versions).parent.mkdir(parents=True)
         Path(self.versions).write_text(json.dumps({"keep": {"sha": "1"}}))