RFR(S): 8074354: tests that intentionally crash the VM can timeout creating a core file
Yumin Qi
yumin.qi at oracle.com
Wed Mar 18 17:06:27 UTC 2015
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