diff --git a/Library/Homebrew/lock_file.rb b/Library/Homebrew/lock_file.rb index cad31b80ae..a49a6bb0a4 100644 --- a/Library/Homebrew/lock_file.rb +++ b/Library/Homebrew/lock_file.rb @@ -5,6 +5,9 @@ require "fcntl" # A lock file to prevent multiple Homebrew processes from modifying the same path. class LockFile + class OpenFileChangedOnDisk < RuntimeError; end + private_constant :OpenFileChangedOnDisk + attr_reader :path sig { params(type: Symbol, locked_path: Pathname).void } @@ -18,10 +21,44 @@ class LockFile sig { void } def lock ignore_interrupts do - create_lockfile + next if @lockfile.present? - next if @lockfile.flock(File::LOCK_EX | File::LOCK_NB) + path.dirname.mkpath + begin + lockfile = begin + path.open(File::RDWR | File::CREAT) + rescue Errno::EMFILE + odie "The maximum number of open files on this system has been reached. " \ + "Use `ulimit -n` to increase this limit." + end + lockfile.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) + + if lockfile.flock(File::LOCK_EX | File::LOCK_NB) + # This prevents a race condition in case the file we locked doesn't exist on disk anymore, e.g.: + # + # 1. Process A creates and opens the file. + # 2. Process A locks the file. + # 3. Process B opens the file. + # 4. Process A unlinks the file. + # 5. Process A unlocks the file. + # 6. Process B locks the file. + # 7. Process C creates and opens the file. + # 8. Process C locks the file. + # 9. Process B and C hold locks to files with different inode numbers. 💥 + if !path.exist? || lockfile.stat.ino != path.stat.ino + lockfile.close + raise OpenFileChangedOnDisk + end + + @lockfile = lockfile + next + end + rescue OpenFileChangedOnDisk + retry + end + + lockfile.close raise OperationInProgressError, @locked_path end end @@ -29,15 +66,12 @@ class LockFile sig { params(unlink: T::Boolean).void } def unlock(unlink: false) ignore_interrupts do - next if @lockfile.nil? || @lockfile.closed? + next if @lockfile.nil? + @path.unlink if unlink @lockfile.flock(File::LOCK_UN) @lockfile.close - - if unlink - @path.unlink if @path.exist? - @lockfile = nil - end + @lockfile = nil end end @@ -47,21 +81,6 @@ class LockFile ensure unlock end - - private - - def create_lockfile - return if @lockfile.present? && !@lockfile.closed? - - @path.dirname.mkpath - - begin - @lockfile = @path.open(File::RDWR | File::CREAT) - rescue Errno::EMFILE - odie "The maximum number of open files on this system has been reached. Use `ulimit -n` to increase this limit." - end - @lockfile.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) - end end # A lock file for a formula.