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

David Holmes david.holmes at oracle.com
Wed Oct 24 22:28:41 UTC 2018


On 24/10/2018 10:46 PM, Tony Printezis wrote:
> 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?

Same as most of JDK now. Direct push by a Committer after appropriate 
local testing and preferably running through jdk/submit repo for build 
and test purposes.

Thanks,
David

> 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


More information about the hotspot-runtime-dev mailing list