RFR(S): 8074354: tests that intentionally crash the VM can timeout creating a core file
Yumin Qi
yumin.qi at oracle.com
Wed Mar 25 18:11:13 UTC 2015
Hi, all
I updated the webrev with a new change to test case:
test/runtime/Unsafe/RangeCheck.java
Add -XX:-CreateCoredumpOnCrash to test, no dump needed on this case.
new webrev: http://cr.openjdk.java.net/~minqi/8074354/webrev05/
Thanks
Yumin
On 3/23/2015 11:48 AM, Yumin Qi wrote:
> 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