From patchwork Fri Mar 18 19:18:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 11403 Received: (qmail 53115 invoked by alias); 18 Mar 2016 19:25:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 53037 invoked by uid 89); 18 Mar 2016 19:25:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=1435, callbacks, waits, Hx-languages-length:4740 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 18 Mar 2016 19:25:32 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 9DC744B for ; Fri, 18 Mar 2016 19:18:42 +0000 (UTC) Received: from cascais.lan (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2IJIYk6028091 for ; Fri, 18 Mar 2016 15:18:42 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 08/30] Fix signal handler/event-loop races Date: Fri, 18 Mar 2016 19:18:12 +0000 Message-Id: <1458328714-4938-9-git-send-email-palves@redhat.com> In-Reply-To: <1458328714-4938-1-git-send-email-palves@redhat.com> References: <1458328714-4938-1-git-send-email-palves@redhat.com> GDB's core signal handling suffers from a classical signal handler / mainline code race: int gdb_do_one_event (void) { ... /* First let's see if there are any asynchronous signal handlers that are ready. These would be the result of invoking any of the signal handlers. */ if (invoke_async_signal_handlers ()) return 1; ... /* Block waiting for a new event. (...). */ if (gdb_wait_for_event (1) < 0) return -1; ... } If a signal is delivered while gdb is blocked in the poll/select inside gdb_wait_for_event, then the select/poll breaks with EINTR, we'll loop back around and call invoke_async_signal_handlers. However, if the signal handler runs between invoke_async_signal_handlers and gdb_wait_for_event, gdb_wait_for_event will block, until the next unrelated event... The fix is to a struct serial_event, and register it in the set of files that select/poll in gdb_wait_for_event waits on. The signal handlers that defer work to invoke_async_signal_handlers call mark_async_signal_handler, which is adjusted to also set the new serial event in addition to setting a flag, and is thus now is garanteed to immediately unblock the next gdb_select/poll call, up until invoke_async_signal_handlers is called and the event is cleared. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * event-loop.c: Include "ser-event.h". (async_signal_handlers_serial_event): New global. (async_signals_handler, initialize_async_signal_handlers): New functions. (mark_async_signal_handler): Set async_signal_handlers_serial_event. (invoke_async_signal_handlers): Clear async_signal_handlers_serial_event. * event-top.c (async_init_signals): Call initialize_async_signal_handlers. --- gdb/event-loop.c | 32 +++++++++++++++++++++++++++++++- gdb/event-loop.h | 2 ++ gdb/event-top.c | 2 ++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/gdb/event-loop.c b/gdb/event-loop.c index 0e1cb2b..052d535 100644 --- a/gdb/event-loop.c +++ b/gdb/event-loop.c @@ -21,6 +21,7 @@ #include "event-loop.h" #include "event-top.h" #include "queue.h" +#include "ser-event.h" #ifdef HAVE_POLL #if defined (HAVE_POLL_H) @@ -262,6 +263,28 @@ static int update_wait_timeout (void); static int poll_timers (void); +/* This event is signalled whenever an asynchronous handler needs to + defer an action to the event loop. */ +static struct serial_event *async_signal_handlers_serial_event; + +/* Callback registered with ASYNC_SIGNAL_HANDLERS_SERIAL_EVENT. */ + +static void +async_signals_handler (int error, gdb_client_data client_data) +{ + /* Do nothing. Handlers are run by invoke_async_signal_handlers + from instead. */ +} + +void +initialize_async_signal_handlers (void) +{ + async_signal_handlers_serial_event = make_serial_event (); + + add_file_handler (serial_event_fd (async_signal_handlers_serial_event), + async_signals_handler, NULL); +} + /* Process one high level event. If nothing is ready at this time, wait for something to happen (via gdb_wait_for_event), then process it. Returns >0 if something was done otherwise returns <0 (this @@ -905,6 +928,7 @@ void mark_async_signal_handler (async_signal_handler * async_handler_ptr) { async_handler_ptr->ready = 1; + serial_event_set (async_signal_handlers_serial_event); } /* See event-loop.h. */ @@ -925,13 +949,19 @@ async_signal_handler_is_marked (async_signal_handler *async_handler_ptr) /* Call all the handlers that are ready. Returns true if any was indeed ready. */ + static int invoke_async_signal_handlers (void) { async_signal_handler *async_handler_ptr; int any_ready = 0; - /* Invoke ready handlers. */ + /* We're going to handle all pending signals, so no need to wake up + the event loop again the next time around. Note this must be + cleared _before_ calling the callbacks, to avoid races. */ + serial_event_clear (async_signal_handlers_serial_event); + + /* Invoke all ready handlers. */ while (1) { diff --git a/gdb/event-loop.h b/gdb/event-loop.h index 1b765e1..155dafa 100644 --- a/gdb/event-loop.h +++ b/gdb/event-loop.h @@ -143,3 +143,5 @@ extern void mark_async_event_handler (struct async_event_handler *handler); /* Mark the handler (ASYNC_HANDLER_PTR) as NOT ready. */ extern void clear_async_event_handler (struct async_event_handler *handler); + +extern void initialize_async_signal_handlers (void); diff --git a/gdb/event-top.c b/gdb/event-top.c index 769485f..9fff2be 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -747,6 +747,8 @@ gdb_readline_no_editing_callback (gdb_client_data client_data) void async_init_signals (void) { + initialize_async_signal_handlers (); + signal (SIGINT, handle_sigint); sigint_token = create_async_signal_handler (async_request_quit, NULL);