bug#62691: Calling system* in the module body hangs Guile, while calling open-pipe* does not

[Timothy Sample]

Timothy Sample <samplet@ngyro.com> writes:

> It turns out that Guile cannot start its signal handling thread during
> module resolution.

I should add that merely using 'call-with-new-thread' in a module being
imported is enough to trigger deadlock.

> A simple fix would be to use the underlying ‘setter’ generic directly:
>
>   (setter thread-join-data thread (cons cv mutex))
>
> It’s hard to read (IMO), but maybe with a comment it’s good enough.

Here’s the patch.

I’m not super familiar with Guile’s threading code, so maybe this quick
fix is a little too quick.  That is, maybe it would be better to avoid
object properties altogether, or maybe we could make other changes to
the way that the new thread signals the calling thread.  We could also
think through the usage of the ‘autoload’ mutex.  The benefit of this
patch is that it is (almost) exactly the same as the old code, so it’s
unlikely to result in nasty surprises.

>From 6b6792c7a21de9be1825719bfca0f01177381cf9 Mon Sep 17 00:00:00 2001
From: Timothy Sample <samplet@ngyro.com>
Date: Tue, 11 Apr 2023 10:22:46 -0600
Subject: [PATCH] Avoid module resolution in 'call-with-new-thread'.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes <https://bugs.gnu.org/62691>.
Reported by Михаил Бахтерев <mike.bakhterev@gmail.com>.

* module/ice-9/threads.scm (call-with-new-thread): Do not use 'set!'
to set object properties while the calling thread is waiting on the
new thread to initialize.
---
 module/ice-9/threads.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/module/ice-9/threads.scm b/module/ice-9/threads.scm
index 5a13cec1d..048d8b085 100644
--- a/module/ice-9/threads.scm
+++ b/module/ice-9/threads.scm
@@ -151,7 +151,15 @@ Once @var{thunk} or @var{handler} returns, the return 
value is made the
                  (lambda ()
                    (lock-mutex mutex)
                    (set! thread (current-thread))
-                   (set! (thread-join-data thread) (cons cv mutex))
+                   ;; Rather than use the 'set!' syntax here, we use the
+                   ;; underlying 'setter' generic function to set the
+                   ;; 'thread-join-data' property on 'thread'.  This is
+                   ;; because 'set!' will try to resolve 'setter' in the
+                   ;; '(guile)' module, which means acquiring the
+                   ;; 'autoload' mutex.  If the calling thread is
+                   ;; already holding that mutex, this will result in
+                   ;; deadlock.  See <https://bugs.gnu.org/62691>.
+                   ((setter thread-join-data) thread (cons cv mutex))
                    (signal-condition-variable cv)
                    (unlock-mutex mutex)
                    (call-with-unblocked-asyncs
-- 
2.39.2