RFR(S): JDK-7041183 Improve error handling in Improve error handling in src/share/vm/services/management.cpp
Jini George
jini.george at oracle.com
Mon Nov 23 02:35:44 UTC 2015
Hi David,
Thanks much for the review and for the suggestions offline. I have addressed your comments and have a new webrev at:
http://cr.openjdk.java.net/~dsamersoff/sponsorship/jingeorg/JDK-7041183/webrev.02/
(Thanks, Dmitry for hosting). Further comments are inlined below.
> -----Original Message-----
>
> On 30/10/2015 9:01 PM, Dmitry Samersoff wrote:
> > Everybody,
> >
> > (* On behalf of Jini George <jini.george at oracle.com> *)
> >
> > Please review the fix
> >
> > http://cr.openjdk.java.net/~dsamersoff/sponsorship/jingeorg/JDK-
> 704118
> > 3/webrev.01/
>
> If you throw IllegalArgumentException it should be obvious at the Java
> level which argument is illegal and why. I can't tell at the Java level
> how that can come about - these seem to be methods invoked on a
> MemoryPoolMXBean - see for example:
>
> https://bugs.openjdk.java.net/browse/JDK-8025089
>
> so the "illegal argument" would seem to be "this" ???
[[Jini]] This has been changed to the more valid IllegalStateException as you suggested. JDK-8025089 is not reproducible anymore and has been closed now.
> I don't see any changes to the code that would currently throw Internal
> Error ??
[[Jini]] I removed the code throwing the Internal Error from the upper layer now since this would have masked the exception thrown from the inner layer.
> The asserts for pool!=NULL seem rather pointless as the method has been
> changed to never return null. If you really want the assert add it to
> the end of the method being called, rather than placing at all the call
> sites.
[[Jini]] I removed these asserts now.
Thanks,
Jini.
More information about the serviceability-dev
mailing list