GNU bug report logs - #27476
Multi-threaded compilation of 'syntax-parameterize' forms crashes

Previous Next

Full log

Message #99 received at 27476 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Andy Wingo <wingo <at> igalia.com>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 27476 <at> debbugs.gnu.org
Subject: Re: bug#27476: libguile/memoize.c is not thread safe,
 so syntax parameter expansion is not thread-safe
Date: Wed, 06 Feb 2019 15:48:51 +0100

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

Hello Andy!

Since guix-core.drv is the best reproducer I have so far for this syntax
parameter crash, I modified (guix self) to print the name of the files
it’s compiling, and here’s the crash I got (on a 24-core machine):

--8<---------------cut here---------------start------------->8---
building /gnu/store/hf324mhj5607hh2izb01dzhwakmn8am8-guix-core.drv...
[ 39/ 78] loading...    100.0% of 39 filesbuilding "guix/config.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/monad-repl.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/store.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/utils.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/memoization.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/profiling.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/build/utils.scm"
[ 39/ 78] compiling...    0.0% of 39 filesbuilding "guix/build/syscalls.scm"
[ 40/ 78] compiling...    2.6% of 39 filesbuilding "guix/deprecation.scm"
[ 41/ 78] compiling...    5.1% of 39 filesbuilding "guix/i18n.scm"
[ 42/ 78] compiling...    7.7% of 39 filesbuilding "guix/serialization.scm"
[ 43/ 78] compiling...   10.3% of 39 filesbuilding "guix/combinators.scm"
[ 44/ 78] compiling...   12.8% of 39 filesbuilding "guix/monads.scm"
[ 45/ 78] compiling...   15.4% of 39 filesbuilding "guix/records.scm"
[ 46/ 78] compiling...   17.9% of 39 filesIn ice-9/psyntax.scm:
  2338:44 19 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./guix/monads.scm")) _ #<procedure build-let (src ids vars val-exps body-exp)> _ _ _)
  1679:45 18 (parse _ _ _ _ _ _ _)
