RFR(S): 8198608: Improvements to command-line flags printing
David Holmes
david.holmes at oracle.com
Sat Mar 3 09:25:30 UTC 2018
Hi Lutz,
Unfortunately there is a problem.
477 ResourceMark rm;
We may not have a current thread - which is the case running the
runtime/CommandLine/TestVMOptions test.
# A fatal error has been detected by the Java Runtime Environment:
#
# Internal Error
(/scratch/opt/mach5/mesos/work_dir/slaves/9190d864-6621-4810-ba08-d8d8c75ba674-S889/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/00601d4b-798e-4a6a-8c81-8798400ded1a/runs/173a6c67-53ed-430a-9a65-86f14d2e0f82/workspace/open/src/hotspot/share/runtime/thread.hpp:720),
pid=27401, tid=27405
# assert(current != __null) failed: Thread::current() called on
detached thread
#
...
--------------- T H R E A D ---------------
Current thread is native thread
Stack: [0x00007f07265e7000,0x00007f07266e8000], sp=0x00007f07266e6150,
free space=1020k
Native frames: (J=compiled Java code, A=aot compiled Java code,
j=interpreted, Vv=VM code, C=native code)
V [libjvm.so+0x18335b2] VMError::report_and_die(int, char const*, char
const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char
const*, int, unsigned long)+0x162
V [libjvm.so+0x18344af] VMError::report_and_die(Thread*, char const*,
int, char const*, char const*, __va_list_tag*)+0x2f
V [libjvm.so+0xb2ea2d] report_vm_error(char const*, int, char const*,
char const*, ...)+0xdd
V [libjvm.so+0x14b258a] os::physical_memory()+0x7a
V [libjvm.so+0x149cc6b] os::print_summary_info(outputStream*, char*,
unsigned long)+0x5b
V [libjvm.so+0x1831ea8] VMError::report(outputStream*, bool)+0x2aa8
V [libjvm.so+0x18335b2] VMError::report_and_die(int, char const*, char
const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char
const*, int, unsigned long)+0x162
V [libjvm.so+0x18344af] VMError::report_and_die(Thread*, char const*,
int, char const*, char const*, __va_list_tag*)+0x2f
V [libjvm.so+0xb2ea2d] report_vm_error(char const*, int, char const*,
char const*, ...)+0xdd
V [libjvm.so+0xdf6f06] Flag::print_on(outputStream*, bool, bool)+0x846
V [libjvm.so+0xdf7184] CommandLineFlags::printFlags(outputStream*,
bool, bool)+0x164
V [libjvm.so+0x6a708c]
Arguments::match_special_option_and_act(JavaVMInitArgs const*,
ScopedVMInitArgs*)+0x35c
V [libjvm.so+0x6b03ab] Arguments::parse(JavaVMInitArgs const*)+0xa9b
V [libjvm.so+0x178473f] Threads::create_vm(JavaVMInitArgs*, bool*)+0xcf
V [libjvm.so+0xff4ebb] JNI_CreateJavaVM+0x9b
C [libjli.so+0x39d1] JavaMain+0x81
Why did you need to introduce the ResourceMark?
David
On 3/03/2018 2:44 PM, David Holmes wrote:
> I'll sponsor this.
>
> I don't see Goetz's email to the list but will take it as per your
> response.
>
> Cheers,
> David
>
> On 3/03/2018 1:04 AM, Schmidt, Lutz wrote:
>> Hi David,
>>
>> it would be great if you could sponsor this change. I was able to
>> successfully test on darwinintel64, linuxs390x, linuxppc64, and
>> linuxx86_64. Our AIX systems are not playing nice with me at the
>> moment (no issues with the change, just general misbehavior).
>>
>> I have modified line 551 according to your suggestion (and line 519 as
>> well). Webrev updated in-place.
>>
>> So let's hope for a second review over the weekend.
>>
>> Regards,
>> Lutz
>>
>>
>> On 02.03.18, 02:19, "David Holmes" <david.holmes at oracle.com> wrote:
>>
>> On 1/03/2018 11:25 PM, Schmidt, Lutz wrote:
>> > Hi David,
>> > thank you for looking at this. You are right, the comment is a
>> useless leftover -> removed in if and else branch.
>> Looks fine. Just need a second reviewer. Do you need a sponsor to
>> test
>> on additional platforms? Otherwise what platforms have you tested?
>> > With the "\n" handling, I believe we are on the safe side. If a
>> newline character is detected in the parameter string, it is replaced
>> by a st->cr() call. That call does the expected on any platform, I
>> would hope. Flag::print_as_flag() (not in the scope of the change)
>> uses a similar handling.
>> >
>> > The newlines are contained in string literals in C code (e.g.
>> default values for parameters) or stem from ccstrlist concatenations.
>> That is all under VM control. So I do not see a risk here. You can try
>> yourself on any platform with the -XX:DisableIntrinsic=test1 parameter
>> multiple times.
>> >
>> > If a user manages to specify a parameter string with platform
>> (windows) specific line terminators and hopes for correct (\n-like)
>> handling, he or she will be disappointed. I would assume the
>> PrintFlags formatting isn't the only place that's impacted.
>> Sorry I mistakenly thought you had modified the newline handling,
>> when
>> you hadn't. If there is an issue it would be preexisting. I was
>> wondering how you would get a multi-line ccstr value. If you
>> entered it
>> on the command-line e.g:
>> java -XX:OnError="Line 1
>> Line2"
>> then I would expect to find the platform line separator within the
>> string. In testing this with the existing PrintFlagsFinal Linux
>> does:
>> ccstrlist OnError = Line 1
>> OnError += Line 2
>> {product} {command line}
>> but testing on Windows is a problem. The regular cmd shell can't
>> take
>> multi-line arguments. If you use the ^ escape trick it actually
>> strips
>> the newline and passes the arg as one line. So I guess the issue is
>> somewhat moot. :)
>> One further nit:
>> 551 st->print("%s", "+=");
>> should just be:
>> 551 st->print("+=");
>> Thanks,
>> David
>> > I have updated the webrev in-place with the comments removed.
>> >
>> > Thanks again, Lutz
>> >
>> >
>> > On 28.02.18, 23:26, "David Holmes" <david.holmes at oracle.com>
>> wrote:
>> >
>> > Hi Lutz,
>> >
>> > On 24/02/2018 2:48 AM, Schmidt, Lutz wrote:
>> > > Dear all,
>> > >
>> > > may I please request reviews for this small enhancement:
>> > >
>> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8198608
>> > > Webrev:
>> http://cr.openjdk.java.net/~lucy/webrevs/8198608.00/
>> > >
>> > > The code in Flag::print_on() so far wasn’t very easy to
>> understand. Changing the layout of what was printed required some deep
>> thinking. I hope that, with my changes, future modifications will be
>> easier.
>> > >
>> > > The before/after output of -XX:+PrintFlagsFinal is
>> identical, except for those argument names which are longer than
>> expected. In that case, the new version prints one space less, which
>> is by intention.
>> >
>> > This all seems okay - and easier to modify further if needed.
>> >
>> > Two minor comments:
>> >
>> > 576 // Flag::print_on(...) redesign (!print_ranges)
>> >
>> > Isn't this the print_ranges case? But in any case not sure
>> a comment
>> > with "redesign" in it is that meaningful given you can't
>> see the old design.
>> >
>> > Does the ccstr newline handling work on all platforms (ie
>> Windows) - I'm
>> > never sure when it suffices to check for '\n' and when we
>> have to check
>> > for the platform specific line terminators.
>> >
>> > Thanks,
>> > David
>> >
>> > > Thank you!
>> > > Lutz
>> > >
>> > >
>> > >
>> > > Dr. Lutz Schmidt | SAP JVM | PI SAP CP Core | T: +49
>> (6227) 7-42834
>> > >
>> >
>> >
>>
More information about the hotspot-runtime-dev
mailing list