RFR (XS): 8066670 - PrintSharedArchiveAndExit does not exit the VM when the archive is invalid

[David Holmes]

Thanks Ioi - good to go.

David

On 5/12/2014 11:37 AM, Ioi Lam wrote:
> Hi David,
>
> Thanks for the review. I have fixed the test case(and also made similar
> changes in the closed test case)
>
> ioilinux ~/jdk/jdk9/hotspot$ hg diff
> diff -r a711ea14195a
> test/runtime/SharedArchiveFile/PrintSharedArchiveAndExit.java
> --- a/test/runtime/SharedArchiveFile/PrintSharedArchiveAndExit.java Thu
> Dec 04 15:20:09 2014 -0800
> +++ b/test/runtime/SharedArchiveFile/PrintSharedArchiveAndExit.java Thu
> Dec 04 17:36:49 2014 -0800
> @@ -45,7 +45,7 @@
>             "-XX:+PrintSharedArchiveAndExit", "-version");
>         output = new OutputAnalyzer(pb.start());
>         output.shouldContain("archive is valid");
> -      output.shouldNotContain("Java HotSpot(TM)"); // Should not print
> JVM version
> +      output.shouldNotContain("java version");     // Should not print
> JVM version
>         output.shouldHaveExitValue(0);               // Should report
> success in error code.
>
>         pb = ProcessTools.createJavaProcessBuilder(
> @@ -56,14 +56,14 @@
>         output.shouldNotContain("Usage:");           // Should not print
> JVM help message
>         output.shouldHaveExitValue(0);               // Should report
> success in error code.
>
> -      // (2) With an valid archive (boot class path has been prepended)
> +      // (2) With an invalid archive (boot class path has been prepended)
>         pb = ProcessTools.createJavaProcessBuilder(
>             "-Xbootclasspath/p:foo.jar",
>             "-XX:+UnlockDiagnosticVMOptions",
> "-XX:SharedArchiveFile=./sample.jsa",
>             "-XX:+PrintSharedArchiveAndExit", "-version");
>         output = new OutputAnalyzer(pb.start());
>         output.shouldContain("archive is invalid");
> -      output.shouldNotContain("Java HotSpot(TM)"); // Should not print
> JVM version
> +      output.shouldNotContain("java version");     // Should not print
> JVM version
>         output.shouldHaveExitValue(1);               // Should report
> failure in error code.
>
>
> - Ioi
>
> On 12/4/14, 5:15 PM, David Holmes wrote:
>> Hi Ioi,
>>
>> Thanks for the new open test. Two nits:
>>
>>   output.shouldNotContain("Java HotSpot(TM)"); // Should not print JVM
>> version
>>
>> This won't work for an OpenJDK build.
>>
>>  59       // (2) With an valid archive (boot class path has been
>> prepended)
>>
>> valid -> invalid I assume?
>>
>> Thanks,
>> David
>>
>> On 4/12/2014 10:04 PM, Ioi Lam wrote:
>>>
>>> On 12/4/14, 3:43 AM, David Holmes wrote:
>>>> On 4/12/2014 9:40 PM, Ioi Lam wrote:
>>>>>
>>>>> On 12/4/14, 3:27 AM, David Holmes wrote:
>>>>>> Hi Ioi,
>>>>>>
>>>>>> On 4/12/2014 8:32 PM, Ioi Lam wrote:
>>>>>>> Hi Folks,
>>>>>>>
>>>>>>> Please review a small fix:
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8066670
>>>>>>> http://cr.openjdk.java.net/~iklam/8066670-PrintSharedArchiveAndExit/
>>>>>>>
>>>>>>> Summary of fix:
>>>>>>>
>>>>>>> Do not set UseSharedSpaces to falseif PrintSharedArchiveAndExit is
>>>>>>> enabled.
>>>>>>>
>>>>>>>      After this fix, the JVM correctly exits when
>>>>>>> PrintSharedArchiveAndExit is enabled and an invalid archive is
>>>>>>> encountered.
>>>>>>
>>>>>> The change to metaspaceShared.cpp is fine.
>>>>>>
>>>>>> In filemap.cpp I'm less clear on the logic. It seems that if
>>>>>> _validating_classpath_entry_table is false then we will still
>>>>>> continue, even if PrintSharedArchiveAndExit is true.
>>>>>>
>>>>> The goal is try to print out as much information as possible. It turns
>>>>> out the most useful information with PrintSharedArchiveAndExit is to
>>>>> find out which part of the classpath is invalid. When
>>>>> _validating_classpath_entry_table is true, we know it's safe to
>>>>> print an
>>>>> error message (about a part of the classpath that's invalid) and
>>>>> continue.
>>>>>
>>>>> When doing other things (_validating_classpath_entry_table==false),
>>>>> it's
>>>>> less clear whether we can continue if a failure is encountered. In
>>>>> this
>>>>> case, since PrintSharedArchiveAndExit is true, RequireSharedSpaces is
>>>>> automatically set to true (by arguments.cpp), so we will print out the
>>>>> error message and exit immediately.
>>>>
>>>> Okay - thanks for explaining.
>>>>
>>>>>>>
>>>>>>>      New test cases are in closed source code.
>>>>>>
>>>>>> Begs the question as to why there can't be an open test for this?
>>>>>>
>>>>> I will add an open test as well.
>>>>
>>> I added the new test in the open code, under the same location:
>>>
>>> http://cr.openjdk.java.net/~iklam/8066670-PrintSharedArchiveAndExit/
>>>
>>> Thanks
>>> - Ioi
>