From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Subject: 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-ID: <87lg2t7z1o.fsf@gnu.org> References: <87h8vvp1q7.fsf@elephly.net> <87377esu1a.fsf@gnu.org> <87k20nz18u.fsf@igalia.com> <87a81jj5gg.fsf@gnu.org> <87bmlyzxj7.fsf@elephly.net> <87shf44ny0.fsf@elephly.net> <878tfi9x15.fsf@gnu.org> <87h8nstms1.fsf@gnu.org> <874ljstlvq.fsf_-_@gnu.org> <87603x6x1f.fsf@igalia.com> <878t8tyyfk.fsf@gnu.org> <87lgct5e06.fsf@igalia.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([209.51.188.92]:47826) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1grOVc-0002QI-Hv for bug-guix@gnu.org; Wed, 06 Feb 2019 09:49:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1grOVa-0002wC-U4 for bug-guix@gnu.org; Wed, 06 Feb 2019 09:49:04 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:34925) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1grOVa-0002vr-M2 for bug-guix@gnu.org; Wed, 06 Feb 2019 09:49:02 -0500 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87lgct5e06.fsf@igalia.com> (Andy Wingo's message of "Wed, 09 May 2018 12:18:01 +0200") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Andy Wingo Cc: 27476@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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=E2=80=99s compiling, and here=E2=80=99s the crash I got (on a 24-core ma= chine): --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 . "./gu= ix/monads.scm")) _ # = _ _ _) 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) ("pl= aceholder" placeholder) ("l-10a3c9?" . #) ?) ?))) In ice-9/psyntax.scm: 1409:12 16 (_ _ _ #)>) 2338:44 15 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./gu= ix/monads.scm")) (hygiene guix monads) # _ _ ((# ?))) 1679:45 14 (parse _ _ _ _ _ _ _) In ice-9/boot-9.scm: 222:17 13 (map1 (((("l-10a3c941d34314a1-4894" macro . #) ("placeholder" placeholder) ("l-10a3c941d3= 4314a1-488f" lexical . #) ("l-10?" . #) ?) . #))) In ice-9/psyntax.scm: 2338:44 12 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./gu= ix/monads.scm")) (hygiene guix monads) # _ _ ((# ?))) 1679:45 11 (parse _ _ _ _ _ _ _) In ice-9/boot-9.scm: 222:17 10 (map1 (((("l-10a3c941d34314a1-48b0" macro . #) ("placeholder" placeholder) ("l-10a3c941d3= 4314a1-48ac" lexical . #) ("l-10?" . #) ?) . #))) In ice-9/psyntax.scm: 2338:44 9 (expand-let _ _ _ ((line . 447) (column . 6) (filename . "./gu= ix/monads.scm")) (hygiene guix monads) # _ _ ((# ?))) 1612:33 8 (parse (((("placeholder" placeholder) ("l-10a3c941d34314a1-48c= 8" lexical . tail-10a3c941d34314a1-48c9) ("l-10a3c941d34314a1-48b0" macro .= #) ?) . #)) ?) 1348:32 7 (syntax-type (>>=3D (mproc head result) (lambda (result) (loop= tail result))) (("placeholder" placeholder) ("l-10a3c941d34314a1-48c8" lex= ical . tail-10a3c941d34314a1-48c9) ("l-?" . #) ?) ?) 1559:32 6 (expand-macro # _ _ _ _ _ _) In ice-9/boot-9.scm: 752:25 5 (dispatch-exception _ _ _) 751:25 4 (dispatch-exception 1 syntax-error (>>=3D ">>=3D (bind) used o= utside of 'with-monad'" ((line . 451) (column . 9) (filename . "./guix/mona= ds.scm")) (>>=3D (mproc head result) (lambda # ?)) #)) In guix/build/compile.scm: 122:6 3 (_ _ . _) In ice-9/boot-9.scm: 829:9 2 (catch #t # # _) 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: >>=3D: >>=3D (bind) used outside of 'with-monad' i= n form (>>=3D (mproc head result) (lambda (result) (loop tail result))) builder for `/gnu/store/hf324mhj5607hh2izb01dzhwakmn8am8-guix-core.drv' fai= led with exit code 1 --8<---------------cut here---------------end--------------->8--- Here (guix monads) was already loaded before, but it=E2=80=99s 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=E2=80=99s a chance that we = get to see the wrong value when we expand (guix monads) (I=E2=80=99m not entire= ly sure about the exact sequence of events.) So I came up with =E2=80=98define-syntax-parameter-once=E2=80=99, which is = like =E2=80=98define-once=E2=80=99 but for syntax parameters (note that we can= =E2=80=99t use =E2=80=98define-once=E2=80=99 in =E2=80=98define-syntax-parameter-once=E2= =80=99 because it expands to a reference to NAME, which doesn=E2=80=99t work for a macro): --=-=-= Content-Type: text/x-patch Content-Disposition: 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))) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable I=E2=80=99ve done a number of rebuilds of guix-core.drv on that 24-core mac= hine and AFAICS that fixes the issue! We=E2=80=99ll also have to use it in (guix gexp), which I=E2=80=99m pretty = sure will fix . I=E2=80=99ll push this workaround if there are no objections. On the Guile side, we could maybe arrange to always have =E2=80=98define-on= ce=E2=80=99 semantics for those bindings introduced at expansion time, as shown below (untested): --=-=-= Content-Type: text/x-patch Content-Disposition: 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) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable WDYT, Andy? The discussion we had at FOSDEM turned out to be very helpful, thanks a lot! Ludo=E2=80=99. --=-=-=--