bug#31878: Module autoloading is not thread safe

[Calvin Heim]

Hi Ludo and Mark,

> However peg.test fails and it may be related

The patch fails when a module has one of its submodules in its uses list, and
the program starts loading the module before it starts loading the submodule.
In the test case, (ice-9 peg) uses (ice-9 peg codegen) and the test starts
loading (ice-9 peg) before starting to load (ice-9 peg codegen).

I haven't uncovered the cause of the failure though.

Sincerely,
Calvin Heim

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

Hi Mark,

Mark H Weaver  skribis:

> I've written a preliminary patch to implement the improved thread-safe
> module autoloading that I outlined in earlier messages in this bug
> report.

Great, thanks a lot!

On a cursory look, it LGTM.  I wonder if we could somehow split
‘resolve-module’ and ‘try-module-autoload’ into smaller chunks because
they’ve become quite large and complex.

For example:

> +(lambda ()
> +  (when mutex
> +((ice-9-threads 'lock-mutex) mutex))
> +  (set-autoloaded! dir-hint name didit)))
> +
> +  ;; If the local module was actually created, then we
> +  ;; now add it to the global module table.
> +  (let ((module (car lazy-module-cell)))
> +(when module
> +  (module-define-submodule! parent-module
> +(car reverse-name)
> +module)))
> +
> +  ;; Signal all threads waiting on the condition variable
> +  ;; for this module to be loaded.
> +  (when cond-var
> +((ice-9-threads 'broadcast-condition-variable) cond-var))
> +
> +  ;; Remove the module from '%modules-being-loaded'.
> +  (set! %modules-being-loaded
> +(assoc-remove! %modules-being-loaded
> +   module-name))
> +
> +  ;; Remove all '%modules-waiting-for' entries that are
> +  ;; directly related to the module that we just loaded
> +  ;; (or attempted to load).
> +  (set! %modules-waiting-for
> +(filter! (lambda (entry)
> +   (not (or (equal? module-name (car entry))
> +(equal? module-name (cdr entry)
> + %modules-waiting-for))

Perhaps this bit could go in a ‘record-module-autoload-completion!’
procedure or something along these lines?

It’s great that you came up with tests to reproduce the problems.  I
confirm that “./check-guile threads.test” works for me.  The tests pass
even if I remove the {boot-9,threads}.scm changes, though.

However peg.test fails and it may be related:

--8<---cut here---start->8---
$ ./check-guile peg.test
Testing /data/src/guile-2.1/meta/guile ... peg.test
with GUILE_LOAD_PATH=/data/src/guile-2.1/test-suite
Running peg.test
Backtrace:
In ice-9/eval.scm:
   293:34 19 (_ #)
In ice-9/boot-9.scm:
   3032:4 18 (define-module* _ #:filename _ #:pure _ #:version _ #:imports _ 
#:exports _ #:replacements _ # _ # _ \u2026)
  2071:24 17 (call-with-deferred-observers #)
  3045:24 16 (_)
   222:29 15 (map1 (((test-suite lib)) ((ice-9 peg)) ((ice-9 pretty-print)) 
((srfi srfi-1
   222:17 14 (map1 (((ice-9 peg)) ((ice-9 pretty-print)) ((srfi srfi-1
  2958:17 13 (resolve-interface (ice-9 peg) #:select _ #:hide _ #:prefix _ 
#:renamer _ #:version _)
In ice-9/threads.scm:
390:8 12 (_ _)
In ice-9/boot-9.scm:
  2881:28 11 (_ #)
In ice-9/threads.scm:
390:8 10 (_ _)
In ice-9/boot-9.scm:
  3171:20  9 (_ #)
   2312:4  8 (save-module-excursion #)
  3191:26  7 (_)
In unknown file:
   6 (primitive-load-path "ice-9/peg" #)
In ice-9/peg.scm:
 20:0  5 (_)
In ice-9/boot-9.scm:
   3032:4  4 (define-module* _ #:filename _ #:pure _ #:version _ #:imports _ 
#:exports _ #:replacements _ # _ # _ \u2026)
  3045:24  3 (_)
   222:17  2 (map1 (((ice-9 peg codegen)) ((ice-9 peg string-peg)) ((ice-9 peg 
simplify-tree)) ((ice-9 peg #)) #))
   2961:6  1 (resolve-interface _ #:select _ #:hide _ #:prefix _ #:renamer _ 
#:version _)
In unknown file:
   0 (scm-error misc-error #f "~A ~S" ("no code for module" (ice-9 peg 
codegen)) #f)

ERROR: In procedure scm-error:
no code for module (ice-9 peg codegen)
--8<---cut here---end--->8---

Thoughts?

Thank you!

Ludo’.

bug#31878: Module autoloading is not thread safe

[Mark H Weaver]

I've written a preliminary patch to implement the improved thread-safe
module autoloading that I outlined in earlier messages in this bug
report.

Comments, suggestions, and testing welcome.

  Mark


>From 897a6f76280612e83f48d63430bf962520c0e7b3 Mon Sep 17 00:00:00 2001
From: Mark H Weaver 
Date: Sun, 21 Oct 2018 09:56:16 -0400
Subject: [PATCH] DRAFT: Fix thread-safe module loading.

* module/ice-9/boot-9.scm (%modules-being-loaded)
(%local-modules-being-loaded, %modules-waiting-for): New variables.
(%force-lazy-module-cell!, %module-waiting-for?)
(%module-waiting-for!): New procedures.
(resolve-module): If the requested module is not in the regular global
module table, look in '%local-modules-being-loaded' and
'%modules-being-loaded', and handle these cases appropriately.  Support
looping without recursively locking the autoload lock.  When
autoloading, unlock the mutex before calling 'try-load-module'.
(try-module-autoload): Add entries to '%modules-being-loaded' and
'%local-modules-being-loaded' before loading the module.  Also, load the
module with the autoload mutex unlocked.  When the load attempt
finishes (or fails), add the module to the regular global module table
if it was ever created, signal the threads waiting for this module, and
remove it from the '*-begin-loaded' and '%modules-waiting-for' tables.
(call-with-module-autoload-lock): Accept a unary procedure instead of a
thunk.
(module-name): Adapt to the new 'call-with-module-autoload-lock'.
(nested-define-module!): If we're asked to define a submodule of a
module that's currently being loaded, install the parent module being
loaded into the global module table.
* module/ice-9/threads.scm (call-with-module-autoload-lock):
Pass the mutex as an argument to the procedure.
* test-suite/tests/threads.test: Add tests.
* test-suite/tests/delayed-test.scm,
test-suite/tests/mutual-delayed-a.scm,
test-suite/tests/mutual-delayed-b.scm,
test-suite/tests/mutual-delayed-c.scm: New files.
* test-suite/Makefile.am (EXTRA_DIST): Add them.
---
 module/ice-9/boot-9.scm   | 292 ++
 module/ice-9/threads.scm  |   4 +-
 test-suite/Makefile.am|   7 +-
 test-suite/tests/delayed-test.scm |  28 +++
 test-suite/tests/mutual-delayed-a.scm |  29 +++
 test-suite/tests/mutual-delayed-b.scm |  29 +++
 test-suite/tests/mutual-delayed-c.scm |  29 +++
 test-suite/tests/threads.test |  66 +-
 8 files changed, 435 insertions(+), 49 deletions(-)
 create mode 100644 test-suite/tests/delayed-test.scm
 create mode 100644 test-suite/tests/mutual-delayed-a.scm
 create mode 100644 test-suite/tests/mutual-delayed-b.scm
 create mode 100644 test-suite/tests/mutual-delayed-c.scm

diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index d8801dada..404a19d49 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -2502,13 +2502,32 @@ interfaces are added to the inports list."
  (tail (cdr names)))
 (if (null? tail)
 (module-define-submodule! cur head module)
-(let ((cur (or (module-ref-submodule cur head)
-   (let ((m (make-module 31)))
- (set-module-kind! m 'directory)
- (set-module-name! m (append (module-name cur)
- (list head)))
- (module-define-submodule! cur head m)
- m
+(let ((cur
+   (or (module-ref-submodule cur head)
+   (let ((dir-name (append (module-name cur)
+   (list head
+ (cond ((assoc dir-name %modules-being-loaded)
+=> (lambda (entry)
+ ;; The module we're being asked to define
+ ;; is a submodule of a module that's
+ ;; currently being loaded.  In this case,
+ ;; we must install the parent module
+ ;; being loaded into the global module
+ ;; table.  This is unfortunate, but it's
+ ;; not clear how to avoid this without
+ ;; changing the structure of the global
+ ;; module table.
+ (let ((m (%force-lazy-module-cell!
+   (cddr entry)
+   dir-name)))
+   (module-define-submodule! cur head m)
+   m)))
+   (else
+(let ((m (make-module 31)))
+  (set-module-kind! m 'directory)
+   

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

Hi Mark,

Mark H Weaver  skribis:

> l...@gnu.org (Ludovic Courtès) writes:
>
>> Mark H Weaver  skribis:
>>
>>> Since Guile (unfortunately) allows cyclic module dependencies, we would
>>> need a mechanism to avoid deadlocks in case modules A and B both import
>>> each other, and two threads concurrently attempt to load those modules.
>>>
>>> The first idea that comes to mind is to also have a global structure
>>> storing a partial order on the modules currently being loaded.  If,
>>> while module A is being loaded, there's an attempt to auto-load module
>>> B, then an entry (A < B) would added to the partial order.  The partial
>>> order would not allow cycles to be introduced, reporting an error in
>>> that case.  In case a cycle would be introduced when adding (A < B),
>>> then the thread would simply be given access to the partially-loaded
>>> module B, by adding B to its local list of modules-being-loaded.
>>
>> Would it enough to (1) use recursive mutexes, and (2) have
>> ‘resolve-module’ lookup modules first in the global name space, and
>> second in the local list of modules being loaded?
>
> Item (2) above is something that I had already envisioned in my
> proposal, although I neglected to mention it.
>
> However, I don't see how recursive mutexes would help here, or how they
> could obviate the need for the other mechanisms I described above.
>
> Suppose module A and module B are mutually dependent on each other.  If
> thread 1 is loading module A concurrently with thread 2 loading module
> B, then thread 1 will be the only thread with access to module A (via
> thread 1's local list) and will hold the lock on it, and similarly for
> thread 2 and module B.
>
> Now, when thread 1 tries to load module B (while it's in the process of
> loading module A), it should normally be blocked until module B is
> finished loading.  If those modules were _not_ mutually dependent on
> each other, we should insist on thread 1 waiting for module B to finish
> loading before gaining access to it.  Only if there is a cyclic
> dependency should it be granted access to the partially-loaded module.
>
> If we simply use recursive mutexes, I think deadlock would occur in this
> case.  Thread 1 would try to grab the lock on module B, which is already
> held by thread 2, and vice versa.  Since it's not self-held, I fail to
> see the relevance of the recursive mutex.

Oh, got it; you’re right.  So yes, the solution you outlined above is
probably what’s needed.

Thanks for explaining!

Ludo’.

bug#31878: Module autoloading is not thread safe

[Mark H Weaver]

Hi Ludovic,

l...@gnu.org (Ludovic Courtès) writes:

> Mark H Weaver  skribis:
>
>> Since Guile (unfortunately) allows cyclic module dependencies, we would
>> need a mechanism to avoid deadlocks in case modules A and B both import
>> each other, and two threads concurrently attempt to load those modules.
>>
>> The first idea that comes to mind is to also have a global structure
>> storing a partial order on the modules currently being loaded.  If,
>> while module A is being loaded, there's an attempt to auto-load module
>> B, then an entry (A < B) would added to the partial order.  The partial
>> order would not allow cycles to be introduced, reporting an error in
>> that case.  In case a cycle would be introduced when adding (A < B),
>> then the thread would simply be given access to the partially-loaded
>> module B, by adding B to its local list of modules-being-loaded.
>
> Would it enough to (1) use recursive mutexes, and (2) have
> ‘resolve-module’ lookup modules first in the global name space, and
> second in the local list of modules being loaded?

Item (2) above is something that I had already envisioned in my
proposal, although I neglected to mention it.

However, I don't see how recursive mutexes would help here, or how they
could obviate the need for the other mechanisms I described above.

Suppose module A and module B are mutually dependent on each other.  If
thread 1 is loading module A concurrently with thread 2 loading module
B, then thread 1 will be the only thread with access to module A (via
thread 1's local list) and will hold the lock on it, and similarly for
thread 2 and module B.

Now, when thread 1 tries to load module B (while it's in the process of
loading module A), it should normally be blocked until module B is
finished loading.  If those modules were _not_ mutually dependent on
each other, we should insist on thread 1 waiting for module B to finish
loading before gaining access to it.  Only if there is a cyclic
dependency should it be granted access to the partially-loaded module.

If we simply use recursive mutexes, I think deadlock would occur in this
case.  Thread 1 would try to grab the lock on module B, which is already
held by thread 2, and vice versa.  Since it's not self-held, I fail to
see the relevance of the recursive mutex.

What do you think?

  Mark

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

Mark H Weaver  skribis:

> I forgot to mention an important aspect of the proposed auto-load
> locking here.  It would not be a global lock, but rather a lock specific
> to the module being loaded.  So, I guess we would need a global table of
> locks for modules-being-loaded.

Right (we can’t add a mutex to the module record without breaking the
ABI.)

> Since Guile (unfortunately) allows cyclic module dependencies, we would
> need a mechanism to avoid deadlocks in case modules A and B both import
> each other, and two threads concurrently attempt to load those modules.
>
> The first idea that comes to mind is to also have a global structure
> storing a partial order on the modules currently being loaded.  If,
> while module A is being loaded, there's an attempt to auto-load module
> B, then an entry (A < B) would added to the partial order.  The partial
> order would not allow cycles to be introduced, reporting an error in
> that case.  In case a cycle would be introduced when adding (A < B),
> then the thread would simply be given access to the partially-loaded
> module B, by adding B to its local list of modules-being-loaded.

Would it enough to (1) use recursive mutexes, and (2) have
‘resolve-module’ lookup modules first in the global name space, and
second in the local list of modules being loaded?

Thanks,
Ludo’.

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

Hi Mark,

Mark H Weaver  skribis:

> reopen 31878
> thanks
>
> Hi Ludovic,
>
> l...@gnu.org (Ludovic Courtès) writes:
>
>> l...@gnu.org (Ludovic Courtès) skribis:
>>
>>> l...@gnu.org (Ludovic Courtès) skribis:
>>>
 I believe this comes from the fact that ‘autoloads-done’ and related
 alists in (ice-9 boot-9) are manipulated in a non-thread-safe fashion.
>>>
>>> Here’s a proposed fix for ‘stable-2.2’ as discussed on #guile, Andy:
>>
>> After further discussion on IRC, I pushed a variant of this patch as
>> commit 761cf0fb8c364e885e4c6fced34563f8157c3b84.
>
> There are problems with this fix, e.g. .
>
> More generally, nearly arbitrary code can be run in the top-level
> expressions of a module.  It could launch other threads which try to
> load modules, or even send messages to other existing threads asking
> them to do work.  In some cases, the body of the module might never
> terminate.  The entire main program might be run from there.  I suspect
> that's not unusual.

Indeed, good catch.  :-/

> I can see another problem as well: while the module is in the process of
> loading, the partially-loaded module is globally visible and accessible
> to other threads.  If I'm not mistaken, with this patch, there's nothing
> preventing other threads from attempting to use the partially-loaded
> module.

The module is not reachable until ‘set-module-name!’ has been called on
it, but ‘process-define-module’ does that right away IIRC, i.e., before
the whole body has been evaluated.  So I guess you’re right: other
threads could stumble upon partially-loaded modules.

If the ‘define-module’ scoped encompassed the whole body like the R6
‘library’ form, it would be easy to determine when the whole module
top-level has been loaded.  Right now, I suppose we have to determine
the end-of-module-top-level “from the outside”, i.e., from
‘resolve-module’ or similar, no?

> I thought about how to fix this thread-safety problem a long time ago,
> and came up with a rough outline of a solution.  The idea is that the
> module should not be added to the global module table until the module
> has finished loading.  While the module is being loaded, it would be
> made visible only to the loading thread, and to any other threads
> spawned during the loading process, by adding the module to a local list
> of modules-being-loaded referenced by a fluid variable.  If any other
> threads attempt to access the module, it would not be found in the
> global module table, and thus trigger an auto-load, which would wait for
> the lock to be released before proceeding.
>
> What do you think?

It sounds like a good idea.

Thanks,
Ludo’.

bug#31878: Module autoloading is not thread safe

[Mark H Weaver]

I wrote:

> I thought about how to fix this thread-safety problem a long time ago,
> and came up with a rough outline of a solution.  The idea is that the
> module should not be added to the global module table until the module
> has finished loading.  While the module is being loaded, it would be
> made visible only to the loading thread, and to any other threads
> spawned during the loading process, by adding the module to a local list
> of modules-being-loaded referenced by a fluid variable.  If any other
> threads attempt to access the module, it would not be found in the
> global module table, and thus trigger an auto-load, which would wait for
> the lock to be released before proceeding.

I forgot to mention an important aspect of the proposed auto-load
locking here.  It would not be a global lock, but rather a lock specific
to the module being loaded.  So, I guess we would need a global table of
locks for modules-being-loaded.

Since Guile (unfortunately) allows cyclic module dependencies, we would
need a mechanism to avoid deadlocks in case modules A and B both import
each other, and two threads concurrently attempt to load those modules.

The first idea that comes to mind is to also have a global structure
storing a partial order on the modules currently being loaded.  If,
while module A is being loaded, there's an attempt to auto-load module
B, then an entry (A < B) would added to the partial order.  The partial
order would not allow cycles to be introduced, reporting an error in
that case.  In case a cycle would be introduced when adding (A < B),
then the thread would simply be given access to the partially-loaded
module B, by adding B to its local list of modules-being-loaded.

Comments and suggestions welcome,

Mark

bug#31878: Module autoloading is not thread safe

[Mark H Weaver]

Hi Ludovic,

l...@gnu.org (Ludovic Courtès) writes:

> l...@gnu.org (Ludovic Courtès) skribis:
>
>> l...@gnu.org (Ludovic Courtès) skribis:
>>
>>> I believe this comes from the fact that ‘autoloads-done’ and related
>>> alists in (ice-9 boot-9) are manipulated in a non-thread-safe fashion.
>>
>> Here’s a proposed fix for ‘stable-2.2’ as discussed on #guile, Andy:
>
> After further discussion on IRC, I pushed a variant of this patch as
> commit 761cf0fb8c364e885e4c6fced34563f8157c3b84.

There are problems with this fix, e.g. .

More generally, nearly arbitrary code can be run in the top-level
expressions of a module.  It could launch other threads which try to
load modules, or even send messages to other existing threads asking
them to do work.  In some cases, the body of the module might never
terminate.  The entire main program might be run from there.  I suspect
that's not unusual.

I can see another problem as well: while the module is in the process of
loading, the partially-loaded module is globally visible and accessible
to other threads.  If I'm not mistaken, with this patch, there's nothing
preventing other threads from attempting to use the partially-loaded
module.

I thought about how to fix this thread-safety problem a long time ago,
and came up with a rough outline of a solution.  The idea is that the
module should not be added to the global module table until the module
has finished loading.  While the module is being loaded, it would be
made visible only to the loading thread, and to any other threads
spawned during the loading process, by adding the module to a local list
of modules-being-loaded referenced by a fluid variable.  If any other
threads attempt to access the module, it would not be found in the
global module table, and thus trigger an auto-load, which would wait for
the lock to be released before proceeding.

What do you think?

  Mark

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

l...@gnu.org (Ludovic Courtès) skribis:

> l...@gnu.org (Ludovic Courtès) skribis:
>
>> I believe this comes from the fact that ‘autoloads-done’ and related
>> alists in (ice-9 boot-9) are manipulated in a non-thread-safe fashion.
>
> Here’s a proposed fix for ‘stable-2.2’ as discussed on #guile, Andy:

After further discussion on IRC, I pushed a variant of this patch as
commit 761cf0fb8c364e885e4c6fced34563f8157c3b84.

Ludo’.

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

l...@gnu.org (Ludovic Courtès) skribis:

> I believe this comes from the fact that ‘autoloads-done’ and related
> alists in (ice-9 boot-9) are manipulated in a non-thread-safe fashion.

Here’s a proposed fix for ‘stable-2.2’ as discussed on #guile, Andy:

diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index 4e51e9281..960cb9fa3 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -1,6 +1,6 @@
 ;;; -*- mode: scheme; coding: utf-8; -*-
 
- Copyright (C) 1995-2014, 2016-2017  Free Software Foundation, Inc.
+ Copyright (C) 1995-2014, 2016-2018  Free Software Foundation, Inc.
 
  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
@@ -2952,8 +2952,11 @@ module '(ice-9 q) '(make-q q-length))}."
 ;;; {Autoloading modules}
 ;;;
 
-;;; XXX FIXME autoloads-in-progress and autoloads-done
-;;;   are not handled in a thread-safe way.
+(define (call-with-module-autoload-lock thunk)
+  ;; This binding is overridden when (ice-9 threads) is available to
+  ;; implement a critical section around the call to THUNK.  It must be
+  ;; used anytime the autoload variables below are used.
+  (thunk))
 
 (define autoloads-in-progress '())
 
@@ -2973,37 +2976,40 @@ but it fails to load."
 file-name-separator-string))
dir-hint-module-name
 (resolve-module dir-hint-module-name #f)
-(and (not (autoload-done-or-in-progress? dir-hint name))
- (let ((didit #f))
-   (dynamic-wind
-(lambda () (autoload-in-progress! dir-hint name))
-(lambda ()
-  (with-fluids ((current-reader #f))
-(save-module-excursion
- (lambda () 
-   (define (call/ec proc)
- (let ((tag (make-prompt-tag)))
-   (call-with-prompt
-tag
-(lambda ()
-  (proc (lambda () (abort-to-prompt tag
-(lambda (k) (values)
-   ;; The initial environment when loading a module is a fresh
-   ;; user module.
-   (set-current-module (make-fresh-user-module))
-   ;; Here we could allow some other search strategy (other than
-   ;; primitive-load-path), for example using versions encoded
-   ;; into the file system -- but then we would have to figure
-   ;; out how to locate the compiled file, do auto-compilation,
-   ;; etc. Punt for now, and don't use versions when locating
-   ;; the file.
-   (call/ec
-(lambda (abort)
-  (primitive-load-path (in-vicinity dir-hint name)
-   abort)
-  (set! didit #t)))
-(lambda () (set-autoloaded! dir-hint name didit)))
-   didit
+
+(call-with-module-autoload-lock
+ (lambda ()
+   (and (not (autoload-done-or-in-progress? dir-hint name))
+(let ((didit #f))
+  (dynamic-wind
+(lambda () (autoload-in-progress! dir-hint name))
+(lambda ()
+  (with-fluids ((current-reader #f))
+(save-module-excursion
+ (lambda () 
+   (define (call/ec proc)
+ (let ((tag (make-prompt-tag)))
+   (call-with-prompt
+   tag
+ (lambda ()
+   (proc (lambda () (abort-to-prompt tag
+ (lambda (k) (values)
+   ;; The initial environment when loading a module is a fresh
+   ;; user module.
+   (set-current-module (make-fresh-user-module))
+   ;; Here we could allow some other search strategy (other than
+   ;; primitive-load-path), for example using versions encoded
+   ;; into the file system -- but then we would have to figure
+   ;; out how to locate the compiled file, do auto-compilation,
+   ;; etc. Punt for now, and don't use versions when locating
+   ;; the file.
+   (call/ec
+(lambda (abort)
+  (primitive-load-path (in-vicinity dir-hint name)
+   abort)
+  (set! didit #t)))
+(lambda () (set-autoloaded! dir-hint name didit)))
+  didit))
 
 
 
@@ -4061,6 +4067,19 @@ when none is available, reading FILE-NAME with READER."
 ;; Load (ice-9 threads), initializing some internal data structures.
 

bug#31878: Module autoloading is not thread safe

[Ludovic Courtès]

Hello,

The backtrace below (from Guile 2.2.3) shows the Guix multi-threaded
module compilation code crashing with an incorrect module resolution
failure (incorrect because the module does exist.)

I believe this comes from the fact that ‘autoloads-done’ and related
alists in (ice-9 boot-9) are manipulated in a non-thread-safe fashion.
Notice that a couple of threads in the backtrace are effectively
running:

  (member x autoloads-done)

Thanks,
Ludo’.

--8<---cut here---start->8---
$ out=/tmp/t NIX_BUILD_CORES=8 setarch x86_64 -R gdb --args  guile 
"--no-auto-compile" "-L" . "-C" . 
"/gnu/store/815przlfissan60p4bdn7cipfidc3q29-guix-extra-builder"
GNU gdb (GDB) 8.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
ERROR: In procedure type-pointer:
ERROR: In procedure gdbscm_type_pointer: Wrong type argument in position 1 
(expecting gdb:type): #f
/home/ludo/.gdbinit:8: Error in sourced command file:
Error while executing Scheme code.
Reading symbols from guile...Reading symbols from 
/gnu/store/nlkf6c702f8c6wsadbmib0w0mnxccqdi-guile-2.2.3-debug/lib/debug//gnu/store/1mr5izrbxwd7cbq8m1xrqm45rzkibpsj-guile-2.2.3/bin/guile.debug...
done. 
done. 
(gdb) shell rm -rf /tmp/t
(gdb) r
Starting program: /gnu/store/gdmn1al64y4366q5nb0v87qdv0q6xpmp-profile/bin/guile 
--no-auto-compile -L . -C . 
/gnu/store/815przlfissan60p4bdn7cipfidc3q29-guix-extra-builder
[Thread debugging using libthread_db enabled]
Using host libthread_db library 
"/gnu/store/l4lr0f5cjd0nbsaaf8b5dmcw1a1yypr3-glibc-2.27/lib/libthread_db.so.1".
[New Thread 0x75e56700 (LWP 6455)]
[New Thread 0x75655700 (LWP 6456)]
[New Thread 0x74e54700 (LWP 6457)]
[New Thread 0x73ee8700 (LWP 6458)]
loading...   97.2% of 109 filesrandom seed for tests: 1529311210
loading...  100.0% of 109 files[New Thread 0x7fffe9c11700 (LWP 6460)]
compiling...  0.0% of 109 files[New Thread 0x7fffe9410700 (LWP 6461)]
com[New Thread 0x7fffe8c0f700 (LWP 6462)]
compiling...  0.0% of 109 files[New Thread 0x7fffe3fff700 (LWP 6463)]
compil[New Thread 0x7fffe37fe700 (LWP 6464)]
compiling...0% o  0.0% of 109 files[New Thread 0x7fffe2ffd700 (LWP 6465)]
compiling...[New Thread 0x7fffe27fc700 (LWP 6466)]
c[New Thread 0x7fffe1ffb700 (LWP 6467)]
ompiling...   0.0% of 109 filesException thrown while printing backtrace:
In procedure Exception thrown while printing backtrace:
In procedure private-lookup: Module named (rnrs bytevectors) does not exist
Exception thrown while printing backtrace:
In procedure private-lookup: Module named (rnrs bytevectors) does not exist
Exception thrown while printing backtrace:
In procedure private-lookup: Module named (rnrs bytevectors) does not exist

Thread 11 "guile" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffe2ffd700 (LWP 6465)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51  ../sysdeps/unix/sysv/linux/raise.c: Dosiero aŭ dosierujo ne ekzistas.
(gdb) thread apply all bt

Thread 13 (Thread 0x7fffe1ffb700 (LWP 6467)):
#0  clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:78
#1  0x776204a0 in ?? () at pthread_create.c:361 from 
/gnu/store/l4lr0f5cjd0nbsaaf8b5dmcw1a1yypr3-glibc-2.27/lib/libpthread.so.0
#2  0x7fffe1ffb700 in ?? ()
#3  0x in ?? ()

Thread 12 (Thread 0x7fffe27fc700 (LWP 6466)):
#0  0x776299df in __libc_write (fd=fd@entry=37, 
buf=buf@entry=0x15dcda0, nbytes=nbytes@entry=13) at 
../sysdeps/unix/sysv/linux/write.c:27
#1  0x77b052ce in fport_write (port=, src=, start=, count=13) at fports.c:597
#2  0x77b34d0c in scm_i_write_bytes (port=#, 
src="#" = {...}, start=0, count=13) at ports.c:2857
#3  0x77b37671 in scm_c_put_latin1_chars (port=#, 
chars=, len=13) at ports.c:3457
#4  0x77b39e95 in scm_simple_format (destination=#, 
message="In procedure ~a: ", args=("private-lookup")) at print.c:1173
#5  0x77b717fd in vm_regular_engine (thread=0x25, vp=0x804b40, 
registers=0xd, resume=-144533025) at vm-engine.c:784
#6  0x77b74e5a in scm_call_n (proc=#, 
argv=argv@entry=0x7fffe27faef0, nargs=nargs@entry=4) at vm.c:1257
#7  0x77af7f94 in scm_call_4 (proc=, 
arg1=arg1@entry=#, arg2=arg2@entry=#f, 
arg3=arg3@entry=misc-error, 
arg4=arg4@entry=("private-lookup" "Module named ~s does not exist" ((rnrs