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

[Andrew Dinn]

Hi Denghui,

On 04/02/2020 03:59, Denghui Dong wrote:
>   I'm so sorry for the late reply.
>   You are right, backport is a simpler way, so I make a webrev for the
> backport of JDK-8183925 for jdk8u-jfr-incubator.(Not sure should I
> create a new thread for this backport)
>   The webrev has some obvious differences with the original changeset
> because some codes had been introduced in JDK-8223147.
>   Please help review it.
> 
> Webrev: http://cr.openjdk.java.net/~ddong/8183925/
> Jira: https://bugs.openjdk.java.net/browse/JDK-8183925
> Original
> changeset: http://hg.openjdk.java.net/jdk10/jdk10/hotspot/rev/786437c6344b
This is all rather messy in that the patch seems to be starting from the
point where the jfr-incubator repo already contains both the old
(WatcherThreadCrashProtection) and most of the new
(ThreadCrashProtection) code i.e. this 'backport' is essentially just
removing the old code. I'm still not clear how or why the new code was
already present but it looks like the differences between this patch and
the upstream one only exist because someone has already done a rather
clumsy job of importing the new functionality.

Anyway, whatever the reason we are here, it looks to me as if this patch
means that the code will now rely on a correct copy of the upstream
ThreadCrashProtection and that all the redundant code for
WatcherThreadCrashProtection has been removed. So, from that point of
view the patch looks ok.

However, before we rush to commit this I'll note I don't see any mention
of testing. Can you explain what you have done to verify that this code
works as expected?

regards,


Andrew Dinn
-----------