RFR: JDK-8059586: hs_err report should treat redirected core pattern.
david.holmes at oracle.com
Tue Dec 2 09:40:45 UTC 2014
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.
> Kind Regards, Thomas
More information about the hotspot-runtime-dev