On Thu, 14 Apr 2022 13:06:58 GMT, Johannes Bechberger <d...@openjdk.java.net> 
wrote:

>> Move the AsyncGetCallTrace method implementation into a separate method and 
>> wrap its call in non-assert compilation mode in `os::ThreadCrashProtection` 
>> like it is done in 
>> [JFR](https://github.com/openjdk/jdk/blob/965ea8d9cd29aee41ba2b1b0b0c67bb67eca22dd/src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp#L165).
>> This prevents AsyncGetCallTrace from crashing on segmentation faults (but 
>> not on `guarantee`s).
>> 
>> If a crash is observed, then the `num_frames` field of the trace is set to 
>> `ticks_unknown_state` (-7) to signal a state that cannot be properly 
>> handled. `ticks_unknown_state` is currently also used for signaling unknown 
>> thread states but this should not be a problem, as the semantic is the same. 
>> If `num_frames` already has an error code then this error code is not 
>> changed. This helps to distinguish between errors in walking threads in Java 
>> and non-Java mode, as `num_frames` is set there before the walking to the 
>> appropriate error code.
>> 
>> _Thanks for @tstuefe for suggesting this._
>
> Johannes Bechberger has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Handle nested ThreadCrashProtection on POSIX

I second the attitude of not snowballing on this PR. Either this is a totally 
hopeless approach and should be abandoned or it is ok short term and then we 
should proceed.

If nothing else, an isolated and focused fix will have higher chances of 
backporting than if we just pile on all the goodies we want from the JFR/ASGCT 
unification (which I think will never get backported). And from the practical 
ponit of view we really need the chance of crashing JVM reduced in all 
important versions back to JDK 8 - I know users should upgrade but there is the 
the nice, theoretical world and then there is the messy world we live in where 
JDK 8 will continue on living for at least a few years and we (Datadog) need to 
support it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8225

Reply via email to