From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: `message' not outputting the newline "atomically" Date: Sat, 6 Jul 2019 18:16:03 -0700 Organization: UCLA Computer Science Department Message-ID: <73e8afc0-2151-9c8b-26c5-454fcd2f361d@cs.ucla.edu> References: <07619925-e367-fb88-2dd8-27addb2e9052@grinta.net> <68b398b1-3790-b32f-535d-6ea2518f79b8@cs.ucla.edu> <83pnn1lkej.fsf@gnu.org> <83tvccjrpo.fsf@gnu.org> <83zhm3i285.fsf@gnu.org> <7a39d680-6234-1301-74e5-62d599f500f6@cs.ucla.edu> <838stfd0pp.fsf@gnu.org> <835zojd0fx.fsf@gnu.org> <834l43czz8.fsf@gnu.org> <5c979663-9ded-4273-11d1-483345053a6f@cs.ucla.edu> <831rz7cvoh.fsf@gnu.org> <540324b5-fad3-ded0-5018-6abb494ce126@cs.ucla.edu> <83blyaaq5l.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------C34261F12FC511A7328B2C98" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="225786"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 Cc: larsi@gnus.org, daniele@grinta.net, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Jul 07 03:16:37 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hjvnA-000wcD-Mq for ged-emacs-devel@m.gmane.org; Sun, 07 Jul 2019 03:16:36 +0200 Original-Received: from localhost ([::1]:33542 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hjvn9-0004p3-2r for ged-emacs-devel@m.gmane.org; Sat, 06 Jul 2019 21:16:35 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:59417) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hjvmx-0004ok-G5 for emacs-devel@gnu.org; Sat, 06 Jul 2019 21:16:25 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hjvmv-0005wR-DB for emacs-devel@gnu.org; Sat, 06 Jul 2019 21:16:23 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:47426) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hjvmp-0005Ze-Ua; Sat, 06 Jul 2019 21:16:16 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 06F0F161DBD; Sat, 6 Jul 2019 18:16:06 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id H8rcx3keAZ6m; Sat, 6 Jul 2019 18:16:04 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 69CBF16260C; Sat, 6 Jul 2019 18:16:04 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 0s-NKRN6pfu4; Sat, 6 Jul 2019 18:16:04 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 2CBE4161DBD; Sat, 6 Jul 2019 18:16:04 -0700 (PDT) In-Reply-To: <83blyaaq5l.fsf@gnu.org> Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:238388 Archived-At: This is a multi-part message in MIME format. --------------C34261F12FC511A7328B2C98 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Eli Zaretskii wrote: > One way of fixing message3_nolog and vmessage would be to use in them > that additional line-buffer stream you defined in your patch. That would be easy enough to do; see the attached patch. However, although it's good to fix the problem for these two functions, we should also fix the problem where it occurs elsewhere. Please see below for more about this. >> As the above example shows, it's quite possible to output more than what >> GNU/Linux fprintf does. So, if "as much as possible" is a hard >> requirement, Emacs must stop using fprintf to format diagnostics like >> the above. > > I meant as much as possible by just using fprintf. I expect that's not precisely what you meant, as it's quite possible to just use fprintf to output more than what Emacs currently would do if Emacs had a serious bug in its error-printing code. For example, we could write our own printf-like function that does its actual work via 'fprintf (stderr, "%c" C)' where C is the next byte to output. That would just use fprintf for I/O, and on GNU/Linux it would output more than what we're doing now, for hypothetical buggy code such as the example I gave. What I *think* you meant is that you want code that outputs diagnostics conveniently, via functions like 'fprintf' and 'message' and 'fatal' etc., to output as much as possible within the constraints of (a) not significantly obstructing convenience and (b) fixing the line-interleaving bugs somehow. I could be wrong, though. > I was certain what I wrote will not be interpreted > in this way. I'm afraid that you were mistaken there. Even now I'm not sure I understand exactly what you meant. It is an area where it's tough to be precise. > using the above for diagnostic output, or for informative > messages output during an Emacs build, is unrelated to the original > problem raised by Lars. That problem was with messages from a running > Emacs, outside of the build process (because the build process use > case can be handled by the Make's -O switch), i.e. when Emacs is run > as part of some script. Sorry, I'm not following you here, as Lars's original email was about messages generated by Emacs running inside the build process. We cannot rely on 'make -O' to address the problem, as 'make -O' slows down development, which is why I typically don't use 'make -O' and don't recommend it for building Emacs interactively. 'make -O' can be useful for buildbots where there's a batch process that waits for build completion before publishing it. However, under an interactive Emacs where I want to act on the first diagnostic right away (before the build finishes), 'make -O' is a net minus because it can delay diagnostics significantly, and this delay costs more than it's worth. And even if we assumed 'make -O' sufficed for builds (which it doesn't), we can't assume these diagnostics are generated from a single instance of GNU Make. It's reasonably common, for example, that from a terminal window I'll run the shell command "emacs &", and then do other stuff in that window while Emacs occasionally outputs stderr diagnostics. (Although I think this is the sort of thing you're alluding to above, I'm not sure.) So we would have an interleaving problem anyway even if 'make -O' worked well. > I reviewed the list, and I think only message3_nolog and > vmessage need to be changed. All the rest are fatal error messages > and debugging diagnostics of sorts, and quite a few of them even > conditioned by some CPP symbol that is only defined in debugging > builds. Unfortunately even fatal error messages and debugging diagnostics can be output in circumstances such as the above, and so can be interleaved confusingly. So the other items in that list still need fixing. Plus, there are other interleaved-I/O problems not in that list; as I mentioned, the list is only partial. So the attached patch does not suffice to fix the problem. --------------C34261F12FC511A7328B2C98 Content-Type: text/x-patch; name="0001-Avoid-interleaving-stderr-in-a-few-cases.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Avoid-interleaving-stderr-in-a-few-cases.patch" >From b88da6aca8d12d0aee9703e54eee4f8a97c4d297 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 6 Jul 2019 17:25:24 -0700 Subject: [PATCH] Avoid interleaving stderr in a few cases * src/sysdep.c (buferr): New static var. (init_standard_fds) [!DOS_NT]: Initialize it. (errstream, errputc, verrprintf, errwrite): New functions. (close_output_streams): Close buferr too. * src/xdisp.c: Include sysstdio.h instead of stdio.h. (message_to_stderr, vmessage): Use the new functions to avoid interleaving stderr. --- src/sysdep.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++--- src/sysstdio.h | 3 +++ src/xdisp.c | 35 +++++++++-------------------- 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/src/sysdep.c b/src/sysdep.c index 48eebb594f..ee17e08482 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -232,6 +232,10 @@ force_open (int fd, int flags) } } +/* A stream that is like stderr, except line buffered when possible. + It is NULL during startup or if initializing it failed. */ +static FILE *buferr; + /* Make sure stdin, stdout, and stderr are open to something, so that their file descriptors are not hijacked by later system calls. */ void @@ -244,6 +248,15 @@ init_standard_fds (void) force_open (STDIN_FILENO, O_WRONLY); force_open (STDOUT_FILENO, O_RDONLY); force_open (STDERR_FILENO, O_RDONLY); + +#ifndef DOS_NT /* _IOLBF does not work on MS-Windows. */ + buferr = fdopen (STDERR_FILENO, "w"); + if (buferr && setvbuf (buferr, NULL, _IOLBF, 0) != 0) + { + fclose (buferr); + buferr = NULL; + } +#endif } /* Return the current working directory. The result should be freed @@ -2769,6 +2782,46 @@ safe_strsignal (int code) return signame; } +/* Output to stderr. */ + +/* Return the error output stream. */ +static FILE * +errstream (void) +{ + FILE *err = buferr; + if (!err) + return stderr; + fflush_unlocked (stderr); + return err; +} + +/* These functions are like fputc, vfprintf, and fwrite, + except that they output to stderr and buffer better on + platforms that support line buffering. This avoids interleaving + output when Emacs and other processes write to stderr + simultaneously, so long as the lines are short enough. When a + single diagnostic is emitted via a sequence of calls of one or more + of these functions, the caller should arrange for the last called + function to output a newline at the end. */ + +void +errputc (int c) +{ + fputc_unlocked (c, errstream ()); +} + +void +verrprintf (char const *fmt, va_list ap) +{ + vfprintf (errstream (), fmt, ap); +} + +void +errwrite (void const *buf, ptrdiff_t nbuf) +{ + fwrite_unlocked (buf, 1, nbuf, errstream ()); +} + /* Close standard output and standard error, reporting any write errors as best we can. This is intended for use with atexit. */ void @@ -2782,9 +2835,10 @@ close_output_streams (void) /* Do not close stderr if addresses are being sanitized, as the sanitizer might report to stderr after this function is invoked. */ - if (ADDRESS_SANITIZER - ? fflush_unlocked (stderr) != 0 || ferror (stderr) - : close_stream (stderr) != 0) + bool err = buferr && (fflush_unlocked (buferr) != 0 || ferror (buferr)); + if (err | (ADDRESS_SANITIZER + ? fflush_unlocked (stderr) != 0 || ferror (stderr) + : close_stream (stderr) != 0)) _exit (EXIT_FAILURE); } diff --git a/src/sysstdio.h b/src/sysstdio.h index a2364c4e1f..4dc9287e43 100644 --- a/src/sysstdio.h +++ b/src/sysstdio.h @@ -24,6 +24,9 @@ #define EMACS_SYSSTDIO_H #include extern FILE *emacs_fopen (char const *, char const *); +extern void errputc (int); +extern void verrprintf (char const *, va_list) ATTRIBUTE_FORMAT_PRINTF (1, 0); +extern void errwrite (void const *, ptrdiff_t); extern void close_output_streams (void); #if O_BINARY diff --git a/src/xdisp.c b/src/xdisp.c index 5fb690e746..85cac3cd29 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -302,7 +302,6 @@ Copyright (C) 1985-1988, 1993-1995, 1997-2019 Free Software Foundation, buffer_posn_from_coords in dispnew.c for how this is handled. */ #include -#include #include #include #include @@ -311,6 +310,7 @@ Copyright (C) 1985-1988, 1993-1995, 1997-2019 Free Software Foundation, #include "atimer.h" #include "composite.h" #include "keyboard.h" +#include "sysstdio.h" #include "systime.h" #include "frame.h" #include "window.h" @@ -10713,7 +10713,7 @@ message_to_stderr (Lisp_Object m) if (noninteractive_need_newline) { noninteractive_need_newline = false; - fputc ('\n', stderr); + errputc ('\n'); } if (STRINGP (m)) { @@ -10727,23 +10727,10 @@ message_to_stderr (Lisp_Object m) else s = m; - /* We want to write this out with a single fwrite call so that - output doesn't interleave with other processes writing to - stderr at the same time. */ - { - int length = min (INT_MAX, SBYTES (s) + 1); - char *string = xmalloc (length); - - memcpy (string, SSDATA (s), length - 1); - string[length - 1] = '\n'; - fwrite (string, 1, length, stderr); - xfree (string); - } + errwrite (SDATA (s), SBYTES (s)); } - else if (!cursor_in_echo_area) - fputc ('\n', stderr); - - fflush (stderr); + if (STRINGP (m) || !cursor_in_echo_area) + errputc ('\n'); } /* The non-logging version of message3. @@ -10888,12 +10875,12 @@ vmessage (const char *m, va_list ap) if (m) { if (noninteractive_need_newline) - fputc ('\n', stderr); - noninteractive_need_newline = false; - vfprintf (stderr, m, ap); - if (!cursor_in_echo_area) - fputc ('\n', stderr); - fflush (stderr); + { + noninteractive_need_newline = false; + errputc ('\n'); + } + verrprintf (m, ap); + errputc ('\n'); } } else if (INTERACTIVE) -- 2.21.0 --------------C34261F12FC511A7328B2C98--