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.geo...@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.