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 22:20:15 UTC 2015
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