[8u] RFR 8233197: Invert JvmtiExport::post_vm_initialized() and Jfr:on_vm_start() start-up order for correct option parsing

Jaroslav Bachorík jaroslav.bachorik at datadoghq.com
Wed Jun 3 07:52:07 UTC 2020


Hi!

On Fri, May 29, 2020 at 10:58 AM Andrew Haley <aph at redhat.com> wrote:
>
> On 28/05/2020 17:25, Jaroslav Bachorík wrote:
> > On Thu, May 28, 2020 at 3:10 PM Andrew Hughes <gnu.andrew at redhat.com> wrote:
> >>
> >>
> >>
> >> On 28/05/2020 13:34, Andrew Haley wrote:
> >>> On 28/05/2020 10:08, Jaroslav Bachorík wrote:
> >>>> I guess so. But such a test is not a part of the original changeset.
> >>>> Is it necessary to be extending the scope of the original patch
> >>>> (https://hg.openjdk.java.net/jdk/jdk/rev/127ca611f19b)?
> >>>
> >>> Yes please. 8u is old code, and your first task is not to break it. If
> >>> this involves being more defensive than upstream, that's fine. Class
> >>> initialization order at startup is notoriously sensitive and responsible
> >>> for breaking things in subtle ways.
> >>
> >> There's also no reason such a change could not then be forwardported to
> >> later versions. If a bug is filed for this, the 8u fix can be committed
> >> under both 8233197 & the new ID, and then the test change alone posted
> >> for review in later versions.
> >
> > Well, there might be a reason - there is no 'native test' support in
> > JDK 8 AFAIK. There are not even any JVMTI native agent jtreg tests -
> > so no harness for building a native agent (on all supported
> > platforms). And to be honest, I don't feel like writing all that
> > support from scratch. If I am missing something and there indeed is a
> > support for testing JVMTI agents, please let me know and I will add a
> > test.
>
> No problem. Like I said, there's no reason that 8u shouldn't have this
> even when upstream doesn't, and there's a very good reason we need it.

Thanks to Mario Torre for pointing me the right direction! I was able
to add a rudimentary test which exhibits JVM crash without this patch
applied and passes with this patch (with an exception and error
message as specified - no JVM crash).

Webrev: http://cr.openjdk.java.net/~jbachorik/8233197/hotspot/webrev.03/
(also including the requested indentation change)

BTW, since this patch missed the cutoff date for 8u262 mostly due to
review would it be ok to consider it still as a part of the critical
fix process?

Cheers,

-JB-

>
> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>


More information about the jdk8u-dev mailing list