bug#31878: Module autoloading is not thread safe

[ludo@gnu.org (Ludovic Courtès)]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> reopen 31878
> thanks
>
> Hi Ludovic,
>
> ludo@gnu.org (Ludovic Courtès) writes:
>
>> ludo@gnu.org (Ludovic Courtès) skribis:
>>
>>> ludo@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. <https://bugs.gnu.org/32367>.
>
> 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’.