Repository navigation
Conversation
| // Cross-process exclusive lock (flock on POSIX, LockFileEx on Windows). | ||
| // The constructor only stores the path: acquire() opens the lock file and | ||
| // waits out contention, so code that never acquires the lock (pure cache | ||
| // reads) works on a read-only cache. |
There was a problem hiding this comment.
need to clean up comments per AGENTS.md
There was a problem hiding this comment.
I literally asked to add some comments and to be verbose (mostly to help the LLM itself) because locking correctly is actually hard. The model often failed without being driven correctly...
But maybe it became too verbose at some points. I'll do a pass
There was a problem hiding this comment.
maybe we don't need a new cpp/h file for it, better to just move it to download.cpp as a private struct
There was a problem hiding this comment.
The file locking mechanism is generic enough and already used in two places: download and hf-cache (which is why it was moved out of download). Also it could be useful elsewhere, like modelscope, docker or when not downloading at all.
There was a problem hiding this comment.
Also it could be useful elsewhere, like modelscope, docker or when not downloading at all.
other model sources must eventually call download.h to download the actual file, so I think it should still part of the download.h
when not downloading at all.
when not downloading at all, we never need to lock anything because we should never write anything
cf7ddeb to
0669e78
Compare
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
0669e78 to
09e13f3
Compare
This branch has not been deployed
2 participants
Overview
This PR adds cross-process locking to the download path, so multiple processes (including the
hftool) can download the same file without corrupting it.Additional information
Supersedes #28803
Requirements