GNU bug report logs - #66387
[PATCH] home: services: Fix race condition when detecting first login

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#66387: closed ([PATCH] home: services: Fix race condition
 when detecting first login)
Date: Thu, 19 Oct 2023 20:31:01 +0000

[Message part 1 (text/plain, inline)]

Your message dated Thu, 19 Oct 2023 22:29:34 +0200
with message-id <87v8b2xsnl.fsf <at> gnu.org>
and subject line Re: [bug#66387] [PATCH] home: services: Fix race condition when detecting first login
has caused the debbugs.gnu.org bug report #66387,
regarding [PATCH] home: services: Fix race condition when detecting first login
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
66387: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66387
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Carlo Zancanaro <carlo <at> zancanaro.id.au>
To: guix-patches <at> gnu.org
Subject: [PATCH] home: services: Fix race condition when detecting first login
Date: Sat,  7 Oct 2023 22:59:05 +1100

* gnu/home/services.scm (compute-on-first-login-script): Use open to
atomically check whether a file exists and create it if not.
---

I run Guix Home on NixOS with SDDM as my display manager. When I log in, I find that there are two shepherd processes running. Looking at the on-first-login script I noticed a race condition that I suspect was causing the issue.

I believe the "open" incantation I have used is atomic, except on NFS, so this should ensure that the gexps are only run once. I have confirmed that this patch fixes my problem. With this patch, when I login I only have a single shepherd process.

 gnu/home/services.scm | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gnu/home/services.scm b/gnu/home/services.scm
index 8d53f2f4d3..95ef66e091 100644
--- a/gnu/home/services.scm
+++ b/gnu/home/services.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2021-2023 Andrew Tropin <andrew <at> trop.in>
 ;;; Copyright © 2021 Xinglu Chen <public <at> yoctocell.xyz>
 ;;; Copyright © 2022-2023 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2023 Carlo Zancanaro <carlo <at> zancanaro.id.au>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -412,20 +413,29 @@ (define (compute-on-first-login-script _ gexps)
      #~(begin
          (use-modules (guix i18n)
                       (guix diagnostics))
+
+       (define (claim-first-run file-name)
+         (catch #t
+           (lambda ()
+             ;; This incantation will raise an error if the file at
+             ;; flag-file-path already exists, and will create it otherwise.
+             (close (open file-name (logior O_CREAT O_EXCL)))
+             #t)
+           (lambda (e)
+             #f)))
+
        #$%initialize-gettext
 
        (let* ((xdg-runtime-dir (or (getenv "XDG_RUNTIME_DIR")
                                    (format #f "/run/user/~a" (getuid))))
               (flag-file-path (string-append
-                               xdg-runtime-dir "/on-first-login-executed"))
-              (touch (lambda (file-name)
-                       (call-with-output-file file-name (const #t)))))
+                               xdg-runtime-dir "/on-first-login-executed")))
          ;; XDG_RUNTIME_DIR dissapears on logout, that means such trick
          ;; allows to launch on-first-login script on first login only
          ;; after complete logout/reboot.
          (if (file-exists? xdg-runtime-dir)
-             (unless (file-exists? flag-file-path)
-               (begin #$@gexps (touch flag-file-path)))
+             (when (claim-first-run flag-file-path)
+               #$@gexps)
              ;; TRANSLATORS: 'on-first-login' is the name of a service and
              ;; shouldn't be translated
              (warning (G_ "XDG_RUNTIME_DIR doesn't exists, on-first-login script

base-commit: b566e1a98a74d84d3978cffefd05295602c9445d
-- 
2.41.0

[Message part 3 (message/rfc822, inline)]

From: Ludovic Courtès <ludo <at> gnu.org>
To: Carlo Zancanaro <carlo <at> zancanaro.id.au>
Cc: 66387-done <at> debbugs.gnu.org, Andrew Tropin <andrew <at> trop.in>,
 paren <at> disroot.org
Subject: Re: [bug#66387] [PATCH] home: services: Fix race condition when
 detecting first login
Date: Thu, 19 Oct 2023 22:29:34 +0200

[Message part 4 (text/plain, inline)]

Hi Carlo,

Carlo Zancanaro <carlo <at> zancanaro.id.au> skribis:

> * gnu/home/services.scm (compute-on-first-login-script): Use open to
> atomically check whether a file exists and create it if not.

Good catch!

I made the following cosmetic changes (‘open-fdes’ is cheaper than
‘open’) and applied it.

Thanks!

Ludo’.

[Message part 5 (text/x-patch, inline)]

diff --git a/gnu/home/services.scm b/gnu/home/services.scm
index 7137925b30..651c068f79 100644
--- a/gnu/home/services.scm
+++ b/gnu/home/services.scm
@@ -414,15 +414,15 @@ (define (compute-on-first-login-script _ gexps)
          (use-modules (guix i18n)
                       (guix diagnostics))
 
-       (define (claim-first-run file-name)
+       (define (claim-first-run file)
          (catch #t
            (lambda ()
-             ;; This incantation will raise an error if the file at
-             ;; flag-file-path already exists, and will create it otherwise.
-             (close (open file-name (logior O_CREAT O_EXCL)))
+             ;; This incantation raises an error if FILE already exists, and
+             ;; creates it otherwise.
+             (close-fdes
+              (open-fdes file (logior O_CREAT O_EXCL O_CLOEXEC)))
              #t)
-           (lambda _
-             #f)))
+           (const #f)))
 
        #$%initialize-gettext
 

Previous Next

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