From dbe1fbab1e7d46258b20382d1587d18e915a5fee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Mon, 3 Jun 2019 17:18:41 +0200 Subject: [PATCH 5/5] nar: Really lock store files. Previously, 'lock-store-file' would immediately close the file descriptor of the '.lock' file, and thus it would immediately release the lock. * guix/nar.scm (lock-store-file, unlock-store-file): Remove. (finalize-store-file): Use 'lock-file' and 'unlock-file' instead. --- guix/nar.scm | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/guix/nar.scm b/guix/nar.scm index 8894f10d2b..29636aa0f8 100644 --- a/guix/nar.scm +++ b/guix/nar.scm @@ -1,5 +1,5 @@ ;;; GNU Guix --- Functional package management for GNU -;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2018 Ludovic Courtès +;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2018, 2019 Ludovic Courtès ;;; Copyright © 2014 Mark H Weaver ;;; ;;; This file is part of GNU Guix. @@ -76,16 +76,6 @@ ;; most of the daemon is in Scheme :-)). But note that we do use a couple of ;; RPCs for functionality not available otherwise, like 'valid-path?'. -(define (lock-store-file file) - "Acquire exclusive access to FILE, a store file." - (call-with-output-file (string-append file ".lock") - (cut fcntl-flock <> 'write-lock))) - -(define (unlock-store-file file) - "Release access to FILE." - (call-with-input-file (string-append file ".lock") - (cut fcntl-flock <> 'unlock))) - (define* (finalize-store-file source target #:key (references '()) deriver (lock? #t)) "Rename SOURCE to TARGET and register TARGET as a valid store item, with @@ -94,25 +84,25 @@ before attempting to register it; otherwise, assume TARGET's locks are already held." (with-database %default-database-file db (unless (path-id db target) - (when lock? - (lock-store-file target)) + (let ((lock (and lock? + (lock-file (string-append target ".lock"))))) - (unless (path-id db target) - ;; If FILE already exists, delete it (it's invalid anyway.) - (when (file-exists? target) - (delete-file-recursively target)) + (unless (path-id db target) + ;; If FILE already exists, delete it (it's invalid anyway.) + (when (file-exists? target) + (delete-file-recursively target)) - ;; Install the new TARGET. - (rename-file source target) + ;; Install the new TARGET. + (rename-file source target) - ;; Register TARGET. As a side effect, it resets the timestamps of all - ;; its files, recursively, and runs a deduplication pass. - (register-path target - #:references references - #:deriver deriver)) + ;; Register TARGET. As a side effect, it resets the timestamps of all + ;; its files, recursively, and runs a deduplication pass. + (register-path target + #:references references + #:deriver deriver)) - (when lock? - (unlock-store-file target))))) + (when lock? + (unlock-file lock)))))) (define (temporary-store-file) "Return the file name of a temporary file created in the store." -- 2.21.0