From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 87325 invoked by alias); 20 Mar 2019 15:46:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 87316 invoked by uid 89); 20 Mar 2019 15:46:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=gotten X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Mar 2019 15:46:51 +0000 Received: by mail-wr1-f65.google.com with SMTP id q1so3360304wrp.0 for ; Wed, 20 Mar 2019 08:46:50 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id f20sm5388121wrg.91.2019.03.20.08.46.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Mar 2019 08:46:47 -0700 (PDT) Subject: Re: [PATCH] Readline: Cleanup some warnings To: Eli Zaretskii References: <20190130085716.75179-1-alan.hayward@arm.com> <20190131075907.GA313@adacore.com> <3463805B-A8BF-4C20-ACE3-C21AE3F7DB62@arm.com> <20190201080533.GA31043@adacore.com> <877eejvfoq.fsf@tromey.com> <1549047248.2630.7.camel@skynet.be> <310315f8-62ab-2eff-042f-9f2ae9de07da@redhat.com> <87wokxtnlt.fsf@tromey.com> <83h8c1wdr5.fsf@gnu.org> <87imwex333.fsf@tromey.com> <711b6636-b02c-edb2-308d-5fddbf4c33a9@redhat.com> <83ftritydv.fsf@gnu.org> Cc: tom@tromey.com, gdb-patches@sourceware.org From: Pedro Alves Message-ID: <3fc20d2b-5a49-928a-b474-f812f43a2c12@redhat.com> Date: Wed, 20 Mar 2019 15:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <83ftritydv.fsf@gnu.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-03/txt/msg00431.txt.bz2 On 03/19/2019 08:14 PM, Eli Zaretskii wrote: >> Cc: gdb-patches@sourceware.org >> From: Pedro Alves > Not sure if the above refers to what I wanted to say, but: as I'm sure > you know, SIGINT handlers on Windows run in a separate thread, created > by the OS, so a Readline SIGINT handler could ruin in parallel both > with Readline's other code and in parallel with GDB's code, depending > on when exactly did the user type Ctrl-C. In a few cases where it was > important to emulate Posix behavior in order not to step on the troes > of the mainline code, I needed to stop the main thread while the > SIGINT handler was running. Could it be that the code we are > discussing does something similar? > >> But again, why isn't that a readline problem, instead of >> a gdb problem? > > I agree: the right solution would be for the Readline's SIGINT handler > to stop the main thread (e.g., by using SuspendThread). I don't sure how having the SIGINT handler stop the main thread is a 100% correct solution. By the time you stop it, the main thread can well be already running readline code, halfway through updating some data structure, even if the mainline code disabled SIGINT temporarily, with _rl_block_sigint, because by the time the mainline code calls _rl_block_sigint, the SIGINT thread may have already have been spawned. Also, you're not ever supposed to use SuspendThread for synchronization, I believe. MSDN at says: "This function is primarily designed for use by debuggers. It is not intended to be used for thread synchronization." And here it says: "The Suspend­Thread function tells the scheduler to suspend the thread but does not wait for an acknowledgment from the scheduler that the suspension has actually occurred." I think a mutex/lock for synchronization would be a better solution within readline. Make all mainline readline entry points grab a mutex on entry and release it on exit. Likewise, make the readline signal handler hold a mutex around any code that is unsafe to run in parallel with mainline code. Note however, that the readline signal handling code was changed in recent years. IIRC, it used to do unsafe non-async-signal-safe things, but it's gotten better. It's because readline signal handling changed that the gdb.gdb/selftest.exp fails under newer readline. And one of the changes, which I think may be relevant here, is that in callback mode, readline no longer installs its own signal handler. Using my "info signal-dispositions" script (*), when using the system readline (v7): (top-gdb) info signal-dispositions 2 Number Name Description Disposition 2 SIGINT Interrupt handle_sigint(int) in section .text of build-sys-readline/gdb/gdb While with the bundled readline I get: (top-gdb) info signal-dispositions 2 Number Name Description Disposition 2 SIGINT Interrupt rl_signal_handler in section .text of build/gdb/gdb I'm not sure whether readline still installs its own signal handler internally in other situations, but it'd be worth it to check that. Maybe we never run any readline signal handler on Windows at all nowadays with recent readlines, which would simplify things for us, rendering the gdb hack in question obsolete. (*) - http://palves.net/list-active-signal-handlers-with-gdb/ Thanks, Pedro Alves > >> I'm still puzzled on why this isn't a readline issue. Shouldn't >> readline's Windows signal handler be synchronizing with mainline >> code such that if a signal handler is running, mainline calls into >> readline would block? > > Yes, I think so. > >> I think there must be something else to this. > > Maybe. I will try to read that discussion soon.