fix: use a portable file lock for telemetry installation ids

packages/shared-python/shared/services/telemetry/identity.py

@@ -2,11 +2,22 @@
 
 from __future__ import annotations
 
-import fcntl
 import os
+import sys
+import threading
+from collections.abc import Iterator
+from contextlib import contextmanager
 from pathlib import Path
+from typing import IO
 from uuid import UUID, uuid4
 
+if sys.platform == "win32":
+    import msvcrt
+else:
+    import fcntl
+
+_PROCESS_LOCK = threading.Lock()
+
 
 def get_or_create_installation_id(
     *,
@@ -27,8 +38,7 @@ def get_or_create_installation_id(
     lock_path = installation_id_path.with_suffix(f"{installation_id_path.suffix}.lock")
 
     with lock_path.open("a+", encoding="utf-8") as lock_file:
-        fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
-        try:
+        with _exclusive_file_lock(lock_file):
             existing_installation_id = _read_valid_installation_id(
                 installation_id_path
             )
@@ -41,8 +51,46 @@ def get_or_create_installation_id(
                 generated_installation_id,
             )
             return generated_installation_id
+
+
+@contextmanager
+def _exclusive_file_lock(lock_file: IO[str]) -> Iterator[None]:
+    """Cross-process file lock plus an in-process mutex.
+
+    POSIX uses ``fcntl.flock``. Windows ``msvcrt.locking`` is per-process, so a
+    thread lock is required for concurrent callers in the same interpreter.
+    """
+    with _PROCESS_LOCK:
+        _lock_exclusive(lock_file)
+        try:
+            yield
         finally:
-            fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
+            _unlock_exclusive(lock_file)
+
+
+def _lock_exclusive(lock_file: IO[str]) -> None:
+    if sys.platform == "win32":
+        _ensure_lock_byte(lock_file)
+        lock_file.seek(0)
+        msvcrt.locking(lock_file.fileno(), msvcrt.LK_LOCK, 1)
+        return
+    fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
+
+
+def _unlock_exclusive(lock_file: IO[str]) -> None:
+    if sys.platform == "win32":
+        lock_file.seek(0)
+        msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1)
+        return
+    fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
+
+
+def _ensure_lock_byte(lock_file: IO[str]) -> None:
+    lock_file.seek(0, os.SEEK_END)
+    if lock_file.tell() == 0:
+        lock_file.write("0")
+        lock_file.flush()
+        os.fsync(lock_file.fileno())
 
 
 def _read_valid_installation_id(installation_id_path: Path) -> str:

packages/shared-python/shared/tests/test_telemetry_identity.py

@@ -0,0 +1,100 @@
+"""Unit tests for the stable self-hosted telemetry installation id."""
+
+from __future__ import annotations
+
+import sys
+import threading
+from concurrent.futures import ThreadPoolExecutor
+from pathlib import Path
+from uuid import UUID
+
+import pytest
+
+from shared.services.telemetry.identity import get_or_create_installation_id
+
+
+def test_explicit_installation_id_wins(tmp_path: Path) -> None:
+    installation_id_path = tmp_path / "telemetry-installation-id"
+    explicit_installation_id = "550e8400-e29b-41d4-a716-446655440000"
+
+    installation_id = get_or_create_installation_id(
+        explicit_installation_id=explicit_installation_id,
+        installation_id_path=installation_id_path,
+    )
+
+    assert installation_id == explicit_installation_id
+    assert not installation_id_path.exists()
+
+
+def test_explicit_installation_id_must_be_uuid(tmp_path: Path) -> None:
+    with pytest.raises(ValueError, match="must be a UUID"):
+        get_or_create_installation_id(
+            explicit_installation_id="not-a-uuid",
+            installation_id_path=tmp_path / "telemetry-installation-id",
+        )
+
+
+def test_missing_file_generates_uuid(tmp_path: Path) -> None:
+    installation_id_path = tmp_path / "telemetry-installation-id"
+
+    installation_id = get_or_create_installation_id(
+        explicit_installation_id="",
+        installation_id_path=installation_id_path,
+    )
+
+    UUID(installation_id)
+    assert installation_id_path.read_text(encoding="utf-8").strip() == installation_id
+
+
+def test_generated_id_is_stable_across_calls(tmp_path: Path) -> None:
+    installation_id_path = tmp_path / "telemetry-installation-id"
+
+    first = get_or_create_installation_id(
+        explicit_installation_id="",
+        installation_id_path=installation_id_path,
+    )
+    second = get_or_create_installation_id(
+        explicit_installation_id="",
+        installation_id_path=installation_id_path,
+    )
+
+    assert first == second
+
+
+def test_invalid_existing_file_is_replaced(tmp_path: Path) -> None:
+    installation_id_path = tmp_path / "telemetry-installation-id"
+    installation_id_path.write_text("not-a-uuid\n", encoding="utf-8")
+
+    installation_id = get_or_create_installation_id(
+        explicit_installation_id="",
+        installation_id_path=installation_id_path,
+    )
+
+    UUID(installation_id)
+    assert installation_id_path.read_text(encoding="utf-8").strip() == installation_id
+
+
+def test_concurrent_calls_return_the_same_id(tmp_path: Path) -> None:
+    installation_id_path = tmp_path / "telemetry-installation-id"
+    barrier = threading.Barrier(8)
+
+    def _resolve() -> str:
+        barrier.wait(timeout=5)
+        return get_or_create_installation_id(
+            explicit_installation_id="",
+            installation_id_path=installation_id_path,
+        )
+
+    with ThreadPoolExecutor(max_workers=8) as executor:
+        results = list(executor.map(lambda _: _resolve(), range(8)))
+
+    assert len(set(results)) == 1
+    assert installation_id_path.read_text(encoding="utf-8").strip() == results[0]
+
+
+@pytest.mark.skipif(sys.platform != "win32", reason="fcntl is POSIX-only")
+def test_identity_import_does_not_require_fcntl() -> None:
+    assert "fcntl" not in sys.modules
+    from shared.services.telemetry import identity as identity_module
+
+    assert identity_module.get_or_create_installation_id is get_or_create_installation_id