[PATCH] D78896: [Support] Add file lock/unlock functions

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 2 inline comments as done.
sepavloff added a comment.

In D78896#2079169 <https://reviews.llvm.org/D78896#2079169>, @labath wrote:

> It's a pitty about flock&solaris. It's a much saner api, and a lot closer to the windows implementation.
>
> I guess we have to stick to fcntl then. The new version looks good. I just think it would be good to acknowledge the weirdness of the flock api in the method documentation. Maybe something like "Care should be taken when using this function in a multithreaded context, as it may not prevent other threads in the same process from obtaining a lock on the same file, even if they are using a different file descriptor."


Added the note in the description of the function in FileSystem.h.



================
Comment at: llvm/lib/Support/Unix/Path.inc:1066
+      return std::error_code(Error, std::generic_category());
+    usleep(1000);
+  } while (std::chrono::steady_clock::now() < End);
----------------
MaskRay wrote:
> labath wrote:
> > MaskRay wrote:
> > > I feel uneasy with usleep(1000). Why is it needed?
> > That's because the os provides no mechanism to wait for a lock to become available for a given amount of time (well.... one could use `SIGALRM`s to achieve that, but those come with problems of their own..)
> > 
> > Using exponential backoff for this might be a good idea, but I also think that can wait until this becomes a problem.
> The standard `try_*` (std::mutex::try_lock, pthread_mutex_trylock, etc) return immediately upon a failure. The backoff strategy should be done by the application to avoid the misnomer.
If the function is called as `tryLockFile(FD)` it behaves exactly as the standard `try_*` functions, it returns immediately.

> The backoff strategy should be done by the application to avoid the misnomer.

This use case is often and even standard library provides functions that attempt to do an operation in specified time (*_for). As the only expected usage of file locks is writing to log file by concurrent processes and it requires repeating the operation if unsuccessful, so integrations of the wait loop into the service function seems natural.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78896/new/

https://reviews.llvm.org/D78896