RFR(S): 8074354: tests that intentionally crash the VM can timeout creating a core file
Yumin Qi
yumin.qi at oracle.com
Fri Mar 20 17:22:39 UTC 2015
Hi, All
http://cr.openjdk.java.net/~minqi/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>> 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/>
>>
>> 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/>
>>
>> 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>>> 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/>
>>
>> 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