From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Pip Cet via "Emacs development discussions." Newsgroups: gmane.emacs.devel Subject: Re: SIGPROF + SIGCHLD and igc Date: Sun, 29 Dec 2024 20:44:07 +0000 Message-ID: <87bjwu2pb6.fsf@protonmail.com> References: <87o713wwsi.fsf@telefonica.net> <86bjwwulnc.fsf@gnu.org> <877c7jlxsu.fsf@gmail.com> <86frm7sx4d.fsf@gnu.org> <87a5cfoivh.fsf@gmail.com> <87r05reh9t.fsf@protonmail.com> <87msgfmvrp.fsf@gmail.com> <87bjwvedzn.fsf@protonmail.com> <86ttanqc0p.fsf@gnu.org> Reply-To: Pip Cet Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31726"; mail-complaints-to="usenet@ciao.gmane.io" Cc: eller.helmut@gmail.com, gerd.moellmann@gmail.com, ofv@wanadoo.es, emacs-devel@gnu.org, acorallo@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Dec 30 04:21:22 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1tS6LI-00086h-9i for ged-emacs-devel@m.gmane-mx.org; Mon, 30 Dec 2024 04:21:20 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tS6KJ-0008QH-TV; Sun, 29 Dec 2024 22:20:19 -0500 Original-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 1tS092-0003on-Sr for emacs-devel@gnu.org; Sun, 29 Dec 2024 15:44:16 -0500 Original-Received: from mail-4322.protonmail.ch ([185.70.43.22]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tS090-0000vI-RT for emacs-devel@gnu.org; Sun, 29 Dec 2024 15:44:16 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1735505052; x=1735764252; bh=kdjZWLq4SOcLalGf6cUB63bA+N00FJgSfDYbrDJOeBc=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post; b=Yb8HwpRKLqH72uW5rSKrXm869VRZiL76eMjGeAk4YLoouKD39FxcOBRBXy57XFhuE 41nvQnm2wU5JEL/JkYLj4pna1endhJV9G5DXKbgrqh/jeiyTrYwwxETfejeVK+OtI9 UTHwWQp7QpwMsTIw1KXmkxd/ZlqrQhVIruLbpKKD0G3wJC4dXGhyPGqOKQHNn/VSHL BqMhncb5RUneyZaLSjO23xZIFroKaNalhLxme0VktlJ8gM0+VNLrrSrv43bSIy4qgl IlXe8JUD7yBc/o+Irkp24S3CN/vFTS+1CWc+aJsNxfJ64uz39xx53ijIRW51Y+rnEi h6bJG38V7GTdA== In-Reply-To: <86ttanqc0p.fsf@gnu.org> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 7de44d27d8e78e9623fba723a3212b7eb47e1c0a Received-SPF: pass client-ip=185.70.43.22; envelope-from=pipcet@protonmail.com; helo=mail-4322.protonmail.ch X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Sun, 29 Dec 2024 22:20:18 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:327382 Archived-At: Pip Cet writes: > Thanks, I got the idea now! > > Yes, that would work, assuming SIGPROF is never blocked, and that no > signal interrupting the SIGPROF handler attempts to take the arena lock. > (Also, we assume the SIGPROF handler has finished by the time the next > SIGPROF is sent. I think this assumption is fixable, though, and we'd > need to fix it for other signals). Now I've had some time to think about it, I really like this idea, and think it's our best bet. Thanks! Here's a sketch. The idea is that all signals go through the signal receiver thread, which serializes them, picks one, enters the arena, tells the main thread that this signal is now safe to run, re-kills the main thread, waits for confirmation, leaves the arena, waits for confirmation AGAIN, then goes back to sleep. However, the re-kill cannot happen just with the same signal, because we need to block that in the main thread, in the case of SIGCHLD (and who knows which other signals are level-triggered in this way, so just do it for all of them). Therefore, we kill the main thread twice, once with SIGURG (no particular reason for that choice) to tell it to unblock signals and then with the original signal to mark it pending. (It would be nicer to raise() the signal in its signal handler after blocking it, but IIRC that doesn't work). Assumes atomicity and visibility of stores, as does the rest of Emacs. This also assumes the signal isn't supposed to be blocked by the time we receive the SIGURG. If we need that, we need a shadow signal mask, I guess. Pip diff --git a/src/igc.c b/src/igc.c index 4b59352681b..a07e331b792 100644 --- a/src/igc.c +++ b/src/igc.c @@ -814,8 +814,57 @@ IGC_DEFINE_LIST (igc_thread); /* The real signal mask we want to restore after handling pending * signals. */ sigset_t signal_mask; + + pthread_t receiver_thread_id; + int signal_receiver_waiting; + int signal_receiver_pipe[2]; }; =20 +#define SIGNAL_HANDLER_STARTED 'A' +#define SIGNAL_HANDLER_FINISHED 'Z' + +extern pthread_t main_thread_id; + +extern void ArenaEnter (mps_arena_t arena); +extern void ArenaLeave (mps_arena_t arena); + +static void *signal_receiver_thread(void *gc_v) +{ + struct igc *gc =3D gc_v; + sigset_t full_mask; + sigfillset (&full_mask); + pthread_sigmask (SIG_SETMASK, &full_mask, NULL); + + while (true) + { + sigset_t empty_mask; + sigemptyset (&empty_mask); + sigsuspend (&empty_mask); + + while (gc->signals_pending) +=09{ +=09 gc->signals_pending =3D 0; + +=09 for (int i =3D 0; i < ARRAYELTS (gc->pending_signals); i++) +=09 if (gc->pending_signals[i]) +=09 { +=09=09ArenaEnter (gc->arena); +=09=09gc->signal_receiver_waiting =3D i; +=09=09pthread_kill (main_thread_id, SIGURG); +=09=09pthread_kill (main_thread_id, i); +=09=09char c; +=09=09while (read (gc->signal_receiver_pipe[0], &c, 1) !=3D 1); +=09=09if (c !=3D SIGNAL_HANDLER_STARTED) +=09=09 emacs_abort (); +=09=09ArenaLeave (gc->arena); +=09=09while (read (gc->signal_receiver_pipe[0], &c, 1) !=3D 1); +=09=09if (c !=3D SIGNAL_HANDLER_FINISHED) +=09=09 emacs_abort (); +=09 } +=09} + } +} + static bool process_one_message (struct igc *gc); =20 /* The global registry. */ @@ -4618,6 +4667,10 @@ make_igc (void) root_create_exact_ptr (gc, ¤t_thread); root_create_exact_ptr (gc, &all_threads); =20 + if (pipe (gc->signal_receiver_pipe)) + emacs_abort (); + pthread_create (&gc->receiver_thread_id, NULL, signal_receiver_thread, g= c); + enable_messages (gc, true); return gc; } @@ -4921,36 +4974,55 @@ gc_signal_handler_can_run (int sig) { eassume (sig >=3D 0); eassert (sig < ARRAYELTS (global_igc->pending_signals)); - if (igc_busy_p ()) + if (pthread_equal (pthread_self (), global_igc->receiver_thread_id)) + return false; + else if (pthread_equal (pthread_self (), main_thread_id)) { - sigset_t sigs; - global_igc->signals_pending =3D 1; - global_igc->pending_signals[sig] =3D 1; - sigemptyset (&sigs); - sigaddset (&sigs, sig); - pthread_sigmask (SIG_BLOCK, &sigs, NULL); - /* We cannot raise (sig) here, because there are platforms where - * it doesn't work. */ - return false; + if (global_igc->signal_receiver_waiting !=3D sig) +=09{ +=09 sigset_t sigs; +=09 global_igc->signals_pending =3D 1; +=09 global_igc->pending_signals[sig] =3D 1; +=09 sigemptyset (&sigs); +=09 sigaddset (&sigs, sig); +=09 pthread_sigmask (SIG_BLOCK, &sigs, NULL); +=09 pthread_kill (global_igc->receiver_thread_id, sig); +=09 return false; +=09} + else if (sig =3D=3D SIGURG) +=09{ +=09 sigset_t sigs; +=09 sigemptyset (&sigs); +=09 for (int i =3D 0; i < ARRAYELTS (global_igc->pending_signals); i++) +=09 if (global_igc->pending_signals[i]) +=09 sigaddset (&sigs, i); +=09 pthread_sigmask (SIG_UNBLOCK, &sigs, NULL); +=09 return false; +=09} + else +=09global_igc->signal_receiver_waiting =3D 0; + char c =3D SIGNAL_HANDLER_STARTED; + while (write (global_igc->signal_receiver_pipe[1], &c, 1) !=3D 1); } return true; } =20 +bool +gc_signal_handler_finished (int sig) +{ + if (pthread_equal (pthread_self (), main_thread_id)) + { + char c =3D SIGNAL_HANDLER_FINISHED; + while (write (global_igc->signal_receiver_pipe[1], &c, 1) !=3D 1); + } + + return true; +} + /* Called from `maybe_quit'. This assumes no signals are blocked. */ void gc_maybe_quit (void) { - while (global_igc->signals_pending) - { - global_igc->signals_pending =3D 0; - for (int i =3D 0; i < ARRAYELTS (global_igc->pending_signals); i++) -=09if (global_igc->pending_signals[i]) -=09 { -=09 global_igc->pending_signals[i] =3D 0; -=09 raise (i); -=09 } - pthread_sigmask (SIG_SETMASK, &global_igc->signal_mask, NULL); - } } =20 DEFUN ("igc--add-extra-dependency", Figc__add_extra_dependency, diff --git a/src/lisp.h b/src/lisp.h index 48585c2d8a1..c6c7f772e43 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -48,6 +48,7 @@ #define EMACS_LISP_H union gc_header; extern void gc_maybe_quit (void); extern bool gc_signal_handler_can_run (int); +extern bool gc_signal_handler_finished (int); #else INLINE void gc_maybe_quit (void) { @@ -57,6 +58,10 @@ #define EMACS_LISP_H { return true; } +INLINE bool gc_signal_handler_finished (int sig) +{ + return true; +} union gc_header { }; #endif =20 diff --git a/src/sysdep.c b/src/sysdep.c index 9270bfa97d9..0eb1466b6a4 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -1719,7 +1719,7 @@ emacs_sigaction_init (struct sigaction *action, signa= l_handler_t handler) } =20 #ifdef FORWARD_SIGNAL_TO_MAIN_THREAD -static pthread_t main_thread_id; +pthread_t main_thread_id; #endif =20 /* SIG has arrived at the current process. Deliver it to the main @@ -1759,6 +1759,7 @@ deliver_process_signal (int sig, signal_handler_t han= dler) handler (sig); =20 errno =3D old_errno; + gc_signal_handler_finished (sig); } =20 /* Static location to save a fatal backtrace in a thread.