GNU bug report logs - #25018
GC incorrectly removes the temporary root file of the calling process

Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.

Package: guix; Severity: important; Reported by: ludo@HIDDEN (Ludovic Courtès); dated Thu, 24 Nov 2016 14:08:01 UTC; Maintainer for guix is bug-guix@HIDDEN.
Severity set to 'important' from 'normal' Request was from ludo@HIDDEN (Ludovic Courtès) to control <at> debbugs.gnu.org. Full text available.

Message received at submit <at> debbugs.gnu.org:


Received: (at submit) by debbugs.gnu.org; 24 Nov 2016 14:07:57 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Thu Nov 24 09:07:57 2016
Received: from localhost ([127.0.0.1]:40028 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1c9ugv-00021s-3K
	for submit <at> debbugs.gnu.org; Thu, 24 Nov 2016 09:07:57 -0500
Received: from eggs.gnu.org ([208.118.235.92]:36224)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <ludo@HIDDEN>) id 1c9ugr-00021a-Hm
 for submit <at> debbugs.gnu.org; Thu, 24 Nov 2016 09:07:55 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <ludo@HIDDEN>) id 1c9ugl-00055f-Dk
 for submit <at> debbugs.gnu.org; Thu, 24 Nov 2016 09:07:48 -0500
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org
X-Spam-Level: 
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD
 autolearn=disabled version=3.3.2
Received: from lists.gnu.org ([2001:4830:134:3::11]:34537)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)
 (Exim 4.71) (envelope-from <ludo@HIDDEN>) id 1c9ugl-00055R-A8
 for submit <at> debbugs.gnu.org; Thu, 24 Nov 2016 09:07:47 -0500
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55404)
 by lists.gnu.org with esmtp (Exim 4.71)
 (envelope-from <ludo@HIDDEN>) id 1c9ugh-0000fr-0d
 for bug-guix@HIDDEN; Thu, 24 Nov 2016 09:07:47 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <ludo@HIDDEN>) id 1c9ugd-00051i-RU
 for bug-guix@HIDDEN; Thu, 24 Nov 2016 09:07:42 -0500
Received: from fencepost.gnu.org ([2001:4830:134:3::e]:45485)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <ludo@HIDDEN>)
 id 1c9ugR-0004x4-GQ; Thu, 24 Nov 2016 09:07:27 -0500
Received: from pluto.bordeaux.inria.fr ([193.50.110.57]:57750 helo=pluto)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <ludo@HIDDEN>)
 id 1c9ugQ-0001ud-Ud; Thu, 24 Nov 2016 09:07:27 -0500
From: ludo@HIDDEN (Ludovic =?utf-8?Q?Court=C3=A8s?=)
To: Eelco Dolstra <eelco.dolstra@HIDDEN>
Subject: GC incorrectly removes the temporary root file of the calling process
X-URL: http://www.fdn.fr/~lcourtes/
X-Revolutionary-Date: 4 Frimaire an 225 de la =?utf-8?Q?R=C3=A9volution?=
X-PGP-Key-ID: 0x090B11993D9AEBB5
X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc
X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4  0CFB 090B 1199 3D9A EBB5
X-OS: x86_64-unknown-linux-gnu
Date: Thu, 24 Nov 2016 15:07:24 +0100
Message-ID: <87d1hl55ur.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x
X-Received-From: 2001:4830:134:3::11
X-Spam-Score: -8.0 (--------)
X-Debbugs-Envelope-To: submit
Cc: Hartmut Goebel <h.goebel@HIDDEN>, bug-guix@HIDDEN
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -8.0 (--------)

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Hello,

