[8u] [JFR] RFR: 8236160: Missing ThreadCrashProtection for JFR in signal handler

[Andrew Dinn]

Hi Denghui,

On 18/12/2019 06:37, Denghui Dong wrote:
> Hi all,
>   Please help review the following changeset:
> Jira: https://bugs.openjdk.java.net/browse/JDK-8236160
> Webrev: http://cr.openjdk.java.net/~ddong/8236160/webrev.00/hotspot.changeset
> Summary:
> JFR thread sampler uses ThreadCrashProtection to protect thread crash, so it's necessary to call
> ThreadCrashProtection::check_crash_protection in signal handler.
> And a more elegant way is to merge ThreadCrashProtection and WatcherThreadCrashProtection, but this changeset didn't do that.
I'm not clear why you are calling both

  os::WatcherThreadCrashProtection::check_crash_protection(sig, t);

and

  os::ThreadCrashProtection::check_crash_protection(sig, t);

Is it not possible simply to rely on os::ThreadCrashProtection alone as
happens updtream?

I am assuming that you have already back-ported class
os::ThreadCrashProtection to the JFR incubator repo? Did you need to
change it's behaviour in any way in order to do that? Does that have
anything to do with why your are calling both routines?

Also, how have you tested this patch?

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill