WIP another windows test

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

Author: jku

tests/test_updater_ng.py

@@ -362,8 +362,6 @@ def test_parallel_updaters(self) -> None:
         # Refresh many updaters in parallel many times, using the same local metadata cache.
         # This should reveal race conditions.
 
-        # windows on GitHub actions *will* fail with "[Errno 36] Resource deadlock avoided"
-        # if iterations is in the hundreds
         iterations = 50
         process_count = 10
 
@@ -396,7 +394,6 @@ def test_parallel_updaters(self) -> None:
                 errout += "Parallel Refresh script failed:"
                 errout += f"\nprocess stdout: \n{stdout.decode()}"
                 errout += f"\nprocess stderr: \n{stderr.decode()}"
-
         if errout:
             self.fail(
                 f"One or more scripts failed parallel refresh test:\n{errout}"

tox.ini

@@ -29,7 +29,7 @@ commands_pre =
     python3 -m pip install --force-reinstall git+https://github.com/secure-systems-lab/securesystemslib.git@main#egg=securesystemslib[crypto]
 
 commands =
-    python3 -m coverage run -m unittest
+    python3 -m coverage run -m unittest -v
     python3 -m coverage report -m
 
 [testenv:lint]

tuf/ngclient/updater.py

@@ -58,6 +58,8 @@
 import os
 import shutil
 import tempfile
+import time
+import traceback
 from pathlib import Path
 from typing import IO, TYPE_CHECKING, cast
 from urllib import parse
@@ -79,21 +81,44 @@
     # advisory file locking for posix
     import fcntl
 
-    def _lock_file(f: IO) -> None:
-        if f.writable():
+    @contextlib.contextmanager
+    def _lock_file(path: str) -> Iterator[IO]:
+        with open(path, "wb") as f:
             fcntl.lockf(f, fcntl.LOCK_EX)
+            yield f
 
 except ModuleNotFoundError:
     # Windows file locking
     import msvcrt
 
-    def _lock_file(f: IO) -> None:
-        # On Windows we lock a byte range and file must not be empty
-        f.write(b"\0")
-        f.flush()
-        f.seek(0)
+    @contextlib.contextmanager
+    def _lock_file(path: str) -> Iterator[IO]:
+        err = None
+        locked = False
+        for i in range(100):
+            try:
+                with open(path, "wb") as f:
+                    # file must not be empty
+                    f.write(b"\0")
+                    f.flush()
+                    f.seek(0)
+                    msvcrt.locking(f.fileno(), msvcrt.LK_LOCK, 1)
+                    locked = True
+                    yield f
+                    return
+            except OSError as e:
+                if locked:
+                    # yield raised
+                    raise e
+                err = e
+                logger.warning(
+                    "Unsuccessful lock attempt %d for %s: %s", i, path, e
+                )
+                logger.warning(traceback.format_exc())
+                time.sleep(0.3)
 
-        msvcrt.locking(f.fileno(), msvcrt.LK_LOCK, 1)
+        if err:
+            raise err
 
 
 class Updater:
@@ -171,9 +196,9 @@ def _lock_metadata(self) -> Iterator[None]:
         # Ensure the whole metadata directory structure exists
         rootdir = Path(self._dir, "root_history")
         rootdir.mkdir(exist_ok=True, parents=True)
+
         logger.debug("Getting metadata lock...")
-        with open(os.path.join(self._dir, ".lock"), "wb") as f:
-            _lock_file(f)
+        with _lock_file(os.path.join(self._dir, ".lock")):
             yield
         logger.debug("Released metadata lock")
 
@@ -336,8 +361,7 @@ def download_target(
             targetinfo.verify_length_and_hashes(target_file)
 
             target_file.seek(0)
-            with open(filepath, "wb") as destination_file:
-                _lock_file(destination_file)
+            with _lock_file(filepath) as destination_file:
                 shutil.copyfileobj(target_file, destination_file)
 
         logger.debug("Downloaded target %s", targetinfo.path)