From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20106 invoked by alias); 20 Mar 2019 17:50:42 -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 19902 invoked by uid 89); 20 Mar 2019 17:50:42 -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=_and_ X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Mar 2019 17:50:40 +0000 Received: by mail-wr1-f67.google.com with SMTP id w2so3738801wrt.11 for ; Wed, 20 Mar 2019 10:50:40 -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 z14sm3810756wrv.78.2019.03.20.10.50.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Mar 2019 10:50:37 -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> <3fc20d2b-5a49-928a-b474-f812f43a2c12@redhat.com> <83o965sax9.fsf@gnu.org> Cc: tom@tromey.com, gdb-patches@sourceware.org From: Pedro Alves Message-ID: <5751c371-3313-fc24-e4d7-83214eb99d0b@redhat.com> Date: Wed, 20 Mar 2019 17:50: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: <83o965sax9.fsf@gnu.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-03/txt/msg00437.txt.bz2 On 03/20/2019 05:38 PM, Eli Zaretskii wrote: >> Cc: tom@tromey.com, gdb-patches@sourceware.org >> From: Pedro Alves >> Date: Wed, 20 Mar 2019 15:46:47 +0000 >> >>> 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. > > How is this different from what happens on Posix platforms, where the > SIGINT handler can be invoked at any moment, while Readline might be > doing anything at all? On Posix platforms the signal handler runs in the same thread as the mainline code. There's _never_ any parallel execution. When the mainline code calls _rl_block_sigint, you're absolutely sure that the mainline code that follows will not be running in parallel with a SIGINT handler. > >> Also, you're not ever supposed to use SuspendThread for synchronization, I >> believe. > > We are also not supposed to mix CRT and Win32 API functions, but we do > that all the time. That's not at all the same thing, I'm afraid. We're not talking about "in theory you shouldn't, but in practice it's OK.". We're talking about the very real fact that you cannot use it to synchronize correctly, as shown in the blog post I pasted below. If you're using the function to pause a thread, and then do things, but the thread doesn't actually pause before you do things, then you have a problem. > >> 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." > > GNU Make does it for many years, and I have yet to hear a single > complaint about that part. Not hearing a complaint does not mean that the problem isn't real. It probably simply means that the race window is narrow, so the number of people that observe any issue, _and_ are willing to report an issue is small, and also, is someone notices something odd, it may be simply impossible to tell what happened and realize what the problem was, after the fact. > >> 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. > > That's fine with me, but is much more complex, and will probably slow > down the code. I won't object if you want to do it that way, though. > >> 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. > > Maybe we should bring Chet into this discussion. Thanks, Pedro Alves