* [bug#66387] [PATCH] home: services: Fix race condition when detecting first login
@ 2023-10-07 11:59 Carlo Zancanaro
2023-10-09 13:08 ` Carlo Zancanaro
0 siblings, 1 reply; 5+ messages in thread
From: Carlo Zancanaro @ 2023-10-07 11:59 UTC (permalink / raw)
To: 66387
* 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@trop.in>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2022-2023 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2023 Carlo Zancanaro <carlo@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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [bug#66387] [PATCH] home: services: Fix race condition when detecting first login
2023-10-07 11:59 [bug#66387] [PATCH] home: services: Fix race condition when detecting first login Carlo Zancanaro
@ 2023-10-09 13:08 ` Carlo Zancanaro
2023-10-11 11:57 ` Carlo Zancanaro
0 siblings, 1 reply; 5+ messages in thread
From: Carlo Zancanaro @ 2023-10-09 13:08 UTC (permalink / raw)
Cc: 66387
On Sat, Oct 07 2023, Carlo Zancanaro wrote:
> @@ -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)
Whoops, just noticed this should be _, not (e). It still works,
because it crashes the script which stops the gexps from running,
but writes a message to the console. I'll send an updated patch
tomorrow.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug#66387] [PATCH] home: services: Fix race condition when detecting first login
2023-10-09 13:08 ` Carlo Zancanaro
@ 2023-10-11 11:57 ` Carlo Zancanaro
2023-10-19 20:29 ` bug#66387: " Ludovic Courtès
0 siblings, 1 reply; 5+ messages in thread
From: Carlo Zancanaro @ 2023-10-11 11:57 UTC (permalink / raw)
To: 66387
* gnu/home/services.scm (compute-on-first-login-script): Use open to
atomically check whether a file exists and create it if not.
---
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..7137925b30 100644
--- a/gnu/home/services.scm
+++ b/gnu/home/services.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2021-2023 Andrew Tropin <andrew@trop.in>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2022-2023 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2023 Carlo Zancanaro <carlo@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 _
+ #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: 9ad9113fc238ee8de5191a5e15b5153fd149e9fa
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* bug#66387: [PATCH] home: services: Fix race condition when detecting first login
2023-10-11 11:57 ` Carlo Zancanaro
@ 2023-10-19 20:29 ` Ludovic Courtès
2023-10-20 6:55 ` [bug#66387] " Andrew Tropin
0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2023-10-19 20:29 UTC (permalink / raw)
To: Carlo Zancanaro; +Cc: 66387-done, Andrew Tropin, paren
[-- Attachment #1: Type: text/plain, Size: 344 bytes --]
Hi Carlo,
Carlo Zancanaro <carlo@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’.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 966 bytes --]
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [bug#66387] [PATCH] home: services: Fix race condition when detecting first login
2023-10-19 20:29 ` bug#66387: " Ludovic Courtès
@ 2023-10-20 6:55 ` Andrew Tropin
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Tropin @ 2023-10-20 6:55 UTC (permalink / raw)
To: Ludovic Courtès, Carlo Zancanaro; +Cc: 66387-done, paren
[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]
On 2023-10-19 22:29, Ludovic Courtès wrote:
> Hi Carlo,
>
> Carlo Zancanaro <carlo@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’.
>
> 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)
Very nice trick! Carol, Ludo, Thank you for the fix!
> (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
>
--
Best regards,
Andrew Tropin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-20 6:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-07 11:59 [bug#66387] [PATCH] home: services: Fix race condition when detecting first login Carlo Zancanaro
2023-10-09 13:08 ` Carlo Zancanaro
2023-10-11 11:57 ` Carlo Zancanaro
2023-10-19 20:29 ` bug#66387: " Ludovic Courtès
2023-10-20 6:55 ` [bug#66387] " Andrew Tropin
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.