RFR(XS): 8212883: Setting a double manageable flag with jcmd/jinfo crashes the JVM

[Tony Printezis]

Thanks Gerard,

I agree re: test coverage. I’ll follow up on that in my reply to David.

Yes, if in doubt, follow the file’s pattern. :-)

I haven’t pushed any HotSpot changes recently (last few non-HotSpot ones I
pushed directly). What’s the process for HotSpot changes these days?

Tony


—————
Tony Printezis | @TonyPrintezis | tprintezis at twitter.com


On October 23, 2018 at 6:43:31 PM, Gerard Ziemski (gerard.ziemski at oracle.com)
wrote:

hi Tony,

Looks good. Thank you for adding the new type here.

It would be nice to have a test coverage here, but I think we need to wait
with that till we get an actual flag of that type.

A small quibble: I don’t like the inconsistencies in that file's comments,
but that’s a pre-existing condition, so you did the right thing by
following the pattern.

I can sponsor this fix for you if you need it.


cheers


> On Oct 23, 2018, at 5:13 PM, Tony Printezis <tprintezis at twitter.com>
wrote:
>
> Webrev here:
>
> http://cr.openjdk.java.net/~tonyp/8212883/webrev.0/
>
> Currently, HotSpot doesn’t actually have any double manageable flags,
which
> is why I think no-one has hit this before. I recently added a couple to
our
> own builds and I noticed that setting them is not handled correctly in
the VM.
> The fix is pretty trivial (mostly cut-and-paste from what the code does
for
> the other types).
>
> I tested it by introducing a dummy double manageable flag and I can set
it
> with jinfo/jcmd and jconsole (these cover all the various paths in the
> changes). Is it worth expanding the
> serviceability/attach/AttachSetGetFlag.java test to also get/set a double
> flag (I’d need to introduce a dummy double manageable flag to do that
> though)?
>
> Regards,
>
> Tony
>
>
> —————
> Tony Printezis | @TonyPrintezis | tprintezis at twitter.com