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