bug#31879: Caching in the "module import obarray" is not thread-safe

bug#31879: Caching in the "module import obarray" is not thread-safe

[Ludovic Courtès]

Dear thread lovers,

The code below spawns a bunch of threads that look up imported bindings
in a module.  It usually enters an infinite loop and possibly eats all
your memory (on Guile 2.2.3):

--8<---------------cut here---------------start------------->8---
(use-modules (ice-9 threads)
             (ice-9 match)
             (srfi srfi-1))

(define bindings
  ;; Bindings exported by (guile).
  '())

(module-for-each (lambda (name var)
                   (set! bindings (cons name bindings)))
                 (resolve-module '(guile)))

(define thread-count 8)
(define bindings-per-thread
  (floor (/ (length bindings) thread-count)))
(define iface
  (resolve-module '(ice-9 q)))

(define threads
  (unfold (lambda (x) (>= x thread-count))
          (lambda (n)
            (call-with-new-thread
             (lambda ()
               (let loop ((bindings (take (drop bindings
                                                (* n bindings-per-thread))
                                          bindings-per-thread)))
                 (match bindings
                   (() #t)
                   ((head . tail)
                    (module-variable iface head)
                    (loop tail)))))))
          1+
          0))

(for-each join-thread threads)
(pk 'done thread-count)
--8<---------------cut here---------------end--------------->8---

The issue lies in the “import obarray”, which is used as a cache and is
accessed in a non-thread-safe manner:

--8<---------------cut here---------------start------------->8---
static inline SCM
module_imported_variable (SCM module, SCM sym)
{
#define SCM_BOUND_THING_P scm_is_true
  register SCM var, imports;

  /* Search cached imported bindings.  */
  imports = SCM_MODULE_IMPORT_OBARRAY (module);
  var = scm_hashq_ref (imports, sym, SCM_UNDEFINED);
  if (SCM_BOUND_THING_P (var))
    return var;

[...]

    if (SCM_BOUND_THING_P (found_var))
      {
	/* Save the lookup result for future reference.  */
	(void) scm_hashq_set_x (imports, sym, found_var);
	return found_var;
      }
--8<---------------cut here---------------end--------------->8---

Possible solutions:

  1. Add a field in the module record containing a mutex.  Downside is
     that this breaks the ABI, though I’m not sure how much of a problem
     it is.

  2. Make the import obarray a weak hash table since they are
     thread-safe in 2.2.  This should be semantically equivalent to
     using a hash table provided interned symbols are not GC’d.

Thoughts?

Ludo’.

bug#31879: Caching in the "module import obarray" is not thread-safe

[Ludovic Courtès]

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

> The issue lies in the “import obarray”, which is used as a cache and is
> accessed in a non-thread-safe manner:

Fixed in commit 46bcbfa566de19a88e925bd0369e110cae5a6b03, following a
suggestion by Andy.

Ludo’.