RFR(S): 8074354: tests that intentionally crash the VM can timeout creating a core file
Yumin Qi
yumin.qi at oracle.com
Mon Mar 23 18:48:56 UTC 2015
Since Thomas is not a reviewer in open jdk, I need two volunteers (at
least one "R"eviewer).
Dan, since you already reviewed previous version, could you have a look?
Thanks
Yumin
On 3/20/2015 3:20 PM, Yumin Qi wrote:
> Thomas,
>
> Thanks. Yes, it is webrev04. Also, I have updated webrev04 to add
> another test case change: test/runtime/memory/ReadFromNoaccessArea.java
> (Thanks Dan's update)
>
> Thanks
> Yumin
>
> On 3/20/2015 11:55 AM, Thomas Stüfe wrote:
>>
>> Hi Yumin,
>>
>> I think you meant to post webrev.04?
>>
>> I looked at 04, and it looks nice. Thank you!
>>
>> (still only reviewer with r)
>>
>> Thomas
>>
>> On Mar 20, 2015 6:20 PM, "Yumin Qi" <yumin.qi at oracle.com
>> <mailto:yumin.qi at oracle.com>> wrote:
>>
>> Hi, All
>>
>> http://cr.openjdk.java.net/~minqi/8074354/webrev03/
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev03/>
>>
>> This version is based on Thomas' suggestion.
>> Tests passed vm.runtime.quick.testlist, JPRT and manual tests.
>>
>> Thanks
>> Yumin
>>
>> On 3/18/2015 10:06 AM, Yumin Qi wrote:
>>
>>
>> On 3/18/2015 4:26 AM, Thomas Stüfe wrote:
>>
>> Hi Yumin,
>>
>> - just aesthetics, but on all implementations of
>> os_abort(), at least on the UNIX variants, maybe we could
>> consistently rename the parameters to be "siginfo" and
>> "context" instead of using Windows terms?
>>
>> Sure.
>>
>> - On check_create_dump_limit(), you do not need
>> exceptionRecord, contextRecord parameters anymore. The
>> only parameters now needed are buffer and buffer size,
>> which is not even an output parameter, just a scratch
>> buffer for this function to use for printf. For output, it
>> calls VMError::report_coredump_status(...) at the end to
>> provide information about the core file.
>>
>> - I would rename that function from
>> check_create_dump_limit() to check_dump_limit() - nothing
>> is created anymore.
>>
>> Yes, no need to carry those two parameters. I tried to keep
>> less changes, but decided to change now.
>>
>> - on Windows, in os::abort(), there is no point anymore in
>> calling VMError::report_coredump_status(...) because that
>> information is only used during error log writing which
>> already happened. It only makes sense to call this
>> function in check_create_dump_limit(), which happens
>> before error log writing.
>> Instead, maybe error messages like "Call to
>> MiniDumpWriteDump() failed" should just be written to
>> stderr? This would be consistent with Unix, where the OS
>> writes a short message on stderr if core file writing fails.
>>
>> Sure, will direct output to stderr.
>>
>> - there is now a new test,
>> test/runtime/ErrorHandling/SafeFetchInErrorHandlingTest.java
>> - could you please also add "-XX:-CreateCoredumpOnCrash" ?
>> I just added the test and did not want to add the option
>> before it was available.
>>
>> OK
>>
>> Thanks
>> Yumin
>>
>> Thanks for your work!
>>
>> Thomas
>>
>>
>> On Tue, Mar 17, 2015 at 2:04 AM, Yumin Qi
>> <yumin.qi at oracle.com <mailto:yumin.qi at oracle.com>
>> <mailto:yumin.qi at oracle.com <mailto:yumin.qi at oracle.com>>>
>> wrote:
>>
>> Hi, Dan and all
>>
>> I have updated webrev at:
>> http://cr.openjdk.java.net/~minqi/8074354/webrev03/
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev03/>
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev03/>
>>
>> 1) bug changed synopsis
>> 8074354: Make CreateMinidumpOnCrash a new name
>> and
>> available on all platforms
>> 2) tests: JPRT, vm.runtime.quick.testlist (on
>> Windows), jtreg on
>> Windows and Linux/Unix.
>>
>> Thanks
>> Yumin
>>
>>
>> On 3/12/2015 12:59 PM, Daniel D. Daugherty wrote:
>>
>> I believe there are both JavaTest/JTREG tests in
>> the hotspot repo
>> and there are VM/NSK tests from back when we did
>> phone home...
>>
>> Check with Christian or Misha...
>>
>> Dan
>>
>>
>> On 3/12/15 1:50 PM, Yumin Qi wrote:
>>
>> Thanks, Dan
>>
>> Will look at and run those tests. Are they
>> under nsk?
>>
>> Yumin
>>
>> On 3/12/2015 12:29 PM, Daniel D. Daugherty
>> wrote:
>>
>> Yumin,
>>
>> There are some error handler tests. You
>> should find
>> those and make
>> sure that you run them on Windows since
>> you're
>> changing the order
>> there...
>>
>> Dan
>>
>>
>> On 3/12/15 12:42 PM, Yumin Qi wrote:
>>
>> Hi, Thomas and all
>>
>> The second version of the change:
>> webrev:
>> http://cr.openjdk.java.net/~minqi/8074354/webrev02/
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev02/>
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev02/>
>>
>> In this version:
>> 1) switch order on Windows to first
>> print log
>> file as other platform, then dump core
>> file if
>> needed. When VMError created after
>> crash, siginfo
>> and context which are ExceptionRecord
>> and
>> ContextRecord on Windows are recorded,
>> mini (or
>> full, due to 'or' used so type will be
>> mini
>> anyway) dump creates dump file based
>> on those two
>> as before.
>> 2) to make os::abort to get above
>> two pointers,
>> added two more fields to the function
>> parameters:
>> 3) changes for
>> test/runtime/ErrorHandling/SecondaryError.java ---
>> added "-XX:-CreateCoredumpOnCrash"
>>
>> - static void abort(bool
>> dump_core = true);
>> + static void abort(bool dump_core
>> = true,
>> void *siginfo = NULL, void *context =
>> NULL);
>>
>> Tests: JPRT and manually.
>>
>> Thanks
>> Yumin
>>
>> On 3/11/2015 3:47 AM, Thomas Stüfe
>> wrote:
>>
>> Hi Yumin,
>>
>> There is also
>> test/runtime/ErrorHandling/SecondaryErrorTest.java
>> - could you please add
>> "-XX:-CreateCoredumpOnCrash" ?
>> Thank you!
>>
>> Beyond that, as I wrote in the bug
>> report
>> comments:
>>
>> "This is also a problem on Windows -
>> MiniDumpWriteDump() may hang
>> infinitly. And on
>> Windows this is worse than under
>> UNIX because
>> we create the Dump before writing
>> the hs-err
>> file, so if the Dump hangs, we get
>> no error
>> log. I would like to revert the
>> order: create
>> the minidump after writing the
>> error log, the
>> same way Unix does it. We did this
>> in our JVM
>> (SAP) because for us, error logs
>> are more
>> useful than minidumps. "
>>
>> So, I would like to see os::abort
>> on Windows
>> call MiniDumpWriteDump(), and thus
>> the mini
>> dump writing moved after the
>> error log
>> writing. This would also make the
>> code way
>> cleaner because the control flow
>> would be the
>> same on all platforms.
>>
>> I understand that this may be out
>> of scope for
>> your change, but I would like to
>> know what
>> others think about this.
>>
>> Kind regards, Thomas
>>
>>
>>
>>
>>
>>
>>
>> On Wed, Mar 11, 2015 at 8:02 AM,
>> Yumin Qi
>> <yumin.qi at oracle.com
>> <mailto:yumin.qi at oracle.com>
>> <mailto:yumin.qi at oracle.com
>> <mailto:yumin.qi at oracle.com>>
>> <mailto:yumin.qi at oracle.com
>> <mailto:yumin.qi at oracle.com>
>> <mailto:yumin.qi at oracle.com
>> <mailto:yumin.qi at oracle.com>>>> wrote:
>>
>> Please review:
>>
>> bug:
>> https://bugs.openjdk.java.net/browse/JDK-8074354
>> webrev:
>> http://cr.openjdk.java.net/~minqi/8074354/webrev01/
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev01/>
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev01/>
>> <http://cr.openjdk.java.net/%7Eminqi/8074354/webrev01/>
>>
>> Summary: Tests timed out when
>> VM crashes
>> and dumping core file
>> which in the test case is not
>> needed. To
>> make core not created,
>> the fix changed
>> CreateMinidumpOnCrash to
>> CreateCoredumpOnCrash,
>> the former is only used on
>> Windows and the
>> latter for all
>> platforms. When VM crashes on
>> non Windows,
>> core file generated as
>> default if OS sets core dump
>> allowed.
>> Default value of
>> CreateCoredumpOnCrash set to
>> 'true' to
>> keep same behavior on non
>> Windows platforms, but changed
>> for Windows
>> --- original is false,
>> not create minidump on
>> Windows. With
>> CreateCoredumpOnCrash turned
>> off, no core file will be
>> generated.
>> CreateMinidumpOnCrash still
>> can be used on commandline but
>> only as
>> alias for the new flag.
>>
>> Tests: JPRT, manual tests
>> setting
>> CreateMinidumpOnCrash on
>> commandline to verify flag
>> change as alias.
>>
>> Thanks
>> Yumin
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
More information about the hotspot-runtime-dev
mailing list