On Mon, 31 Jan 2022 22:12:30 GMT, David Holmes <david.hol...@oracle.com> wrote:
> Hi Christian, > > Sorry for the delay in coming back to this, I wanted to see what other > feedback arose. No problem, thanks for your feedback David! > > > That's a valid concern. I've also asked myself this question when I had > > > initially started using some assertions. We should not crash again during > > > error reporting. I've therefore tried to be as conservative as possible > > > and added bailouts instead, also in loops when reading data. But of > > > course, this is just a best effort and by no means a guarantee to be safe > > > (especially in terms of crashes). What could be alternatives to make this > > > better? > > > > > > If the parsing code turns out to be very problematic in a signal handling > > context, then we could disable it in that context. So we really want to try > > and do a lot of testing by throwing random signals at the VM and see what > > breaks. > > Source information in hs-err file stacks can be tremendously useful. Lets try > the retry-callstack-dumping without features idea in case of a secondary > crash, outlined above, first. Should we still handle that in a separate RFE later or should this go along with this patch/prerequisite? What do you think? > > > > Secondly, on the same issue the use of unified logging within this code > > > > seems even more likely to be problematic - I'm not aware of us > > > > currently using UL during error reporting. It may work in basic > > > > usecases but if it triggers logfile rotation or other more complex > > > > actions what then? > > > > > > > > > I haven't thought about this before. To be honest, I think UL printing of > > > the `dwarf` tag is only useful during development when adding something > > > new to the parser or when debugging. I don't see much value of these > > > messages otherwise - even less for a Java user. As a first step, I could > > > change the logs from `log_X()` to `log_develop_X()` but that just shifts > > > the problem to non-product builds. Another option (or additional thing) > > > could be to guard the log messages with a new develop flag that's > > > disabled by default. By setting it for development, we accept that it > > > might be unsafe which should be fine. > > > > > > I think changing the logging to develop only is a reasonable step. I don't > > see logging of crash handling / error reporting as generally useful for the > > end user. > > I think the right way to go longterm would be to give us a minimalistic safe > logging API for these cases (signal handling, pre-initialization) or make UL > safe to use always. That would be ideal if UL usage could be made safe in the future for these cases. But as for now, I will start by changing the logging to develop to limit UL usage to debug builds only which does not affect end users anymore. ------------- PR: https://git.openjdk.java.net/jdk/pull/7126