In ice-9/boot-9.scm:
   222:17 17 (map1 (((("placeholder" placeholder) ("l-10a3c941d34314a1-4889" lexical . failure-10a3c941d34314a1-488a) ("placeholder" placeholder) ("placeholder" placeholder) ("l-10a3c9?" . #) ?) ?)))
In ice-9/psyntax.scm:
  1409:12 16 (_ _ _ #<syntax (#<syntax failure>)>)
  2338:44 15 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./guix/monads.scm")) (hygiene guix monads) #<procedure build-let (src ids vars val-exps body-exp)> _ _ ((#<syntax match-o?> ?)))
  1679:45 14 (parse _ _ _ _ _ _ _)
In ice-9/boot-9.scm:
   222:17 13 (map1 (((("l-10a3c941d34314a1-4894" macro . #<procedure 460ab80 at ice-9/eval.scm:333:13 (a)>) ("placeholder" placeholder) ("l-10a3c941d34314a1-488f" lexical . #) ("l-10?" . #) ?) . #)))
In ice-9/psyntax.scm:
  2338:44 12 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./guix/monads.scm")) (hygiene guix monads) #<procedure build-let (src ids vars val-exps body-exp)> _ _ ((#<syntax match-o?> ?)))
  1679:45 11 (parse _ _ _ _ _ _ _)
In ice-9/boot-9.scm:
   222:17 10 (map1 (((("l-10a3c941d34314a1-48b0" macro . #<procedure 40f9b40 at ice-9/eval.scm:333:13 (a)>) ("placeholder" placeholder) ("l-10a3c941d34314a1-48ac" lexical . #) ("l-10?" . #) ?) . #)))
In ice-9/psyntax.scm:
  2338:44  9 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./guix/monads.scm")) (hygiene guix monads) #<procedure build-let (src ids vars val-exps body-exp)> _ _ ((#<syntax match-d?> ?)))
  1612:33  8 (parse (((("placeholder" placeholder) ("l-10a3c941d34314a1-48c8" lexical . tail-10a3c941d34314a1-48c9) ("l-10a3c941d34314a1-48b0" macro . #<procedure 40f9b40 at ice-9/eval.?>) ?) . #)) ?)
  1348:32  7 (syntax-type (>>= (mproc head result) (lambda (result) (loop tail result))) (("placeholder" placeholder) ("l-10a3c941d34314a1-48c8" lexical . tail-10a3c941d34314a1-48c9) ("l-?" . #) ?) ?)
  1559:32  6 (expand-macro #<procedure 17a5ba0 at ice-9/eval.scm:333:13 (a)> _ _ _ _ _ _)
In ice-9/boot-9.scm:
   752:25  5 (dispatch-exception _ _ _)
   751:25  4 (dispatch-exception 1 syntax-error (>>= ">>= (bind) used outside of 'with-monad'" ((line . 451) (column . 9) (filename . "./guix/monads.scm")) (>>= (mproc head result) (lambda # ?)) #))
In guix/build/compile.scm:
    122:6  3 (_ _ . _)
In ice-9/boot-9.scm:
    829:9  2 (catch #t #<procedure 40f93e0 at guix/build/compile.scm:122:6 ()> #<procedure 7fffefd02888 at guix/build/compile.scm:122:6 args> _)
In guix/build/compile.scm:
   125:21  1 (_)
In unknown file:
           0 (make-stack #t)
guix/build/compile.scm:125:21: Syntax error:
./guix/monads.scm:452:9: >>=: >>= (bind) used outside of 'with-monad' in form (>>= (mproc head result) (lambda (result) (loop tail result)))
builder for `/gnu/store/hf324mhj5607hh2izb01dzhwakmn8am8-guix-core.drv' failed with exit code 1
--8<---------------cut here---------------end--------------->8---

Here (guix monads) was already loaded before, but it’s only when
compiling (guix monads), so after it had been loaded, that we get the
error.  The syntax parameter in question is defined in (guix monads)
itself.

I drew the conclusion that our syntax parameter is redefined when we
compile or when we load (guix monads), so there’s a chance that we get
to see the wrong value when we expand (guix monads) (I’m not entirely
sure about the exact sequence of events.)

So I came up with ‘define-syntax-parameter-once’, which is like
‘define-once’ but for syntax parameters (note that we can’t use
‘define-once’ in ‘define-syntax-parameter-once’ because it expands to a
reference to NAME, which doesn’t work for a macro):

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

diff --git a/guix/monads.scm b/guix/monads.scm
index 6ae616aca9..1bbf79c8ba 100644
--- a/guix/monads.scm
+++ b/guix/monads.scm
@@ -274,12 +274,20 @@ more optimizations."
                    (_
                     #'generic-name))))))))))
 
-(define-syntax-parameter >>=
+(define-syntax-rule (define-syntax-parameter-once name proc)
+  (eval-when (load eval expand compile)
+    (define name
+      (if (module-locally-bound? (current-module) 'name)
+          (module-ref (current-module) 'name)
+          (make-syntax-transformer 'name 'syntax-parameter
+                                   (list proc))))))
+
+(define-syntax-parameter-once >>=
   ;; The name 'bind' is already taken, so we choose this (obscure) symbol.
   (lambda (s)
     (syntax-violation '>>= ">>= (bind) used outside of 'with-monad'" s)))
 
-(define-syntax-parameter return
+(define-syntax-parameter-once return
   (lambda (s)
     (syntax-violation 'return "return used outside of 'with-monad'" s)))
 

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

I’ve done a number of rebuilds of guix-core.drv on that 24-core machine
and AFAICS that fixes the issue!

We’ll also have to use it in (guix gexp), which I’m pretty sure will fix
<https://issues.guix.info/issue/28144>.

I’ll push this workaround if there are no objections.

On the Guile side, we could maybe arrange to always have ‘define-once’
semantics for those bindings introduced at expansion time, as shown
below (untested):

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

--- a/module/ice-9/psyntax.scm
+++ b/module/ice-9/psyntax.scm
@@ -296,9 +296,10 @@
 
       (define put-global-definition-hook
         (lambda (symbol type val)
-          (module-define! (current-module)
-                          symbol
-                          (make-syntax-transformer symbol type val))))
+          (unless (module-locally-bound? (current-module) symbol)
+            (module-define! (current-module)
+                            symbol
+                            (make-syntax-transformer symbol type val)))))
     
       (define get-global-definition-hook
         (lambda (symbol module)

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

WDYT, Andy?

The discussion we had at FOSDEM turned out to be very helpful, thanks
a lot!

Ludo’.

Previous Next

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