From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Helmut Eller Newsgroups: gmane.emacs.devel Subject: Re: MPS: a random backtrace while toying with gdb Date: Sun, 30 Jun 2024 20:42:03 +0200 Message-ID: <87v81qp91g.fsf@gmail.com> References: <87bk3jh8bt.fsf@localhost> <87wmm6rcv1.fsf@gmail.com> <86le2mhhsj.fsf@gnu.org> <875xtqramd.fsf@gmail.com> <86cynyhfsn.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="6515"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: gerd.moellmann@gmail.com, pipcet@protonmail.com, yantar92@posteo.net, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Jun 30 20:42:50 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 1sNzVh-0001Wu-QA for ged-emacs-devel@m.gmane-mx.org; Sun, 30 Jun 2024 20:42:49 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sNzV4-0001Td-RJ; Sun, 30 Jun 2024 14:42:10 -0400 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 1sNzV3-0001TI-4i for emacs-devel@gnu.org; Sun, 30 Jun 2024 14:42:09 -0400 Original-Received: from mail-lj1-x230.google.com ([2a00:1450:4864:20::230]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sNzV1-0001Mb-0c; Sun, 30 Jun 2024 14:42:08 -0400 Original-Received: by mail-lj1-x230.google.com with SMTP id 38308e7fff4ca-2ebec2f11b7so25155401fa.2; Sun, 30 Jun 2024 11:42:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1719772925; x=1720377725; darn=gnu.org; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=jnXKSTECvIIAFvpUFii5iIfgMy9IuQOPasxqu8UP/Jg=; b=ZEOkFlF91Qtg8YtKrWtLPfRg7I0XEgfCy+9eTunbBCzSiwrWBbnqBJ/q+3+lNnI0zK TVmNkh+KnT/2akzzdyDmwptsx0Srydr6FGWVhBj1FubE+J5s0jzmk/49QaLqrfq2O06q v2hnsYVbBy1CU5XwF6bFG0MxPTI4m/kUj7dNI5zfTgwnjDzTMSwShctp5jnom8foz/AR 5DAtYDJkeacpKGyPh4PEV4HkergPS0Omb3S2X6h3pipbHg/looYTnuQZtaG863+T2muh kINqYYnatTKDwtrG+7Fm63Pxfo2OX3l3s2Rw5BG72zPYJQSmBHahW7yusENR5lFiiywD pY9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719772925; x=1720377725; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=jnXKSTECvIIAFvpUFii5iIfgMy9IuQOPasxqu8UP/Jg=; b=K1HwLMb5qB6bscNVzSD5cz8JQFVlGEXtK11HAlxJgiclYoyCrepyyhcbHZB4JhUcQC kZh7bmEaScmT9eLJaz8SxlomKuCOxtYJotvXjAL+kr8F1JyG5/h1JzN6VT2P2HoPeci6 5q30t03NC1Ump6hpnD/cXGEI/sHbbKmRfqs3iXly46D1OwcG1yCjk71IM/PT+DEnOG0T NuoGGTDicLpv3R3MZQ+cNTAi98WY7unAJ1RbMTPisJOnWfvShlOhRNpw/pgnPRAQjUGV WSKYLnX0Sal0AW56pKXnq9TNvPJH2LFuc0W60JV0T4CJO7BdoQnRaj+DO4qRi8D2K+NI 1egQ== X-Forwarded-Encrypted: i=1; AJvYcCV0vn8fD5UPyKIJm/GM2oeo79P+PNMNGqFRp8jxk8ZmCncKklurbr8eOxJobkFvrH9WcA0L+N2uXaUjsSVFRbXDblee X-Gm-Message-State: AOJu0Yx8R93Vi6WHMicixVzRRwnsYpvJgGj7pW0HogXhax2M09Y1jS9E BgZkvCDdC9tVDnZISbf/+U0hpa9st2IjgMh3N9hi5cKGKmGhHtvzJIX8pQ== X-Google-Smtp-Source: AGHT+IFGrMVrwiY1IVhu68vYFX4SahHnu1WEIrDSUxbJMEOvIrfchOJucH1NunlenwE5seEylrLWYA== X-Received: by 2002:a2e:b7c9:0:b0:2ec:56b9:259b with SMTP id 38308e7fff4ca-2ee5e6bd1demr20209041fa.49.1719772924388; Sun, 30 Jun 2024 11:42:04 -0700 (PDT) Original-Received: from caladan (dial-187254.pool.broadband44.net. [212.46.187.254]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4256b09abbfsm120205485e9.35.2024.06.30.11.42.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Jun 2024 11:42:03 -0700 (PDT) In-Reply-To: <86cynyhfsn.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 30 Jun 2024 13:43:20 +0300") Received-SPF: pass client-ip=2a00:1450:4864:20::230; envelope-from=eller.helmut@gmail.com; helo=mail-lj1-x230.google.com 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, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action 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:320975 Archived-At: --=-=-= Content-Type: text/plain On Sun, Jun 30 2024, Eli Zaretskii wrote: >> You don't do anything useful other than packing up the arguments that >> the signal handler receives and put them in the queue. > > What arguments are those? SIGCHLD doesn't tell us the PID of the > process (or any other data that could be used to identify the > process), AFAICT. What if two or more sub-processes exited while we > are in MPS-land? This patch below implements the idea I was thinking about. >> The MPS documentation says quite clearly that a scanner must not access >> MPS-managed memory other than the block it receives as argument: > > What do you mean by "scanner"? I was talking about our signal > handlers. If they qualify as "scanner", please explain why. I mean dflt_scan. > Anyway, if a signal handler cannot do anything useful, the only > alternative is to block select signals around code which could violate > those restrictions. We will see. --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Fix-igc_check_fwd.patch >From f4080ec23a0630bd3c5c37c853cd2528faec92c4 Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Sun, 30 Jun 2024 20:15:51 +0200 Subject: [PATCH 1/2] Fix igc_check_fwd * src/igc.c (igc_check_fwd): Add missing argument to has_header. --- src/igc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/igc.c b/src/igc.c index 324bcfa112f..2165a69cacf 100644 --- a/src/igc.c +++ b/src/igc.c @@ -598,7 +598,7 @@ alloc_hash (void) void igc_check_fwd (void *client) { - if (has_header (client)) + if (has_header (client, true)) { struct igc_header *h = client_to_base (client); igc_assert (h->obj_type != IGC_OBJ_FWD); -- 2.39.2 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-Process-SIGCHLD-asynchronously.patch >From 7b8715470bdec2dd0808c530c19ca4b749c6178b Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Sun, 30 Jun 2024 20:17:05 +0200 Subject: [PATCH 2/2] Process SIGCHLD asynchronously Introduce a async_work_queue that is processed as part of process_pending_signals. * src/keyboard.h (struct async_work_item) (enum async_work_type): New types. (enqueue_async_work): New function. * src/keyboard.c (struct async_work_queue): New type. (async_work_queue): New global variable. (async_work_queue_len, async_work_queue_pop_front) (do_async_work, do_async_work_1): New helpers. (enqueue_async_work): Implementation. (process_pending_signals): Call do_async_work; * src/process.h (process_sigchld_async): New function. * src/process.c (process_sigchld_async): Factored out from handle_child_signal. (handle_child_signal): Use enqueue_async_work. --- src/keyboard.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/keyboard.h | 16 +++++++++++++ src/process.c | 62 +++++++++++++++++++++++++++---------------------- src/process.h | 2 ++ 4 files changed, 116 insertions(+), 27 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index 06599ed0fdf..71286df8915 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -374,6 +374,15 @@ #define GROW_RAW_KEYBUF \ menu bar. */ static Lisp_Object menu_bar_touch_id; +#define ASYNC_WORK_QUEUE_CAPACITY 64 + +struct async_work_queue { + uint8_t start, end; + struct async_work_item items[ASYNC_WORK_QUEUE_CAPACITY]; +}; + +static struct async_work_queue async_work_queue; + /* Global variable declarations. */ @@ -415,6 +424,7 @@ #define READABLE_EVENTS_IGNORE_SQUEEZABLES (1 << 2) static char *find_user_signal_name (int); static void store_user_signal_events (void); static bool is_ignored_event (union buffered_input_event *); +static void do_async_work (void); /* Advance or retreat a buffered input event pointer. */ @@ -8176,6 +8186,7 @@ process_pending_signals (void) pending_signals = false; handle_async_input (); do_pending_atimers (); + do_async_work (); } /* Undo any number of BLOCK_INPUT calls down to level LEVEL, @@ -12845,6 +12856,58 @@ is_ignored_event (union buffered_input_event *event) return !NILP (Fmemq (ignore_event, Vwhile_no_input_ignore_events)); } +static uint8_t +async_work_queue_len (struct async_work_queue *q) +{ + uint8_t cap = ASYNC_WORK_QUEUE_CAPACITY; + return (q->start <= q->end + ? q->end - q->start + : cap - q->start + q->end); +} + +static struct async_work_item +async_work_queue_pop_front (struct async_work_queue *q) +{ + if (async_work_queue_len (q) == 0) + emacs_abort (); + struct async_work_item item = q->items[q->start]; + uint8_t cap = ASYNC_WORK_QUEUE_CAPACITY; + q->start = (q->start + 1) % cap; + return item; +} + +void +enqueue_async_work (struct async_work_item item) +{ + struct async_work_queue *q = &async_work_queue; + uint8_t cap = ASYNC_WORK_QUEUE_CAPACITY; + if (async_work_queue_len (q) == cap) + emacs_abort (); + q->items[q->end] = item; + q->end = (q->end + 1) % cap; + pending_signals = 1; +} + +static void +do_async_work_1 (struct async_work_item item) +{ + switch (item.type) + { + case ASYNCWORK_SIGCHLD: + process_sigchld_async (item.u.sigchld); + return; + } + emacs_abort (); +} + +static void +do_async_work (void) +{ + struct async_work_queue *q = &async_work_queue; + while (async_work_queue_len (q) > 0) + do_async_work_1 (async_work_queue_pop_front (q)); +} + static void syms_of_keyboard_for_pdumper (void); void diff --git a/src/keyboard.h b/src/keyboard.h index c7ae1f7f0fa..8fb592dacff 100644 --- a/src/keyboard.h +++ b/src/keyboard.h @@ -527,4 +527,20 @@ kbd_buffer_store_event_hold (struct input_event *event, INLINE_HEADER_END +enum async_work_type +{ + ASYNCWORK_SIGCHLD +}; + +struct async_work_item +{ + enum async_work_type type; + union + { + int sigchld; + } u; +}; + +extern void enqueue_async_work (struct async_work_item item); + #endif /* EMACS_KEYBOARD_H */ diff --git a/src/process.c b/src/process.c index a3b2d2e54bd..5939d16c37e 100644 --- a/src/process.c +++ b/src/process.c @@ -7584,33 +7584,8 @@ child_signal_notify (void) static void dummy_handler (int sig) {} static signal_handler_t volatile lib_child_handler; -/* Handle a SIGCHLD signal by looking for known child processes of - Emacs whose status have changed. For each one found, record its - new status. - - All we do is change the status; we do not run sentinels or print - notifications. That is saved for the next time keyboard input is - done, in order to avoid timing errors. - - ** WARNING: this can be called during garbage collection. - Therefore, it must not be fooled by the presence of mark bits in - Lisp objects. - - ** USG WARNING: Although it is not obvious from the documentation - in signal(2), on a USG system the SIGCLD handler MUST NOT call - signal() before executing at least one wait(), otherwise the - handler will be called again, resulting in an infinite loop. The - relevant portion of the documentation reads "SIGCLD signals will be - queued and the signal-catching function will be continually - reentered until the queue is empty". Invoking signal() causes the - kernel to reexamine the SIGCLD queue. Fred Fish, UniSoft Systems - Inc. - - ** Malloc WARNING: This should never call malloc either directly or - indirectly; if it does, that is a bug. */ - -static void -handle_child_signal (int sig) +void +process_sigchld_async (int sig) { Lisp_Object tail, proc; bool changed = false; @@ -7688,6 +7663,39 @@ handle_child_signal (int sig) /* Wake up `wait_reading_process_output'. */ child_signal_notify (); +} + +/* Handle a SIGCHLD signal by looking for known child processes of + Emacs whose status have changed. For each one found, record its + new status. + + All we do is change the status; we do not run sentinels or print + notifications. That is saved for the next time keyboard input is + done, in order to avoid timing errors. + + ** WARNING: this can be called during garbage collection. + Therefore, it must not be fooled by the presence of mark bits in + Lisp objects. + + ** USG WARNING: Although it is not obvious from the documentation + in signal(2), on a USG system the SIGCLD handler MUST NOT call + signal() before executing at least one wait(), otherwise the + handler will be called again, resulting in an infinite loop. The + relevant portion of the documentation reads "SIGCLD signals will be + queued and the signal-catching function will be continually + reentered until the queue is empty". Invoking signal() causes the + kernel to reexamine the SIGCLD queue. Fred Fish, UniSoft Systems + Inc. + + ** Malloc WARNING: This should never call malloc either directly or + indirectly; if it does, that is a bug. */ + +static void +handle_child_signal (int sig) +{ + struct async_work_item item = {ASYNCWORK_SIGCHLD, .u.sigchld = sig}; + enqueue_async_work (item); + lib_child_handler (sig); #ifdef NS_IMPL_GNUSTEP /* NSTask in GNUstep sets its child handler each time it is called. diff --git a/src/process.h b/src/process.h index f497a64c3d1..1d59a658d95 100644 --- a/src/process.h +++ b/src/process.h @@ -308,4 +308,6 @@ pset_gnutls_cred_type (struct Lisp_Process *p, Lisp_Object val) INLINE_HEADER_END +extern void process_sigchld_async (int sig); + #endif /* EMACS_PROCESS_H */ -- 2.39.2 --=-=-=--