bug#33014: 26.1.50; 27.0.50; Fatal error after re-evaluating a thread's function

[Gemini Lasswell]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Anyway, are you saying that stack marking doesn't work in optimized
>> > code?  We've been using this technique for the last 17 years without
>> > problems; why would the fact that we have more than one thread change
>> > that?  The same arguments you submit are valid for a single-threaded
>> > Emacs, right?
>> 
>> Apparently so.  I set up a single-threaded situation where I could
>> redefine a function while exec_byte_code was running it, and got a
>> segfault.  I've gained some insights from debugging this version of the
>> bug which I will put into a separate email.
>
> If this is the case, then I think we should protect the definition of
> a running function from GC, in some way, either by making sure it is
> referenced by some stack-based Lisp object, even in heavily optimized
> code (e.g., by using 'volatile' qualifiers); or by some other method
> that will ensure that definition is marked and not swept.

Maybe code optimizers have improved over the last 17 years?

I have patched Emacs with a 'volatile' on the definition of 'fun' in
Ffuncall, and so far haven't managed to reproduce the bug with it:

>From a1fc2dfd392e0ba8754159d855da231a56ca275b Mon Sep 17 00:00:00 2001
From: Gemini Lasswell <gazally@runbox.com>
Date: Sun, 14 Oct 2018 12:12:04 -0700
Subject: [PATCH] * src/eval.c (Ffuncall): Make local variable 'fun' volatile
 (bug#33014)

---
 src/eval.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 5e25caaa84..75b30f9c7d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2817,8 +2817,8 @@ Thus, (funcall \\='cons \\='x \\='y) returns (x . y).
 usage: (funcall FUNCTION &rest ARGUMENTS)  */)
   (ptrdiff_t nargs, Lisp_Object *args)
 {
-  Lisp_Object fun, original_fun;
-  Lisp_Object funcar;
+  Lisp_Object volatile fun;
+  Lisp_Object original_fun, funcar;
   ptrdiff_t numargs = nargs - 1;
   Lisp_Object val;
   ptrdiff_t count;
-- 
2.16.4

I'll go back now to working on my benchmarking project which I hope
someday will make it easy to see if that 'volatile' causes measurable
harm to performance.  I'll also keep using 'eval-region' and 'eval-buffer'
while I have threads running byte-compiled functions which get redefined
by doing that, and report back here if I encounter this bug again.