RFR: 8027429: Add diagnostic command VM.info to get hs_err print-out
Thomas Stüfe
thomas.stuefe at gmail.com
Fri Nov 13 10:25:23 UTC 2015
Hi Coleen,
I think coupling the safe-for-info operations to STEP numbers > 200 has
some drawbacks : whenever you decide to move a STEP from one category to
another (like, decide a step is too unsafe to use in info command after
all), you need to shuffle the code around, which causes unnecessary diffs,
and also you change the order of the hs-err file, which may confuse some
tests. We (SAP) have a lot of tests where we parse the hs-err file for
correctness.
Maybe another alternative would be to add a parameter to the STEP macro
which defines when the step is to be executed:
#define STEP(n, when, s)
STEP(60, ON_ERROR, "(printing exception/signal name)")
STEP(590, ON_ERROR|ON_INFO, "(printing dynamic libraries)" )
----
http://cr.openjdk.java.net/~coleenp/8027429.01/src/share/vm/utilities/vmError.cpp.udiff.html
+ # define STEP(n, s) } if (_current_step < n) { if (!_vm_info_cmd) {
_current_step = n; _current_step_info = s; }
I would prefer to if the _current_step number were always increased, even
if the STEP itself is not executed, not just in the error case. For one, it
makes the code easier to read, but there is also the typical error where
you introduce a new step by copy-pasting another step, forget to increase
the step number and the step does not get executed. The error would now be
hidden for the info case. So, you may introduce an ON_INFO step which works
in info case but gets skipped in error case.
Kind Regards, Thomas
On Thu, Nov 5, 2015 at 10:01 PM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:
>
> Hi,
>
> I've done some refactoring and cleanup on this which the refactoring
> enabled. Again, it's important that the VM.info output be mostly the same
> order as the hs_err_pid* report file so that sustaining can easily match
> and find output that they are looking for. It's also important not to
> copy this code because there may be new information added that we also want
> in the VM.info output. If any future information that we want to print
> seems unsafe, the vm_info_cmd can be used to exclude it or it can be
> printed only in the error reporting function. Rejecting the entire change
> because of the theoretical unsafe addition of code isn't the right thing to
> do, although yeah, I've argued the other way in the past. This is
> something that is helpful to sustaining and our licensees.
>
> In this patch, the functions that VM.info calls are safe while the JVM is
> running, but the stepping is required because at crash time, they might not
> be. Although I've never seen the error reporting crash in these places.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8027429.01/
> bug link https://bugs.openjdk.java.net/browse/JDK-8027429
>
> This refactoring has been run through RBT.
>
> Thanks,
> Coleen
>
>
> On 11/4/15 2:25 AM, David Holmes wrote:
>
>> On 4/11/2015 1:36 AM, Mattis Castegren wrote:
>>
>>> Also note that some of the crash information is not interesting for
>>> VM.info. For example, we don't want to print a thread dump or the
>>> registers/stack data for any specific thread. If people want thread dumps,
>>> they can use the separate Dcmd. We only want the general system state.
>>>
>>
>> I'm a little surprised that thread info is not of interest, but as you
>> say that is available by other means - which was part of my earlier point
>> about there already being safe means to get some of the info.
>>
>> If this is really only about the small subset of safe "system
>> information" that VMError reports, then I see even less motivation to
>> piggy-back VM-info onto VMerror and entangle the two. If each block of
>> information can be considered a single "step" in the error case then we can
>> simply factor out the actual printing parts and share them. If the error
>> case wants fine-grained steps then we can't do that.
>>
>> David
>>
>> Kind Regards
>>> /Mattis
>>>
>>> -----Original Message-----
>>> From: Coleen Phillimore
>>> Sent: den 3 november 2015 16:25
>>> To: Bertrand Delsart; Thomas Stüfe; Mattis Castegren
>>> Cc: David Holmes; hotspot-runtime-dev at openjdk.java.net
>>> Subject: Re: RFR: 8027429: Add diagnostic command VM.info to get hs_err
>>> print-out
>>>
>>>
>>> Hi,
>>>
>>> I've done a bit of refactoring with this change and have tested it with
>>> and without intentional crashes to the VM.info reporting. If it crashes
>>> within VM.info, there's a stack trace that clearly shows where it comes
>>> from in the hs_err file. And the hs_err_pid file is correctly produced.
>>> If we can make the jcmd come with a warning that it *could* potentially
>>> crash the vm, I think this feature is a good addition for sustaining.
>>>
>>> Without crashing, VM.info gives a lot of information that we normally
>>> give with the hs_err file. The reason for the careful STEP'ping in hs_err
>>> production is that the JVM has already crashed so we can't count on
>>> anything. The VM.info command is designed to be run on a moderately stable
>>> JVM so is less likely to crash on the things that we're printing out.
>>>
>>> I think we should fix the Threads walking code that Fred pointed out to
>>> take the Threads_lock, but I don't think we should *copy* code for VM.info
>>> or copy print_on_error functions.
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 11/3/15 10:18 AM, Bertrand Delsart wrote:
>>>
>>>> On 03/11/2015 15:06, Bertrand Delsart wrote:
>>>>
>>>>> Thanks Thomas,
>>>>>
>>>>> I fully agree it can work. The problem is that it may also fail if we
>>>>> are not careful.
>>>>>
>>>>> Fred noticed for instance that "Threads::print_on_error" is not
>>>>> protected by _vm_info_cmd (step 290). This is an obvious source of
>>>>> potential crashes if you take into account the fact that the
>>>>> print_on_error methods were designed not to use locks.
>>>>>
>>>>
>>>> Fred just pointed out that this may be OK since _thread is NULL.
>>>>
>>>> I'm not sure of why we do not print the threads when there is notion
>>>> of current thread (and I would wonder whether we should print them for
>>>> VMInfo) but luckily that may have been sufficient to prevent the crash.
>>>>
>>>> However, I still do not like it from a design point of view for the
>>>> other reasons stated in my e-mail.
>>>>
>>>> Bertrand.
>>>>
>>>>
>>>>> It seems that the focus has been on removing what was clearly
>>>>> useless, not what might be dangerous due to how the method (and what
>>>>> it calls) has been designed.
>>>>>
>>>>> In addition, adding _vm_info_cmd is IMHO not sufficient unless you
>>>>> pass that argument to all layers. This is the biggest design issue.
>>>>> For instance all calls to print_on_error could have this additional
>>>>> argument so that the different components can adapt their behavior
>>>>> (and are aware there are now two use cases). I would start by
>>>>> Threads::print_on_error, grabbing the threads lock in the info case,
>>>>> but the same if true for all methods called by VMError::report. I
>>>>> would go as far as adding a comment in all print_on_error methods
>>>>> stating why the method is safe for both use cases (e.g. why no locks
>>>>> are needed) or adding "assert(!vm_info_cmd)" for methods
>>>>> VMError:report is supposed to skip.
>>>>> The current design, where the various components may not be aware
>>>>> that they are called for two different purposes, is not robust enough.
>>>>>
>>>>> Note also that being 'robust enough for the "crash" case' is not a
>>>>> strong enough guarantee for a general purpose info DCmd...
>>>>> particularly if, as you point out, an error that occurs in one of the
>>>>> info STEP crashes the VM instead of just bringing us to the next
>>>>> error reporting STEP. Now, as stated before, I could agree if we use
>>>>> this DCmd only when a crash is acceptable.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Bertrand
>>>>>
>>>>> On 02/11/2015 14:32, Thomas Stüfe wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> we have implemented in the SAP JVM the same mechanism. Our
>>>>>> implementation is in use since a number of years and is quite
>>>>>> useful. So I would agree with Coleen.
>>>>>>
>>>>>> However, there are some things to keep in mind when doing this:
>>>>>>
>>>>>> Error handling is different between the "info-only" and "crash"
>>>>>> case. In
>>>>>> "info-only" case, VMError::report() is called without enclosing
>>>>>> VMError::report_and_die() and we run with the normal signal handler
>>>>>> intact, not with the secondary signal handler. So, normal signal
>>>>>> handling works, which is good. But it also means that any error
>>>>>> inside one of the error reporting STEPs will bring down the VM,
>>>>>> instead of just bringing us to the next error reporting STEP ("Error
>>>>>> occurred during error reporting").
>>>>>>
>>>>>> The problem is that a number of error reporting steps are a bit
>>>>>> risky and written with the assurance that the worst which can happen
>>>>>> is that the STEP will be interrupted. These risky steps should not
>>>>>> be executed in the "only-info" case.
>>>>>>
>>>>>> But the proposed change already does this, it introduces
>>>>>> "_vm_info_cmd"
>>>>>> and conditionally excludes dangerous or pointless error reporting
>>>>>> STEPs (e.g. callstack is pointless).
>>>>>>
>>>>>> As far as I can see, the other concerns concern only the "crash"
>>>>>> case and simply do not apply to the "info-only" case. If an error
>>>>>> reporting step is robust enough for the "crash" case (low stack
>>>>>> usage, not allocating memory to avoid deadlocks etc), it should be
>>>>>> safe enough for the "info" case.
>>>>>>
>>>>>> Kind Regards, Thomas
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Nov 2, 2015 at 12:31 PM, Mattis Castegren
>>>>>> <mattis.castegren at oracle.com <mailto:mattis.castegren at oracle.com>>
>>>>>> wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Of course, when I wrote "Review and push this fix" I meant doing
>>>>>> a
>>>>>> thorough technical review. We do not want VM.info to be unsafe,
>>>>>> and
>>>>>> I will let David reply to the technical questions that has been
>>>>>> raised. I know he did consider some of these issues, but last
>>>>>> week
>>>>>> he was on Java one, and this week he is out. I will let him
>>>>>> comment
>>>>>> once he gets back.
>>>>>>
>>>>>> Kind Regards
>>>>>> /Mattis
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Bertrand Delsart
>>>>>> Sent: den 2 november 2015 10:13
>>>>>> To: David Holmes; Mattis Castegren; Coleen Phillimore;
>>>>>> hotspot-runtime-dev at openjdk.java.net
>>>>>> <mailto:hotspot-runtime-dev at openjdk.java.net>
>>>>>> Subject: Re: RFR: 8027429: Add diagnostic command VM.info to get
>>>>>> hs_err print-out
>>>>>>
>>>>>> On 02/11/2015 02:45, David Holmes wrote:
>>>>>> > On 30/10/2015 8:38 PM, Mattis Castegren wrote:
>>>>>> >> Hi
>>>>>> >>
>>>>>> >> I agree that there may be things we could add to VM.info
>>>>>> that we
>>>>>> >> couldn't add in an hs_err file due to the fact that we are
>>>>>> crashing.
>>>>>> >> However, I really do believe that we should use the same
>>>>>> code.
>>>>>> The
>>>>>> >> goal of the hs_err file is exactly the same as the goal of
>>>>>> VM.info,
>>>>>> >> to print as much information about the system as possible so
>>>>>> that
>>>>>> >> when we look at an hs_err file or the output from VM.info,
>>>>>> we get as
>>>>>> >> much information about the system as possible. That way we
>>>>>> can
>>>>>> reduce
>>>>>> >> the need to go back and forth with the customer to ask for
>>>>>> more
>>>>>> information.
>>>>>> >>
>>>>>> >> If we enumerate what we need in VM.info, the list would look
>>>>>> very
>>>>>> >> much like what we already do in VMError, and therefore I
>>>>>> think we
>>>>>> >> should use the same code. If we find something that we would
>>>>>> want,
>>>>>> >> that is not currently printed in hs_err files, I think we
>>>>>> have
>>>>>> two options:
>>>>>> >>
>>>>>> >> 1) Add it to hs_err files as well. If it is interesting when
>>>>>> >> gathering customer information before a crash, it probably is
>>>>>> >> interesting after a crash
>>>>>> >> 2) If it for some reason cannot be captured when crashing,
>>>>>> add it to
>>>>>> >> the code with the condition that we don't print it during
>>>>>> crash
>>>>>> time.
>>>>>> >>
>>>>>> >> I would therefore suggest the following plan of action
>>>>>> >>
>>>>>> >> 1) Review and push this fix to see if there are any
>>>>>> technical issues
>>>>>> >> with regards to thread safety, etc. I know David has had
>>>>>> this in
>>>>>> mind
>>>>>> >> when doing the change. After that, we have a command that
>>>>>> support can
>>>>>> >> use.
>>>>>> >> 2) Think about if there is any information missing, both
>>>>>> from hs_err
>>>>>> >> files and from VM.info. Here, we may want to ask support
>>>>>> about if
>>>>>> >> there is any additional information they need to ask for
>>>>>> when they
>>>>>> >> get this data. If possible, this information should be added
>>>>>> to both
>>>>>> >> hs_err files and VM.info, we don't want support to have to
>>>>>> ask for
>>>>>> >> VM.info if we already have an hs_err file. Of course, if
>>>>>> there is
>>>>>> >> anything Dev would want to see when analyzing crashes in
>>>>>> testing, we
>>>>>> >> should add that too.
>>>>>> >
>>>>>> > I have no issue with VMError and VM.Info using the same code
>>>>>> (where
>>>>>> > appropriate), but I think it wrong to try and implement
>>>>>> VM.Info
>>>>>> > directly using the actual VMError mechanism. It is crude and
>>>>>> makes it
>>>>>> > difficult to make independent changes to either facility.
>>>>>> >
>>>>>> > Sorry.
>>>>>> >
>>>>>> > David
>>>>>>
>>>>>> Furthermore, we cannot just "Review and push this fix to see if
>>>>>> there are any technical issues". Two reviewers have already
>>>>>> stated
>>>>>> that there are issues and these issues must be addressed before
>>>>>> pushing.
>>>>>>
>>>>>> The answer cannot just be that we need this feature, it must
>>>>>> address
>>>>>> the technical issues that were reported.
>>>>>>
>>>>>> One possible answer is that we all agree that VMInfo would be
>>>>>> unsafe.
>>>>>> The method would have to be renamed UnsafeVMInfo and it must be
>>>>>> clear that its 'impact' would be very risky, potentially crashing
>>>>>> the JVM.
>>>>>>
>>>>>> If this is sufficient for your needs then we could agree with
>>>>>> it. My
>>>>>> only additional requirement would be to clearly mark that the
>>>>>> method
>>>>>> has been called so that we do not waste time on non-issues if
>>>>>> calling VMInfo leads to a crash (or, even worse but less likely,
>>>>>> to
>>>>>> a silent data corruption that may lead to problems later). The
>>>>>> bare
>>>>>> minimum would be to double check that a crash during an
>>>>>> execution of
>>>>>> VM.info is clearly identifiable (this is worth double checking
>>>>>> because of the use of static buffers for the error reporting, see
>>>>>> below).
>>>>>>
>>>>>> If an 'unsafe' VMInfo DCmd is not sufficient, you should wonder
>>>>>> whether a safe version of it is still a "small enhancement". A
>>>>>> JEP
>>>>>> may be required.
>>>>>>
>>>>>> VMError::report and the methods it calls (print_on_error) have
>>>>>> been
>>>>>> written with assumptions clearly stated in the comments above
>>>>>> VMError::report (see below the two most important ones). HotSpot
>>>>>> error reporting strategy differed from JRockit. This explains why
>>>>>> the JRockit approach cannot be applied as is.
>>>>>>
>>>>>> I agree with David that you can try leveraging some of the logic
>>>>>> from VMError::report. However, to be sure there is no
>>>>>> misunderstanding, let's be clearer about David's "(where
>>>>>> appropriate)": we would for instance have to review all the
>>>>>> called
>>>>>> methods, check which ones purposefully ignore locking
>>>>>> (potentially
>>>>>> crashing) and provide alternate code for the unsafe ones (which
>>>>>> may
>>>>>> not be trivial in some cases). As a simple example, a method
>>>>>> called
>>>>>> "print_on_error" should not be called when we are not in an
>>>>>> erroneous condition. However, in some components of the JVM, the
>>>>>> low
>>>>>> level implementations of 'print_on_error' and 'print_info_safely'
>>>>>> could share the same code.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Bertrand.
>>>>>>
>>>>>> Here are the important comments about VMError::report
>>>>>> assumptions:
>>>>>>
>>>>>> // In general, a hang in error handler is much worse than a
>>>>>> crash or
>>>>>> internal
>>>>>> // error, as it's harder to recover from a hang. Deadlock can
>>>>>> happen
>>>>>> if we
>>>>>> // try to grab a lock that is already owned by current thread,
>>>>>> or if the
>>>>>> // owner is blocked forever (e.g. in os::infinite_sleep()). If
>>>>>> possible, the
>>>>>> // error handler and all the functions it called should avoid
>>>>>> grabbing any
>>>>>> // lock. An important thing to notice is that memory allocation
>>>>>> needs a
>>>>>> lock.
>>>>>> //
>>>>>> // We should avoid using large stack allocated buffers. Many
>>>>>> errors
>>>>>> happen
>>>>>> // when stack space is already low. Making things even worse is
>>>>>> that
>>>>>> there
>>>>>> // could be nested report_and_die() calls on stack (see above).
>>>>>> Only one
>>>>>> // thread can report error, so large buffers are statically
>>>>>> allocated in
>>>>>> data
>>>>>> // segment.
>>>>>>
>>>>>>
>>>>>>
>>>>>> >
>>>>>> >
>>>>>> >> Kind Regards
>>>>>> >> /Mattis
>>>>>> >>
>>>>>> >> -----Original Message-----
>>>>>> >> From: David Holmes
>>>>>> >> Sent: den 29 oktober 2015 23:20
>>>>>> >> To: Mattis Castegren; Coleen Phillimore;
>>>>>> >> hotspot-runtime-dev at openjdk.java.net
>>>>>> <mailto:hotspot-runtime-dev at openjdk.java.net>
>>>>>> >> Subject: Re: RFR: 8027429: Add diagnostic command VM.info to
>>>>>> get
>>>>>> >> hs_err print-out
>>>>>> >>
>>>>>> >> On 29/10/2015 11:36 PM, Mattis Castegren wrote:
>>>>>> >>> Some background: We have this command in JRockit. The
>>>>>> information you
>>>>>> >>> gather when you crash to give a good summary of what system
>>>>>> you run
>>>>>> >>> on is pretty much exactly the information you need to get a
>>>>>> good
>>>>>> >>> summary on a system that has not crashed. The JRockit
>>>>>> command is
>>>>>> >>> extremely useful for support, and saves a lot of work going
>>>>>> back and
>>>>>> >>> forth asking about system information.
>>>>>> >>>
>>>>>> >>> Also, if we write something new in the hs_err file, like if
>>>>>> there has
>>>>>> >>> been any out of memory errors, we often would want the same
>>>>>> >>> information in the VM.info output. From my experience in
>>>>>> >>> Sustaining/Support, I can't think of any information you
>>>>>> would want
>>>>>> >>> in VM.info that you wouldn't also want in the hs_err file
>>>>>> and the
>>>>>> >>> other way around, apart from details about the crash
>>>>>> (obviously).
>>>>>> >>>
>>>>>> >>> I don't see a reason to exactly enumerate what information
>>>>>> VM.info
>>>>>> >>> should provide. From a sustaining/support perspective, we
>>>>>> want a
>>>>>> >>> one-stop command to gather as much useful information as
>>>>>> possible,
>>>>>> >>> which is the same idea we have for the hs_err file.
>>>>>> >>
>>>>>> >> The reason to enumerate what is required is to see where that
>>>>>> >> information already exists and can be collected from. The
>>>>>> VMError
>>>>>> >> report has to be very careful about what it does and how it
>>>>>> does it
>>>>>> >> because of the fact we may have crashed and the general
>>>>>> process
>>>>>> state
>>>>>> >> is indeterminate. A Dcmd can simply gather whatever
>>>>>> information is
>>>>>> >> required from the available sources in whatever order or
>>>>>> format
>>>>>> desired.
>>>>>> >>
>>>>>> >> I have no problem with this command, just how it has been
>>>>>> proposed to
>>>>>> >> implement it.
>>>>>> >>
>>>>>> >> David
>>>>>> >>
>>>>>> >>> Kind Regards
>>>>>> >>> /Mattis
>>>>>> >>>
>>>>>> >>> -----Original Message-----
>>>>>> >>> From: David Holmes
>>>>>> >>> Sent: den 29 oktober 2015 13:08
>>>>>> >>> To: Coleen Phillimore; hotspot-runtime-dev at openjdk.java.net
>>>>>> <mailto:hotspot-runtime-dev at openjdk.java.net>
>>>>>> >>> Subject: Re: RFR: 8027429: Add diagnostic command VM.info
>>>>>> to get
>>>>>> >>> hs_err print-out
>>>>>> >>>
>>>>>> >>> On 29/10/2015 10:02 PM, Coleen Phillimore wrote:
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> On 10/29/15 7:58 AM, David Holmes wrote:
>>>>>> >>>>> On 29/10/2015 9:26 PM, Coleen Phillimore wrote:
>>>>>> >>>>>>
>>>>>> >>>>>> So I actually disagree. I don't think there should be an
>>>>>> >>>>>> additional separate mechanism to get the same information
>>>>>> that we
>>>>>> >>>>>> get with hs_err reporting. I've wanted to have a
>>>>>> feature like
>>>>>> >>>>>> this for a long time.
>>>>>> >>>>>>
>>>>>> >>>>>> I pre-reviewed this change and I thought it looked good
>>>>>> in
>>>>>> general.
>>>>>> >>>>>> I didn't see the thread iteration problem that Fred
>>>>>> refers to
>>>>>> >>>>>> below, but I think the individual problems can be fixed.
>>>>>> >>>>>>
>>>>>> >>>>>> The last thing I want is this code to be copied
>>>>>> somewhere else.
>>>>>> >>>>>
>>>>>> >>>>> Factored as needed not copied. VMError is not an "info"
>>>>>> reporting
>>>>>> >>>>> mechanism.
>>>>>> >>>>
>>>>>> >>>> If you look at the things that are reported in each "STEP",
>>>>>> there's a
>>>>>> >>>> small amount of code and the order is important.
>>>>>> >>>
>>>>>> >>> The order is less important in an "info" request I would
>>>>>> think.
>>>>>> >>>
>>>>>> >>>> I'd like the vm info to use the same order and report what
>>>>>> it
>>>>>> can do
>>>>>> >>>> safely. Refactoring 5 lines of code into functions
>>>>>> doesn't
>>>>>> make
>>>>>> >>>> sense.
>>>>>> >>>
>>>>>> >>> I need to consider exactly what it is the "info" needs to
>>>>>> report in
>>>>>> >>> more detail. There are existing facilities (system
>>>>>> properties,
>>>>>> >>> management
>>>>>> >>> APIs) for various bits of runtime information, which
>>>>>> VMError can't
>>>>>> >>> utilize but a Dcmd can.
>>>>>> >>>
>>>>>> >>> David
>>>>>> >>>
>>>>>> >>>> Coleen
>>>>>> >>>>
>>>>>> >>>>>
>>>>>> >>>>> David
>>>>>> >>>>>
>>>>>> >>>>>> Coleen
>>>>>> >>>>>>
>>>>>> >>>>>> On 10/28/15 8:48 PM, David Holmes wrote:
>>>>>> >>>>>>> I agree with Fred, this kind of info reporting should
>>>>>> not be
>>>>>> >>>>>>> piggy-backed onto VMError handling for the reasons
>>>>>> stated
>>>>>> (and the
>>>>>> >>>>>>> error handling logic is complicated enough as it is!).
>>>>>> For
>>>>>> things
>>>>>> >>>>>>> like thread lists there are already safe management
>>>>>> functions that
>>>>>> >>>>>>> can/should be used.
>>>>>> >>>>>>>
>>>>>> >>>>>>> Thanks,
>>>>>> >>>>>>> David H.
>>>>>> >>>>>>>
>>>>>> >>>>>>> On 29/10/2015 3:29 AM, Frederic Parain wrote:
>>>>>> >>>>>>>> Hi David,
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> I haven't review all the code yet, but I'm concerned
>>>>>> with the
>>>>>> >>>>>>>> fact that the diagnostic command is calling
>>>>>> VMError::report().
>>>>>> >>>>>>>> This method has been implemented to be executed in the
>>>>>> particular
>>>>>> >>>>>>>> context of fatal errors, and its usage while the VM is
>>>>>> running
>>>>>> >>>>>>>> normally seems dangerous.
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> For instance, VMError::report() consciously avoids
>>>>>> grabbing locks
>>>>>> >>>>>>>> because of the risk of deadlock during the error
>>>>>> reporting.
>>>>>> >>>>>>>> The consequence is that some data structures are
>>>>>> browsed in an
>>>>>> >>>>>>>> unsafe way. One example: VMError::report() calls
>>>>>> >>>>>>>> Threads::print_on_error() which iterates over the
>>>>>> thread list
>>>>>> >>>>>>>> *without owning the Threads_lock*.
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> The implementation of the diagnostic command seems
>>>>>> also to
>>>>>> >>>>>>>> exclude a lot of reporting from the initial
>>>>>> VMError::report()
>>>>>> >>>>>>>> method. Have you considered implementing a new MT-safe
>>>>>> reporting
>>>>>> >>>>>>>> method rather than trying to modify the special
>>>>>> VMError::report()
>>>>>> >>>>>>>> methods? (Note that some code factorization between
>>>>>> >>>>>>>> VMError::report() and this new method should be
>>>>>> possible).
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> Thanks,
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> Fred
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> On 28/10/2015 17:18, david buck wrote:
>>>>>> >>>>>>>>> Hi!
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> Please review my change for this small enhancement.
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8027429
>>>>>> >>>>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~dbuck/8027429_jdk9_01/
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> Cheers,
>>>>>> >>>>>>>>> -Buck
>>>>>> >>>>>>>>
>>>>>> >>>>>>
>>>>>> >>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Bertrand Delsart, Grenoble Engineering Center
>>>>>> Oracle, 180 av. de l'Europe, ZIRST de Montbonnot
>>>>>> 38330 Montbonnot Saint Martin, FRANCE
>>>>>> bertrand.delsart at oracle.com <mailto:bertrand.delsart at oracle.com>
>>>>>> Phone : +33 4 76 18 81 23
>>>>>>
>>>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>> NOTICE: This email message is for the sole use of the intended
>>>>>> recipient(s) and may contain confidential and privileged
>>>>>> information. Any unauthorized review, use, disclosure or
>>>>>> distribution is prohibited. If you are not the intended
>>>>>> recipient,
>>>>>> please contact the sender by reply email and destroy all copies
>>>>>> of
>>>>>> the original message.
>>>>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>
More information about the hotspot-runtime-dev
mailing list