Hi Alex,
On 22/08/2018 5:36 AM, Alex Menkov wrote:
Hi David,
Updated fix:
http://cr.openjdk.java.net/~amenkov/sh2java/step1_zgc/webrev.01
I used "catch (Throwable)" instead suggested "catch (Error |
RuntimeException ex)" (we want to catch anything can be thrown)
Interesting that it worked. Throwable is a checked-exception so you
normally can't throw a Throwable unless you declare "throws Throwable".
But it seems our exception processing logic is getting smarter and can
see that the only Throwable's that can be caught are in fact the
unchecked Error and RuntimeException (which is why I suggested them) as
no checked-exceptions are possible from the code that is called.
Also I updated String.split to use \\R for the line-separators as you
suggested in review for
JDK-8209604: [TEST] rewrite com/sun/jdi shell tests to java version - step2
Looks good! Tkanks.
David
--alex
On 08/20/2018 18:29, David Holmes wrote:
Hi Alex,
On 21/08/2018 8:24 AM, Alex Menkov wrote:
Hi all,
please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8209605
webrev:
http://cr.openjdk.java.net/~amenkov/sh2java/step1_zgc/webrev/
The fix:
- updates the fix so it works as original shell test:
- drops "-Xmx32m" argument for debuggee (for shell test default
jtreg -Xmx512m was effective);
Okay. I don't think you need the historical commentary though - the
information in the bug report is sufficient.
- drops requirement for "Pause Full (System.gc())" (different GCs
use different logging, original test does not require it);
Okay .
- fixes "ArrayIndexOutOfBoundsException: Index 0 out of bounds for
length 0". The cause of the issue is String.split(sep), which can
returns zero-length array;
Okay.
- improve error handling - debuggee must be terminated if jdb
execution fails.
For this part I suggest:
catch (Error | RuntimeException ex)
so that, for example, OOME also triggers debuggee shutdown.
Thanks,
David
--alex
