RFR: JDK-8059586: hs_err report should treat redirected core pattern.

[David Holmes]

On 1/12/2014 10:57 PM, Thomas Stüfe wrote:
> Hi Yasumasa,
>
> On Mon, Dec 1, 2014 at 10:45 AM, Yasumasa Suenaga <yasuenag at gmail.com
> <mailto:yasuenag at gmail.com>> wrote:
>
>     Hi Thomas, David,
>
>     Sorry, I didn't think about async signal safety.
>
>         That would work, VmError::report_and_die() is singlethreaded. At
>         least the part which dumps out the core file name.
>
>
>     I think that signal handler (in this case) may run concurrency with
>     other thread.
>     If another thread calls malloc(3) in JNI, C Heap corruption may occur.
>
>
> No, malloc(3) should be thread safe on our platforms. But this was not
> the point. If I understood David right, he suggested using a static
> buffer inside get_core_path() for assembling the core path, which would
> make get_core_path() thread-unsafe (multiple threads calling it would
> get garbled results). But as get_core_path() is only called from within
> VmError::report_and_die() and that section is only ever executed by one
> thread, Davids suggestion would still work.

Yes that is what I was suggesting.

>     I want to rewrite a patch as below:
>
>       - Use async signal safety functions.
>         fopen -> open, fgets -> read, etc.

This is commendable if it is practical, but error reporting already does 
many, many things that are not async-signal safe, so there is no need to 
go to extreme measures here.

>       - Use O_BUFLEN for buffer size.
>         O_BUFLEN is defined to 2000 in ostream.hpp .
>         This macro is used in various points. VMError::coredump_message is
>         also defined with this value.
>
>
> I think PATH_MAX is fine. I think O_BUFLEN was originally used as a max.
> length of temporary buffers to assemble an output line. And then it
> spread a bit. But your intend is to hold a path and using PATH_MAX
> clearly documents this.
> And, to really nitpick, right now you do not handle ERANGE with
> get_current_path() (if the provided buffer is too small), which is
> probably fine because it is improbable that a path is larger than
> PATH_MAX. But if you change the size of the buffer to something which
> may be smaller than PATH_MAX (O_BUFLEN), get_current_directory() may fail.
>
> I like your patch, I think it could be a nice time safer when
> core_pattern is something unusual. But I also see Staffans point of
> too-much-complexity. So I will keep out of this discussion until the
> real Reviewers decided what to do :)

I have a hard time evaluating the merits of the patch as I don't work in 
an environment where this extra info is needed. But I take it on good 
faith that it is useful for the context Yasumasa describes.

David

> Kind Regards, Thomas
>