Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Amy Lu
On 5/3/17 12:53 AM, Daniel Fuchs wrote: Hi Amy, Looks good generally - there are a couple of cases where the description that was passed to the AssertionError in the original file is dropped (for instance

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Amy Lu
Thank you Pavel, Paul, Roger and Daniel for your reviewing. On 5/3/17 5:02 AM, Roger Riggs wrote: But ": Must throw ClassCastException for parameter which is not Comparable." in Empty navigableMap is much more useful. And wrapping exceptions in exceptions makes debugging much harder, because

tutorial on using Cleaner-based finalization

2017-05-02 Thread Rick Hillegas
When I compile Apache Derby using JDK 9 build 167, I see several instances of the following warning: warning: [deprecation] finalize() in Object has been deprecated The javadoc for java.lang.Object.finalize() suggests that affected classes should migrate their finalization to a coding

Re: RFR: 8078267 - Add test to verify that a module based JDBC driver via the service-provider loading mechanism

2017-05-02 Thread Lance Andersen
Hi Joe, > On May 2, 2017, at 8:34 PM, huizhe wang wrote: > > Hi Lance, > > In DriverManagerModuleTests, at line 83, did you mean to assign the result > of DM::getDriver to d2 although an Exception is expected? I must have accidentally deleted that and never noticed

Re: RFR: 8078267 - Add test to verify that a module based JDBC driver via the service-provider loading mechanism

2017-05-02 Thread huizhe wang
Hi Lance, In DriverManagerModuleTests, at line 83, did you mean to assign the result of DM::getDriver to d2 although an Exception is expected? Otherwise, the patch looks good to me. Best, Joe On 5/2/2017 11:33 AM, Lance Andersen wrote: Hi all, The above issue adds a test to verify that

Re: RFR: JDK-8179538: Update jdk.jdi to be HTML-5 friendly

2017-05-02 Thread serguei.spit...@oracle.com
Hi Kumar, It looks fine to me too. Thanks, Serguei On 5/2/17 16:23, Mandy Chung wrote: Looks fine to me. Mandy On May 2, 2017, at 12:40 PM, Kumar Srinivasan wrote: Hello, Please review changes to make jdk.jdi HTML5 friendly, table cell padding has not

Re: RFR: JDK-8179538: Update jdk.jdi to be HTML-5 friendly

2017-05-02 Thread Mandy Chung
Looks fine to me. Mandy > On May 2, 2017, at 12:40 PM, Kumar Srinivasan > wrote: > > Hello, > > Please review changes to make jdk.jdi HTML5 friendly, > table cell padding has not been addressed and will be fixed > separately, this takes care of others. > >

Re: AppCDS in OpenJDK?

2017-05-02 Thread Jiangli Zhou
Hi Deepak, In JDK 8u40, AppCDS is an experimental (i.e can not be used in production) feature and is not available in OpenJDK. Following is related info from release notes for 8u40 : Application Class Data Sharing

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Pavel Rappo
> On 2 May 2017, at 22:02, Roger Riggs wrote: > > If the test was being written new I would advocate to add messages that > reflect the semantics of > the test case, not the exception message. > > But ": Must throw ClassCastException for parameter which is not

Re: Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

2017-05-02 Thread Mandy Chung
Hi Peter, Looking at it again, you are right that no need to skip inflation. The change is simplified. I have verified with and without -Dsun.reflect.noInflation=true. http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8020801/webrev.01/ thanks Mandy > On May 2, 2017, at 1:17 PM, Peter

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Roger Riggs
Hi Pavel, In one case, 'should throw NPE' is not a useful message; for example in parallelPrefix. If the test was being written new I would advocate to add messages that reflect the semantics of the test case, not the exception message. But ": Must throw ClassCastException for parameter

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Pavel Rappo
I'm not sure I fully understand the points mentioned by Roger, Daniel and Paul. Nevertheless I will kindly disagree with all of them, just for the sake of having a constructive discussion :-) My understanding is that org.testng.Assert.assertThrows provides a rich message explaining what has

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Alan Bateman
On 02/05/2017 19:00, Remi Forax wrote: Hi Kirk, --patch-module both at compile time and at runtime is what your are looking for. Right, and more details in the "Patching module content" section of JEP 261 [1]. -Alan [1] http://openjdk.java.net/jeps/261#Patching-module-content

Re: Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

2017-05-02 Thread Peter Levart
On 05/02/2017 06:56 PM, Mandy Chung wrote: On May 2, 2017, at 3:14 AM, Peter Levart wrote: I don't quite understand the need for bypassing the inflation of native into generated method accessor The VM native reflection implementation does not know about this

RFR: JDK-8179538: Update jdk.jdi to be HTML-5 friendly

2017-05-02 Thread Kumar Srinivasan
Hello, Please review changes to make jdk.jdi HTML5 friendly, table cell padding has not been addressed and will be fixed separately, this takes care of others. Note: Some of the errors were due to incorrect@throws but in all cases there is the correct one further down, please use sdiffs in

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Paul Sandoz
Hi, Looks good. Preserving the messages where possible would be good as raised by Roger and Daniel. — As a follow on it would be nice to avoid the duplication of the assertThrowsNPE etc. I wish by default for TestNG jtreg would include an additional general helper library in scope that we

RFR: 8078267 - Add test to verify that a module based JDBC driver via the service-provider loading mechanism

2017-05-02 Thread Lance Andersen
Hi all, The above issue adds a test to verify that a module based JDBC driver works via the service-provider loading mechanism. The webrev can be found at http://cr.openjdk.java.net/~lancea/8078267/webrev.00/index.html Best

AppCDS in OpenJDK?

2017-05-02 Thread Ramana, Deepak
Hello, I am not sure if this is the right forum to ask this question. Apologies in advance, if this is not the correct place. We want to use the AppCDS feature that is available in the commercial release of Java 8 update 40. Is this feature available in OpenJDK? If so, can we use this in

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Remi Forax
Hi Kirk, --patch-module both at compile time and at runtime is what your are looking for. cheers, Rémi - Mail original - > De: "Kirk Pepperdine" > À: "Alan Bateman" > Cc: "jigsaw-dev" ,

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Kirk Pepperdine
Hi Alan, One of the techniques that I’ve used in the past for troubleshooting is to prepend modified classes to the bootstrap class path. I fear all this mucking about with modularization is going to leave the road to diagnostics littered with a ton of tools that will not longer work. As it

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Michael Nascimento
On Tue, May 2, 2017 at 2:15 PM, Paul Sandoz wrote: > At one point we were mulling back porting the Java 9 runtime changes to > Java 8 and therefore we could go back to Java 7, but we decided not to > proceed with that, which is what i suspect you may be thinking about. >

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Paul Sandoz
> On 2 May 2017, at 09:45, Michael Nascimento wrote: > > On Tue, May 2, 2017 at 11:51 AM, Brian Goetz wrote: > >> 8 can deal with multi-release JARs. >> > > I missed that. Starting with what update? Is there any link I could use to > share this

Re: Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

2017-05-02 Thread Mandy Chung
> On May 2, 2017, at 3:14 AM, Peter Levart wrote: > > I don't quite understand the need for bypassing the inflation of native into > generated method accessor The VM native reflection implementation does not know about this alternate `reflected$XXX` mechanism. No VM

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Daniel Fuchs
Hi Amy, Looks good generally - there are a couple of cases where the description that was passed to the AssertionError in the original file is dropped (for instance http://cr.openjdk.java.net/~amlu/8023897/webrev.00/test/java/util/Map/Defaults.java.frames.html) Is that going to be an issue when

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Brian Goetz
8 can deal with multi-release JARs. The extra goop is ignored. So a 7/8/9 MRJar is perfectly possible (and in fact, the whole point of doing them in the first place, so library writers could publish JARs that span 7/8/9.) On 5/2/2017 2:50 AM, Jeremy Manson wrote: Maybe when there's something

Re: JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Pavel Rappo
Hi Amy, Thanks a lot for doing this! I personally think it's a beginning of a very valuable refactoring to our test base. It turns out this little piece of functionality, namely, checking that a particular exception is thrown from code is error-prone. I have seen many buggy implementations

Re: Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

2017-05-02 Thread Andrew Dinn
Hi Mandy, On 02/05/17 03:37, Mandy Chung wrote: > Webrev: > http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8020801/webrev.00/ > > . . . Not an official review, I am afraid, but the patch looks good and is much appreciated! regards, Andrew Dinn --- Senior Principal Software

Re: Review Request: JDK-8020801: Apply the restriction of invoking MethodHandles.lookup to j.l.r.Method.invoke

2017-05-02 Thread Peter Levart
Hi Mandy, On 05/02/2017 04:37 AM, Mandy Chung wrote: Webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8020801/webrev.00/ The big hammer check disallowing MethodHandles::lookup be called by system classes defined by the bootstrap class loader was added as defense-in-depth to prevent

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Andrew Dinn
On 25/04/17 09:19, Andrew Dinn wrote: > This discussion really ought to be happening on the Byteman forum but > anyway ... > > Yes, Alan is right that this is exactly what is going on. Byteman on > jdk9 (the 4.0.0-BETA release series) now uses method handles in place of > reflection.

JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

2017-05-02 Thread Amy Lu
Please review this test-only change. Some java/util tests use a function executeAndCatch which essentially asserts that an exception is thrown, while some other tests use assertThrows for doing the same work. For both cases, with jtreg upgraded to testng 6.9.5 (CODETOOLS-7901639), test can

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Alan Bateman
On 02/05/2017 07:50, Jeremy Manson wrote: : If we follow this path, before we migrate to Java 9, we would need to make sure all of our code builds and the tests pass with Java 9 and Java 8. We can't make all of the code build and the tests pass with Java 9 as-is, because many of them use

Re: Accessing module internals from bytecode rewriting agent

2017-05-02 Thread Jeremy Manson
Maybe when there's something breaking in Java 10 or 11, I can do that. Right now, that approach requires me to have a fork of my code, have special build goop, rely on yet another Java 9 feature, and make my relatively widely used OSS project require Java 9 to build *prior to Java 9's release -*