Re: [Patch, libfortran] Thread safety and simplification of error printing

[Janne Blomqvist <blomqvist.janne@gmail.com>]

PING

On Sun, May 8, 2011 at 19:53, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> On Sun, May 8, 2011 at 14:40, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> On Sat, May 7, 2011 at 19:35, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>>> Hi,
>>>
>>> the error printing functionality (in io/unix.c) st_printf and
>>> st_vprintf are not thread-safe as they use a static buffer. However,
>>> since these routines are used when something has gone wrong, we
>>> shouldn't use malloc() to allocate thread-specific buffers or anything
>>> fancy like that. The way the buffer is used is that it is filled with
>>> vs(n)printf() and then the buffer is written using write(); the
>>> simplest way to fix this is to just print directly using vfprintf(),
>>> which is what this patch does. Also, the patch uses
>>> flockfile/funlockfile in a few places to increase the likelihood of
>>> multiple related messages to end up together on the output.
>>>
>>> As stderr is unbuffered and stdout is buffered, this patch also gets
>>> rid of the functionality to choose where to send the error output, as
>>> we don't want to deal with buffering in the error handling path. IMHO
>>> this is no loss, as all targets/shells/batch schedulers/ etc. where
>>> this might be relevant, offer functionality to redirect stderr to
>>> wherever stdout goes. So there is no need for a gfortran-specific
>>> mechanism for this.
>>
>> While this patch makes error printing thread-safe, it's no longer
>> async-signal-safe as the stderr lock might lead to a deadlock. So I'm
>> retracting this patch and thinking some more about this problem.
>
> So here is an updated patch. Compared to the status quo the main
> differences are that it uses a stack allocated buffer to do the
> formatting instead of a static one. This prevents conflicts from
> multiple threads and in contrast to the approach in the previous patch
> should also be async-signal-safe (or at least, less likely to go
> belly-up when called from within a signal handler than using
> vfprintf() directly). It also introduces a new function to write a
> string to standard error without any formatting (estr_write), which is
> simpler and less likely to cause problems than the general purpose
> vs(n)printf() used before. This can be used in lieu of st_printf in
> the majority of cases where no formatting is done.
>
> Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
>
> frontend ChangeLog:
>
> 2011-05-08  Janne Blomqvist  <jb@gcc.gnu.org>
>
>        * gfortran.texi: Remove GFORTRAN_USE_STDERR documentation.
>
>
> library ChangeLog:
>
> 2011-05-08  Janne Blomqvist  <jb@gcc.gnu.org>
>
>        * io/unix.c (st_vprintf,st_printf): Move to runtime/error.c.
>        * libgfortran.h (struct options_t): Remove use_stderr field.
>        (st_vprintf,st_printf): Move prototypes.
>        (estr_write): New prototype.
>        * runtime/error.c (sys_exit): Use estr_write instead of st_printf.
>        (estr_write): New function.
>        (st_vprintf): Move from io/unix.c, use stack allocated buffer,
>        always output to stderr.
>        (st_printf): Move from io/unix.c.
>        (show_locus): Use a local variable instead of static.
>        (os_error): Use estr_write instead of st_printf.
>        (runtime_error): Likewise.
>        (runtime_error_at): Likewise.
>        (runtime_warning_at): Likewise.
>        (internal_error): Likewise.
>        (generate_error): Likewise.
>        (generate_warning): Likewise.
>        (notify_std): Likewise.
>        * runtime/pause.c (do_pause): Likewise.
>        (pause_string): Likewise.
>        * runtime/stop.c (stop_string): Likewise.
>        (error_stop_string): Likewise.
>        * config/fpu_aix.h (set_fpu): Likewise.
>        * config/fpu_generic.h (set_fpu): Likewise.
>        * config/fpu_glibc.h (set_fpu): Likewise.
>        * config/fpu-sysv.h (set_fpu): Likewise.
>        * runtime/backtrace.c (dump_glibc_backtrace): Likewise.
>        (show_backtrace): Likewise.
>        * runtime/environ.c (print_spaces): Likewise.
>        (show_string): Likewise.
>        (show_variables): Likewise.
>        (variable_table[]): Remove GFORTRAN_USE_STDERR entry.
>
>
> --
> Janne Blomqvist
>



-- 
Janne Blomqvist