From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#12471: Avoid some signal-handling races, and simplify. Date: Fri, 21 Sep 2012 00:42:45 -0700 Organization: UCLA Computer Science Department Message-ID: <505C1A75.9030509@cs.ucla.edu> References: <50590626.2070407@cs.ucla.edu> <83lig6yoim.fsf@gnu.org> <505AB299.9090605@cs.ucla.edu> <837grozizw.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1348213424 2821 80.91.229.3 (21 Sep 2012 07:43:44 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 21 Sep 2012 07:43:44 +0000 (UTC) Cc: 12471@debbugs.gnu.org, lekktu@gmail.com To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Sep 21 09:43:48 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TExtl-0000LT-Uu for geb-bug-gnu-emacs@m.gmane.org; Fri, 21 Sep 2012 09:43:42 +0200 Original-Received: from localhost ([::1]:42735 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TExtg-0005Z8-Lf for geb-bug-gnu-emacs@m.gmane.org; Fri, 21 Sep 2012 03:43:36 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:57661) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TExtY-0005YB-Uf for bug-gnu-emacs@gnu.org; Fri, 21 Sep 2012 03:43:34 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TExtX-0007re-6n for bug-gnu-emacs@gnu.org; Fri, 21 Sep 2012 03:43:28 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:36532) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TExtX-0007rZ-3N for bug-gnu-emacs@gnu.org; Fri, 21 Sep 2012 03:43:27 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1TExv4-0001Nv-2g for bug-gnu-emacs@gnu.org; Fri, 21 Sep 2012 03:45:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 21 Sep 2012 07:45:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 12471 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 12471-submit@debbugs.gnu.org id=B12471.13482134815284 (code B ref 12471); Fri, 21 Sep 2012 07:45:01 +0000 Original-Received: (at 12471) by debbugs.gnu.org; 21 Sep 2012 07:44:41 +0000 Original-Received: from localhost ([127.0.0.1]:46078 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TExuj-0001NB-1a for submit@debbugs.gnu.org; Fri, 21 Sep 2012 03:44:41 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:42202) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1TExuf-0001N3-PG for 12471@debbugs.gnu.org; Fri, 21 Sep 2012 03:44:39 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 6C9D639E8013; Fri, 21 Sep 2012 00:43:01 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gTD5N1OvLW3n; Fri, 21 Sep 2012 00:43:00 -0700 (PDT) Original-Received: from [192.168.1.3] (pool-108-23-119-2.lsanca.fios.verizon.net [108.23.119.2]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id A6BA639E800D; Fri, 21 Sep 2012 00:43:00 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120827 Thunderbird/15.0 In-Reply-To: <837grozizw.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:64678 Archived-At: On 09/20/2012 10:44 AM, Eli Zaretskii wrote: > The only volatile variable that is involved is > interrupt_input_blocked, which is a global variable. How does a > function help in this regard? UNBLOCK_INPUT (the macro in the current trunk) accesses that variable four times in the usual case. unblock_input (the function in the proposed patch) accesses it just twice. This lessens the number of race conditions and makes the code run a bit faster. (It doesn't eliminate the races, but that's OK, one bug fix at a time.) unblock_input does fewer accesses by using a local variable 'level'. Local variables like that are cleaner when they're in functions. One can put them into macros too, but then one has the problem of capture. It's a relatively minor thing; it could still be done with a macro. But since the whole thing basically needed to be rewritten anyway, I figured we might as well do it more cleanly, with a fucntion. > can we use 'pthread_kill' as you show above, instead of 'raise'? That would not work on older POSIXish hosts that lack pthread_kill. In contrast, 'raise' should be portable to all C89-or-better hosts. > On Windows, 'kill' is redefined to call 'emacs_abort', when its > first arg is Emacs's own PID and the second arg is SIGABRT. For consistency, how about if Windows also defines 'raise' to call 'emacs_abort', when the signal is SIGABRT? That should fix the problem, and it shouldn't require any change to the mainline code. It could be something as simple as this in a Windows-specific include file, perhaps: #define raise(sig) kill (getpid (), sig) > And I don't want to redefine yet another library function, if it can > be avoided. But it does seem the simpler approach here, overall. > I think this feature of producing a backtrace on a fatal > error will be hated by both users and Emacs maintainers, > but that's another subject for another thread. > Can we do better than that? Ideally, there should be a single > function, the signal handler, which does everything. I tried that initially but the code was bulkier and more error-prone. Each handler needed to have an extra local variable (to save errno) and to call two auxiliary-library functions, one for setup and the other for teardown. In contrast, the proposed approach requires no additional locals and one auxiliary-library function. It's true that it requires two functions for each per-process signal, one for the thread catching the signal and one for the main process, but I find that actually simpler, since it makes it clearer which handler code is run where. Anyway, pointers to signal handlers are standard practice in POSIXish code, and it's not particularly exotic to pass such a pointer around or to call the handler via the pointer. > Or maybe it would work to set up things so that no non-main threads > ever get any signals Yes, that was my first thought too! I'd rather do that; it'd be simpler. > is there some pthread_* function that can > setup a callback to run each time a thread is created? Unfortunately not, which is why I settled with the proposed approach > Given that SIGFPE will not happen due to calculations, how about > changing its handler to call fatal_error_backtrace, rather than signal > a Lisp-level error? Yes, that makes sense, and it's what the proposed patch already does on IEEE hosts. But I see that the code isn't very clear about that. I'll replace it with the following, which I hope makes it clearer. /* Typically SIGFPE is thread-specific and is fatal, like SIGILL. But on a non-IEEE host SIGFPE can come from a trap in the Lisp interpreter's floating point operations, so treat SIGFPE as an arith-error if it arises in the main thread. */ if (IEEE_FLOATING_POINT) sigaction (SIGFPE, &thread_fatal_action, 0); else { emacs_sigaction_init (&action, deliver_arith_signal); sigaction (SIGFPE, &action, 0); }