The =E2=80=98readTempRoots=E2=80=99 function in gc.cc has this:

        /* Try to acquire a write lock without blocking.  This can
           only succeed if the owning process has died.  In that case
           we don't care about its temporary roots. */
        if (lockFile(*fd, ltWrite, false)) {
            printMsg(lvlError, format("removing stale temporary roots file =
`%1%'") % path);
            unlink(path.c_str());

There=E2=80=99s a thinko here: locking the file also succeeds when the lock=
 is
already held by the calling process.

In that case, this code ends up removing the temporary root file of
calling process, which is bad.  Here=E2=80=99s a sample session:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix)
scheme@(guile-user)> (define s (open-connection))
scheme@(guile-user)> (current-build-output-port (current-error-port))
$2 =3D #<output: file /dev/pts/9>
scheme@(guile-user)> (set-build-options s #:verbosity 10)
$3 =3D #t
scheme@(guile-user)> (add-text-to-store s "foo" "bar!")
acquiring global GC lock `/var/guix/gc.lock'
acquiring read lock on `/var/guix/temproots/4259'
acquiring write lock on `/var/guix/temproots/4259'
downgrading to read lock on `/var/guix/temproots/4259'
locking path `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo'
lock acquired on `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo.lock'
`/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo' has hash `c756ef12a70bad1=
0c9ac276ecd1213ea7cc3a2e6c462ba47e4f9a88756a055d0'
lock released on `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo.lock'
$4 =3D "/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo"
scheme@(guile-user)> (delete-paths s (list $4))
acquiring global GC lock `/var/guix/gc.lock'
finding garbage collector roots...
executing `/gnu/store/l99rkv2713nl53kr3gn4akinvifsx19h-guix-0.11.0-3.7ca3/l=
ibexec/guix/list-runtime-roots' to find additional roots
[=E2=80=A6]
reading temporary root file `/var/guix/temproots/4259'
removing stale temporary roots file `/var/guix/temproots/4259'
[=E2=80=A6]
considering whether to delete `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-=
foo'
|   invalidating path `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo'
|   deleting `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo'
|   recursively deleting path `/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-=
foo'
|   |   /gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo
deleting `/gnu/store/trash'
recursively deleting path `/gnu/store/trash'
|   /gnu/store/trash
deleting unused links...
deleting unused link `/gnu/store/.links/1l2ml1b8ga7rwi3vlqn4wsic6z7a2c9csvi=
7mk4i1b8blw9fymn7'
note: currently hard linking saves 6699.22 MiB
$5 =3D ("/gnu/store/0siy93lggjw7sfdg8gsvrzafaa974h2d-foo")
$6 =3D 4096
--8<---------------cut here---------------end--------------->8---

Notice the =E2=80=9Cremoving stale temporary roots file=E2=80=9D message.

Eelco: shouldn=E2=80=99t it be changed along the lines of the attached path?

Thanks,
Ludo=E2=80=99.


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline

diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc
index 72eff52..d92388f 100644
--- a/nix/libstore/gc.cc
+++ b/nix/libstore/gc.cc
@@ -2,6 +2,7 @@
 #include "misc.hh"
 #include "local-store.hh"
 
+#include <string>
 #include <functional>
 #include <queue>
 #include <algorithm>
@@ -225,10 +226,10 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
         //FDPtr fd(new AutoCloseFD(openLockFile(path, false)));
         //if (*fd == -1) continue;
 
-        /* Try to acquire a write lock without blocking.  This can
-           only succeed if the owning process has died.  In that case
-           we don't care about its temporary roots. */
-        if (lockFile(*fd, ltWrite, false)) {
+        /* Try to acquire a write lock without blocking.  This can only
+           succeed if the owning process has died, in which case we don't care
+           about its temporary roots, or if we are the owning process.  */
+        if (i.name != std::to_string(getpid()) && lockFile(*fd, ltWrite, false)) {
             printMsg(lvlError, format("removing stale temporary roots file `%1%'") % path);
             unlink(path.c_str());
             writeFull(*fd, "d");

--=-=-=--




Acknowledgement sent to ludo@HIDDEN (Ludovic Courtès):
New bug report received and forwarded. Copy sent to bug-guix@HIDDEN. Full text available.
Report forwarded to bug-guix@HIDDEN:
bug#25018; Package guix. Full text available.
Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.
Last modified: Mon, 23 Jan 2017 22:30:02 UTC

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997 nCipher Corporation Ltd, 1994-97 Ian Jackson.