Expand file lock scope to resolve concurrency issues during downloads (#3063)

* Fix race conditions on symlink-less filesystems by extending lock coverage

* Update src/huggingface_hub/file_download.py

Co-authored-by: célina <hanouticelina@gmail.com>

* move normalization of lock_path before mkdir

* Apply style fixes

---------

Co-authored-by: célina <hanouticelina@gmail.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: 3 people

src/huggingface_hub/file_download.py

@@ -1130,16 +1130,6 @@ def _hf_hub_download_to_cache_dir(
     # In that case store a ref.
     _cache_commit_hash_for_specific_revision(storage_folder, revision, commit_hash)
 
-    # If file already exists, return it (except if force_download=True)
-    if not force_download:
-        if os.path.exists(pointer_path):
-            return pointer_path
-
-        if os.path.exists(blob_path):
-            # we have the blob already, but not the pointer
-            _create_symlink(blob_path, pointer_path, new_blob=False)
-            return pointer_path
-
     # Prevent parallel downloads of the same file with a lock.
     # etag could be duplicated across repos,
     lock_path = os.path.join(locks_dir, repo_folder_name(repo_id=repo_id, repo_type=repo_type), f"{etag}.lock")
@@ -1152,9 +1142,21 @@ def _hf_hub_download_to_cache_dir(
     if os.name == "nt" and len(os.path.abspath(blob_path)) > 255:
         blob_path = "\\\\?\\" + os.path.abspath(blob_path)
 
+    Path(lock_path).parent.mkdir(parents=True, exist_ok=True)
+
+    # pointer already exists -> immediate return
+    if not force_download and os.path.exists(pointer_path):
+        return pointer_path
+
+    # Blob exists but pointer must be (safely) created -> take the lock
+    if not force_download and os.path.exists(blob_path):
+        with WeakFileLock(lock_path):
+            if not os.path.exists(pointer_path):
+                _create_symlink(blob_path, pointer_path, new_blob=False)
+            return pointer_path
+
     # Local file doesn't exist or etag isn't a match => retrieve file from remote (or cache)
 
-    Path(lock_path).parent.mkdir(parents=True, exist_ok=True)
     with WeakFileLock(lock_path):
         _download_to_tmp_and_move(
             incomplete_path=Path(blob_path + ".incomplete"),