From: ludo@gnu.org (Ludovic Courtès)
To: Mark H Weaver <mhw@netris.org>
Cc: 31878@debbugs.gnu.org
Subject: bug#31878: Module autoloading is not thread safe
Date: Mon, 22 Oct 2018 12:10:15 +0200 [thread overview]
Message-ID: <87k1manvg8.fsf@gnu.org> (raw)
In-Reply-To: <871s8j17xq.fsf@netris.org> (Mark H. Weaver's message of "Sun, 21 Oct 2018 14:16:49 -0400")
Hi Mark,
Mark H Weaver <mhw@netris.org> 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 (_ #<directory (guile-user) 1f5c140>)
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 #<procedure 1f06eb0 at ice-9/boot-9.scm:3033:5 ()>)
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 (_ #<mutex 1f5ff80>)
In ice-9/threads.scm:
390:8 10 (_ _)
In ice-9/boot-9.scm:
3171:20 9 (_ #<mutex 1f5ff80>)
2312:4 8 (save-module-excursion #<procedure 21baf90 at ice-9/boot-9.scm:3172:21 ()>)
3191:26 7 (_)
In unknown file:
6 (primitive-load-path "ice-9/peg" #<procedure 20c2c20 at ice-9/boot-9.scm:3178:37 ()>)
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’.
next prev parent reply other threads:[~2018-10-22 10:10 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-18 9:43 bug#31878: Module autoloading is not thread safe Ludovic Courtès
2018-06-18 11:11 ` Ludovic Courtès
2018-06-18 12:17 ` Ludovic Courtès
2018-08-22 23:22 ` Mark H Weaver
2018-08-23 2:18 ` Mark H Weaver
2018-08-23 13:54 ` Ludovic Courtès
2018-08-23 19:40 ` Mark H Weaver
2018-08-24 8:45 ` Ludovic Courtès
2018-10-21 18:16 ` Mark H Weaver
2018-10-22 10:10 ` Ludovic Courtès [this message]
[not found] ` <876002dm18.fsf@netris.org>
2018-08-23 13:51 ` Ludovic Courtès
2022-04-04 11:47 ` Calvin Heim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k1manvg8.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=31878@debbugs.gnu.org \
--cc=mhw@netris.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).