• Philipp Stephani's avatar
    Fix incorrect handling of module runtime and environment pointers. · 23974cfa
    Philipp Stephani authored
    We used to store module runtime and environment pointers in the static
    lists Vmodule_runtimes and Vmodule_environments.  However, this is
    incorrect because these objects have to be kept per-thread.  With this
    naive approach, interleaving module function calls in separate threads
    leads to environments being removed in the wrong order, which in turn
    can cause local module values to be incorrectly garbage-collected.
    The fix isn't completely trivial: specbinding the lists wouldn't work
    either, because then the garbage collector wouldn't find the
    environments in other threads than the current ones, again leading to
    objects being garbage-collected incorrectly.  While introducing custom
    pseudovector types would fix this, it's simpler to put the runtime and
    environment pointers into the specbinding list as new specbinding
    kinds.  This works since we need to unwind them anyway, and we only
    ever treat the lists as a stack.  The thread switching machinery
    ensures that the specbinding lists are thread-local, and that all
    elements of the specbinding lists in all threads are marked during
    garbage collection.
    
    Module assertions now have to walk the specbinding list for the
    current thread, which is more correct since they now only find
    environments for the current thread.  As a result, we can now remove
    the faulty Vmodule_runtimes and Vmodule_environments variables
    entirely.
    
    Also add a unit test that exemplifies the problem.  It interleaves two
    module calls in two threads so that the first call ends while the
    second one is still active.  Without this change, this test triggers
    an assertion failure.
    
    * src/lisp.h (enum specbind_tag): Add new tags for module runtimes and
    environments.
    
    * src/eval.c (record_unwind_protect_module): New function to record a
    module object in the specpdl list.
    (do_one_unbind): Unwind module objects.
    (backtrace_eval_unrewind, default_toplevel_binding, lexbound_p)
    (Fbacktrace__locals): Deal with new specbinding types.
    (mark_specpdl): Mark module environments as needed.
    
    * src/alloc.c (garbage_collect): Remove call to 'mark-modules'.
    Garbage collection of module values is now handled as part of marking
    the specpdl of each thread.
    
    * src/emacs-module.c (Fmodule_load, funcall_module): Use specpdl to
    record module runtimes and environments.
    (module_assert_runtime, module_assert_env, value_to_lisp): Walk
    through specpdl list instead of list variables.
    (mark_module_environment): Rename from 'mark_modules'.  Don't attempt
    to walk though current thread's environments only, since that would
    miss other threads.
    (initialize_environment, finalize_environment): Don't change
    Vmodule_environments variable; environments are now in the specpdl
    list.
    (finalize_environment_unwind, finalize_runtime_unwind): Make 'extern'
    since do_one_unbind now calls them.
    (finalize_runtime_unwind): Don't change Vmodule_runtimes variable;
    runtimes are now in the specpdl list.
    (syms_of_module): Remove Vmodule_runtimes and Vmodule_environments.
    
    * test/data/emacs-module/mod-test.c (Fmod_test_funcall): New test
    function.
    (emacs_module_init): Bind it.
    
    * test/src/emacs-module-tests.el (emacs-module-tests--variable): New
    helper type to guard access to state in a thread-safe way.
    (emacs-module-tests--wait-for-variable)
    (emacs-module-tests--change-variable): New helper functions.
    (emacs-module-tests/interleaved-threads): New unit test.
    23974cfa
alloc.c 210 KB