From unknown Mon Jul 07 23:40:14 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#76589 <76589@debbugs.gnu.org> To: bug#76589 <76589@debbugs.gnu.org> Subject: Status: [PATCH] Exclude the finalizer thread from the =?UTF-8?Q?=E2=80=98all-threads=E2=80=99?= result. Reply-To: bug#76589 <76589@debbugs.gnu.org> Date: Tue, 08 Jul 2025 06:40:14 +0000 retitle 76589 [PATCH] Exclude the finalizer thread from the =E2=80=98all-th= reads=E2=80=99 result. reassign 76589 guile submitter 76589 Ludovic Court=C3=A8s severity 76589 normal tag 76589 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Wed Feb 26 09:56:34 2025 Received: (at submit) by debbugs.gnu.org; 26 Feb 2025 14:56:34 +0000 Received: from localhost ([127.0.0.1]:54378 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tnIpt-0006u2-Ks for submit@debbugs.gnu.org; Wed, 26 Feb 2025 09:56:34 -0500 Received: from lists.gnu.org ([2001:470:142::17]:47060) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tnIpp-0006eB-UI for submit@debbugs.gnu.org; Wed, 26 Feb 2025 09:56:31 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tnIpR-0003n5-F4 for bug-guile@gnu.org; Wed, 26 Feb 2025 09:56:05 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tnIpP-000584-Np; Wed, 26 Feb 2025 09:56:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:Subject:To:From:in-reply-to: references; bh=lktRBVMAVUfls8Ua+R4PULAO1LkvDpEED9Ewic7R/9E=; b=l0VVb2UBFghORE ndaM8v7j3MXU89TGMskgZwBxN+ytWvVHAaRrhnhqax/S6b9ASGjJS4OVrOVkoEuVj3uQqmabugvIf U/pPxEyIx/41fep2wv45WhkhXzYck9vctgZ2Y1ldg7yyb5YbI5ueDVArf+jqOS4MBopXSEcfvOp3a JWZ1GZrQjPKVWcf/w9/yyY6O+hHHvxlnEi2EjSYEV7+LrrGqF/YLFGZmsyrxY12PGSZn6ogokuse0 ouxFuQZ0l7DVaOv6TKXnipJd5uFhyxiue2ALqIszpexTNN/dcSNNsdPF7WAfHk9y/xp23rHss5aFA AnucHWVl1pTa0L2R9vig==; From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= To: bug-guile@gnu.org Subject: [PATCH] =?UTF-8?q?Exclude=20the=20finalizer=20thread=20from=20the?= =?UTF-8?q?=20=E2=80=98all-threads=E2=80=99=20result.?= Date: Wed, 26 Feb 2025 15:55:20 +0100 Message-ID: <20250226145520.19492-1-ludo@gnu.org> X-Mailer: git-send-email 2.48.1 X-Debbugs-Cc: Olivier Dion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: submit Cc: Simon Josefsson , =?UTF-8?q?Ludovic=20Court=C3=A8s?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Fixes . Fixes a bug whereby “echo '(environ)' | guile” would wrongfully trigger the multiple-thread warning. * libguile/finalizers.c (finalizer_thread): New variable. (finalization_thread_proc): Set it. (scm_i_is_finalizer_thread): New function. (run_finalization_thread): Clear FINALIZER_THREAD. * libguile/finalizers.h (scm_i_is_finalizer_thread): New declaration. * libguile/threads.c (scm_all_threads): Use it. * NEWS: Update. Reported-by: Simon Josefsson --- NEWS | 8 +++++++- libguile/finalizers.c | 21 ++++++++++++++++++--- libguile/finalizers.h | 5 ++++- libguile/threads.c | 7 +++++-- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 3328a03cf..2a59874d4 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,5 @@ Guile NEWS --- history of user-visible changes. -Copyright (C) 1996-2024 Free Software Foundation, Inc. +Copyright (C) 1996-2025 Free Software Foundation, Inc. See the end for copying conditions. Please send Guile bug reports to bug-guile@gnu.org. @@ -69,6 +69,12 @@ every line in a file. ** Immutable stringbufs are now 8-byte aligned on all systems Previously they could end up with an alignment that violated the type tag for their type (e.g. ending up tagged as immediates SCM_IMP()). +** 'all-threads' no longer includes the finalizer thread + () + Previously 'all-threads' would include the finalizer thread. This, + in turn, would trigger warnings from 'primitive-fork' and 'environ' + suggesting they are being called in a multi-threaded context, when in + fact user code did not create any thread. Changes in 3.0.10 (since 3.0.9) diff --git a/libguile/finalizers.c b/libguile/finalizers.c index 1370755bf..47495b595 100644 --- a/libguile/finalizers.c +++ b/libguile/finalizers.c @@ -1,4 +1,4 @@ -/* Copyright 2012-2014,2018-2020,2022 +/* Copyright 2012-2014,2018-2020,2022,2025 Free Software Foundation, Inc. This file is part of Guile. @@ -37,6 +37,7 @@ #include "gsubr.h" #include "init.h" #include "threads.h" +#include "atomics-internal.h" #include "finalizers.h" @@ -217,10 +218,14 @@ read_finalization_pipe_data (void *data) return NULL; } - + +static scm_i_pthread_t finalizer_thread; + static void* finalization_thread_proc (void *unused) { + scm_atomic_set_pointer ((void **) &finalizer_thread, + (void *) pthread_self ()); while (1) { struct finalization_pipe_data data; @@ -255,10 +260,20 @@ finalization_thread_proc (void *unused) } } +int +scm_i_is_finalizer_thread (struct scm_thread *t) +{ + scm_i_pthread_t us = + (scm_i_pthread_t) scm_atomic_ref_pointer ((void **) &finalizer_thread); + return pthread_equal (t->pthread, us); +} + static void* run_finalization_thread (void *arg) { - return scm_with_guile (finalization_thread_proc, arg); + void *res = scm_with_guile (finalization_thread_proc, arg); + scm_atomic_set_pointer ((void **) &finalizer_thread, NULL); + return res; } static void diff --git a/libguile/finalizers.h b/libguile/finalizers.h index 44bafb22e..a92a74be1 100644 --- a/libguile/finalizers.h +++ b/libguile/finalizers.h @@ -1,7 +1,7 @@ #ifndef SCM_FINALIZERS_H #define SCM_FINALIZERS_H -/* Copyright 2012, 2013, 2014, 2018 +/* Copyright 2012, 2013, 2014, 2018, 2025 Free Software Foundation, Inc. This file is part of Guile. @@ -42,6 +42,9 @@ SCM_INTERNAL void scm_i_finalizer_pre_fork (void); thread. */ SCM_INTERNAL void scm_i_register_async_gc_callback (void (*callback) (void)); +/* Return true if THREAD is the finalizer thread. */ +SCM_INTERNAL int scm_i_is_finalizer_thread (struct scm_thread *thread); + SCM_API int scm_set_automatic_finalization_enabled (int enabled_p); SCM_API int scm_run_finalizers (void); diff --git a/libguile/threads.c b/libguile/threads.c index 77e99da74..6b4510d53 100644 --- a/libguile/threads.c +++ b/libguile/threads.c @@ -1,4 +1,4 @@ -/* Copyright 1995-1998,2000-2014,2018-2019,2023-2024 +/* Copyright 1995-1998,2000-2014,2018-2019,2023-2025 Free Software Foundation, Inc. This file is part of Guile. @@ -47,6 +47,7 @@ #include "dynwind.h" #include "eval.h" #include "extensions.h" +#include "finalizers.h" #include "fluids.h" #include "gc-inline.h" #include "gc.h" @@ -1691,7 +1692,9 @@ SCM_DEFINE (scm_all_threads, "all-threads", 0, 0, 0, for (t = all_threads; t && n > 0; t = t->next_thread) { - if (!t->exited && !scm_i_is_signal_delivery_thread (t)) + if (!t->exited + && !scm_i_is_signal_delivery_thread (t) + && !scm_i_is_finalizer_thread (t)) { SCM_SETCAR (*l, t->handle); l = SCM_CDRLOC (*l); base-commit: f6359a4715d023761454f1bf945633ce4cca98fc -- 2.48.1 From debbugs-submit-bounces@debbugs.gnu.org Wed Feb 26 10:43:09 2025 Received: (at 76589) by debbugs.gnu.org; 26 Feb 2025 15:43:09 +0000 Received: from localhost ([127.0.0.1]:54874 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tnJYz-000130-2X for submit@debbugs.gnu.org; Wed, 26 Feb 2025 10:43:09 -0500 Received: from smtpout.efficios.com ([2607:5300:400:ed00::31e5]:41866) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tnJYw-00012m-CB for 76589@debbugs.gnu.org; Wed, 26 Feb 2025 10:43:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=efficios.com; s=smtpout1; t=1740584585; bh=c/745r9HwBlvZXnroNixqX1v+ay8WYEc93AnBQ6w4To=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=JyQGva+MadnM18ARZQ+C/23yNyMcx/STQRoth4kIAIYzOHc/KjL6C++Kyd47zbnzM IuCLsSZFeTYSAcRkkpZOzq7gGI9hKZb+w/xG3xQ0mNdombsb2oyc3F2r83In139ZHn sFa5+95WiNZPGXMpOz+4MBR/oSR7sJg3jGkw/VfEcdthUai4o/Mgadq+S9mQTHhebH 7ERyM9K2i13H/AMLshS79PC46CqPsm41AmT3i8KGFRRzcpg7YcrQDNkiAJoeMXuODu cHdGqJuiisekTaBqUK9k5ujC3dYxTARiHiCQqV2JHppWye02VCv4yRk1n+euDF7c4g ySdLAlKc09szg== Received: from localhost (157-208-8-209.mc.derytele.com [157.208.8.209]) by smtpout.efficios.com (Postfix) with ESMTPSA id 4Z2zJT2LX8zgN9; Wed, 26 Feb 2025 10:43:05 -0500 (EST) From: Olivier Dion To: Ludovic =?utf-8?Q?Court=C3=A8s?= , 76589@debbugs.gnu.org Subject: Re: bug#76589: [PATCH] Exclude the finalizer thread from the =?utf-8?Q?=E2=80=98all-threads=E2=80=99?= result. In-Reply-To: <20250226145520.19492-1-ludo@gnu.org> Organization: EfficiOS References: <20250226145520.19492-1-ludo@gnu.org> Date: Wed, 26 Feb 2025 10:43:05 -0500 Message-ID: <87frk0sody.fsf@laura> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 76589 Cc: Simon Josefsson , Ludovic =?utf-8?Q?Court=C3=A8s?= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) I think that this change is fine, but it could be better when it comes to testing for multi-threaded environment. First, a Guile internal thread is leaked in the list of threads, which users are not expecting. Perhaps there's code out there that is handling this fine but would break by removing the internal thread. Should the internal thread not be leaked anymore? I think that this question is orthogonal to the problem that this patch aims to solve, that is not emitting a false warning for `environ' and `primitive-fork'. The problem could be solved by keeping an atomic counter of _user created threads_. Increment it when a new thread is created, decrement it when a thread is joined. Read the counter in a predicate `multi-threaded?'. This begs the question whether a multi-threaded environment can become a single-threaded environment again. If not, then keeping a single boolean and storing true in it whenever a user thread is created is enough. Thanks, Olivier On Wed, 26 Feb 2025, Ludovic Court=C3=A8s wrote: > Fixes . > > Fixes a bug whereby =E2=80=9Cecho '(environ)' | guile=E2=80=9D would wron= gfully trigger > the multiple-thread warning. > > * libguile/finalizers.c (finalizer_thread): New variable. > (finalization_thread_proc): Set it. > (scm_i_is_finalizer_thread): New function. > (run_finalization_thread): Clear FINALIZER_THREAD. > * libguile/finalizers.h (scm_i_is_finalizer_thread): New declaration. > * libguile/threads.c (scm_all_threads): Use it. > * NEWS: Update. > > Reported-by: Simon Josefsson > --- > NEWS | 8 +++++++- > libguile/finalizers.c | 21 ++++++++++++++++++--- > libguile/finalizers.h | 5 ++++- > libguile/threads.c | 7 +++++-- > 4 files changed, 34 insertions(+), 7 deletions(-) > > diff --git a/NEWS b/NEWS > index 3328a03cf..2a59874d4 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,5 @@ > Guile NEWS --- history of user-visible changes. > -Copyright (C) 1996-2024 Free Software Foundation, Inc. > +Copyright (C) 1996-2025 Free Software Foundation, Inc. > See the end for copying conditions. >=20=20 > Please send Guile bug reports to bug-guile@gnu.org. > @@ -69,6 +69,12 @@ every line in a file. > ** Immutable stringbufs are now 8-byte aligned on all systems > Previously they could end up with an alignment that violated the type > tag for their type (e.g. ending up tagged as immediates SCM_IMP()). > +** 'all-threads' no longer includes the finalizer thread > + () > + Previously 'all-threads' would include the finalizer thread. This, > + in turn, would trigger warnings from 'primitive-fork' and 'environ' > + suggesting they are being called in a multi-threaded context, when in > + fact user code did not create any thread. >=20=20 > > Changes in 3.0.10 (since 3.0.9) > diff --git a/libguile/finalizers.c b/libguile/finalizers.c > index 1370755bf..47495b595 100644 > --- a/libguile/finalizers.c > +++ b/libguile/finalizers.c > @@ -1,4 +1,4 @@ > -/* Copyright 2012-2014,2018-2020,2022 > +/* Copyright 2012-2014,2018-2020,2022,2025 > Free Software Foundation, Inc. >=20=20 > This file is part of Guile. > @@ -37,6 +37,7 @@ > #include "gsubr.h" > #include "init.h" > #include "threads.h" > +#include "atomics-internal.h" >=20=20 > #include "finalizers.h" >=20=20 > @@ -217,10 +218,14 @@ read_finalization_pipe_data (void *data) >=20=20 > return NULL; > } > -=20=20 > + > +static scm_i_pthread_t finalizer_thread; > + > static void* > finalization_thread_proc (void *unused) > { > + scm_atomic_set_pointer ((void **) &finalizer_thread, > + (void *) pthread_self ()); > while (1) > { > struct finalization_pipe_data data; > @@ -255,10 +260,20 @@ finalization_thread_proc (void *unused) > } > } >=20=20 > +int > +scm_i_is_finalizer_thread (struct scm_thread *t) > +{ > + scm_i_pthread_t us =3D > + (scm_i_pthread_t) scm_atomic_ref_pointer ((void **) &finalizer_threa= d); > + return pthread_equal (t->pthread, us); > +} > + > static void* > run_finalization_thread (void *arg) > { > - return scm_with_guile (finalization_thread_proc, arg); > + void *res =3D scm_with_guile (finalization_thread_proc, arg); > + scm_atomic_set_pointer ((void **) &finalizer_thread, NULL); > + return res; > } >=20=20 > static void > diff --git a/libguile/finalizers.h b/libguile/finalizers.h > index 44bafb22e..a92a74be1 100644 > --- a/libguile/finalizers.h > +++ b/libguile/finalizers.h > @@ -1,7 +1,7 @@ > #ifndef SCM_FINALIZERS_H > #define SCM_FINALIZERS_H >=20=20 > -/* Copyright 2012, 2013, 2014, 2018 > +/* Copyright 2012, 2013, 2014, 2018, 2025 > Free Software Foundation, Inc. >=20=20 > This file is part of Guile. > @@ -42,6 +42,9 @@ SCM_INTERNAL void scm_i_finalizer_pre_fork (void); > thread. */ > SCM_INTERNAL void scm_i_register_async_gc_callback (void (*callback) (vo= id)); >=20=20 > +/* Return true if THREAD is the finalizer thread. */ > +SCM_INTERNAL int scm_i_is_finalizer_thread (struct scm_thread *thread); > + > SCM_API int scm_set_automatic_finalization_enabled (int enabled_p); > SCM_API int scm_run_finalizers (void); >=20=20 > diff --git a/libguile/threads.c b/libguile/threads.c > index 77e99da74..6b4510d53 100644 > --- a/libguile/threads.c > +++ b/libguile/threads.c > @@ -1,4 +1,4 @@ > -/* Copyright 1995-1998,2000-2014,2018-2019,2023-2024 > +/* Copyright 1995-1998,2000-2014,2018-2019,2023-2025 > Free Software Foundation, Inc. >=20=20 > This file is part of Guile. > @@ -47,6 +47,7 @@ > #include "dynwind.h" > #include "eval.h" > #include "extensions.h" > +#include "finalizers.h" > #include "fluids.h" > #include "gc-inline.h" > #include "gc.h" > @@ -1691,7 +1692,9 @@ SCM_DEFINE (scm_all_threads, "all-threads", 0, 0, 0, >=20=20 > for (t =3D all_threads; t && n > 0; t =3D t->next_thread) > { > - if (!t->exited && !scm_i_is_signal_delivery_thread (t)) > + if (!t->exited > + && !scm_i_is_signal_delivery_thread (t) > + && !scm_i_is_finalizer_thread (t)) > { > SCM_SETCAR (*l, t->handle); > l =3D SCM_CDRLOC (*l); > > base-commit: f6359a4715d023761454f1bf945633ce4cca98fc > --=20 > 2.48.1 > > > --=20 Olivier Dion EfficiOS Inc. https://www.efficios.com