Re: JDK 9 RFR of JDK-8172475: Remove usage from Class and ClassLoader

2017-01-09 Thread Mandy Chung
> On Jan 9, 2017, at 5:45 PM, joe darcy wrote: > > Hello, > > The javadoc for java.lang.Class and java.lang.ClassLoader has some usages of > the HTML tag. This tag is not part of HTML 5 and has been replaced > with . In javadoc, direct use of or is > rarely necessary since the {@code } ta

Re: JDK 9 RFR of JDK-8172475: Remove usage from Class and ClassLoader

2017-01-09 Thread joe darcy
Hi Brian, For my rough methodology, I did a search and replace of "" with "{@code} and "" with "}" and then ran the docs build fixing issues with the {@link} with embedded {@code} until the docs build was successful again. Thanks, -Joe On 1/9/2017 5:59 PM, Brian Burkhalter wrote: Hi Joe,

Re: JDK 9 RFR of JDK-8172475: Remove usage from Class and ClassLoader

2017-01-09 Thread Brian Burkhalter
Hi Joe, I did not read every last line but a quick perusal does not reveal any straggling teletype tags. Line 1018 of ClassLoader could use a space before the “-1:” 1018 * {@code b.position()} through {@code b.position() + b.limit() -1 The current copyright year needs to be 2017 (!

JDK 9 RFR of JDK-8172475: Remove usage from Class and ClassLoader

2017-01-09 Thread joe darcy
Hello, The javadoc for java.lang.Class and java.lang.ClassLoader has some usages of the HTML tag. This tag is not part of HTML 5 and has been replaced with . In javadoc, direct use of or is rarely necessary since the {@code } tag can be used instead. In addition, ClassLoader has multiple e

Re: RFR: JDK-8172432,jar cleanup/update for module and mrm jar

2017-01-09 Thread Mandy Chung
> On Jan 9, 2017, at 5:28 PM, Xueming Shen wrote: > > http://cr.openjdk.java.net/~sherman/8172432/webrev/ > > the method has been updated as Paul suggested, to only take the root > module-info.class > and correct versioned meta-inf/versions/n/module-info.class Does it miss “/“ between n and

Re: RFR: JDK-8172432,jar cleanup/update for module and mrm jar

2017-01-09 Thread Xueming Shen
Thanks Mandy! webrev has been updated accordingly. http://cr.openjdk.java.net/~sherman/8172432/webrev/ On 1/9/17, 3:46 PM, Mandy Chung wrote: This is a good cleanup. This makes it easier to add any further validation such as JDK-8171830. Main.java 693 warn("unexpected

Re: RFR: JDK-8172432,jar cleanup/update for module and mrm jar

2017-01-09 Thread Xueming Shen
Paul, Thanks for the comments. The webrev has been updated accordingly. http://cr.openjdk.java.net/~sherman/8172432/webrev On 1/9/17, 2:21 PM, Paul Sandoz wrote: At this time of year: usual review comment to update the years in the license. Main — 987 jentries.stream().for

JDK 9 RFR of 8153250: java.io.File does not handle Windows paths of the form "D:" (no path) correctly

2017-01-09 Thread Brian Burkhalter
Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8153250 Patch: http://cr.openjdk.java.net/~bpb/8153250/webrev.00/ On Windows only, when resolving a child path against a parent, do not interpose a file separator (slash) if the parent is a two-character string

Re: core-libs-dev Digest, Vol 117, Issue 23

2017-01-09 Thread Asim Aslam
> On Jan 9, 2017, at 5:45 PM, core-libs-dev-requ...@openjdk.java.net wrote: > > Send core-libs-dev mailing list submissions to >core-libs-dev@openjdk.java.net > > To subscribe or unsubscribe via the World Wide Web, visit >http://mail.openjdk.java.net/mailman/listinfo/core-libs-dev > or,

Re: RFR (JAXP) 8171243 : CatalogManager.catalogResolver throws FileSystemNotFoundException with jar

2017-01-09 Thread huizhe wang
On 1/9/2017 2:17 PM, Roger Riggs wrote: Hi Joe, a few comments: CatalogManager: - line 58: "will /return /as no mapping is found"; Or is it describing the behavior of the CatalogResolver (which is throw a CatalogException)? (possible more than 1 place) Should have been empty Catalog

Re: RFR: JDK-8172432,jar cleanup/update for module and mrm jar

2017-01-09 Thread Mandy Chung
> On Jan 9, 2017, at 10:21 AM, Xueming Shen wrote: > > Hi, > > Please review the following proposed changes for jar tool > > issue: https://bugs.openjdk.java.net/browse/JDK-8172432 > > webrev: http://cr.openjdk.java.net/~sherman/8172432/webr

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Martin Buchholz
I followed up on my own suggestion and wrote a race-simulating test without the big hammer: /** * Checks that traversal operations collapse a random pattern of * dead nodes as could normally only occur with a race. */ @Test(dataProvider = "traversalActions") public void

Re: RFR of JDK-8172347: Refactoring src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java to improve testability of rmiregistry

2017-01-09 Thread Hamlin Li
On 2017/1/9 13:55, Roger Riggs wrote: Hi Hamlin, Its time to use 2017 for copyright dates. Hi Roger, Yes, the new year! :-) The method name launch does not reflect is really happening. Launch implies an active process or thread is being started. But in this case there is no active element; i

Re: RFR: JDK-8172432,jar cleanup/update for module and mrm jar

2017-01-09 Thread Paul Sandoz
Hi, A nice cleanup. At this time of year: usual review comment to update the years in the license. Main — 987 jentries.stream().forEach( je -> addPackageIfNamed(packages, je)); If you wish you can remove the “.stream()” and go straight to “.forEach(…)” on the Set. 1870

Re: RFR (JAXP) 8171243 : CatalogManager.catalogResolver throws FileSystemNotFoundException with jar

2017-01-09 Thread Roger Riggs
Hi Joe, a few comments: CatalogManager: - line 58: "will /return /as no mapping is found"; Or is it describing the behavior of the CatalogResolver (which is throw a CatalogException)? (possible more than 1 place) Check the copyrights -> 2017 (JarUtils.java) in JAXWS repo: - Options.

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Paul Sandoz
> On 9 Jan 2017, at 13:27, Martin Buchholz wrote: > > My whitebox tests tend to use private access only for reading, but I can > imagine cases where it is useful to give testing code stronger access than > for regular VarHandles, maybe even to write final fields, for example for > fuzz testin

Re: RFR of JDK-8172347: Refactoring src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java to improve testability of rmiregistry

2017-01-09 Thread Roger Riggs
Hi Hamlin, Its time to use 2017 for copyright dates. The method name launch does not reflect is really happening. Launch implies an active process or thread is being started. But in this case there is no active element; it just creates and exports a remote object. Otherwise, looks fine. Thank

Re: RFR of JDK-8172347: Refactoring src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java to improve testability of rmiregistry

2017-01-09 Thread Hamlin Li
Hi Roger, Thank you for reviewing, please check the comments inline, and new webrev: http://cr.openjdk.java.net/~mli/8172347/webrev.03/ Thank you -Hamlin On 2017/1/9 13:04, Roger Riggs wrote: Hi Hamlin, In addition to Mark's comments: How about this for the method comment: /** * Return

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Martin Buchholz
On Mon, Jan 9, 2017 at 11:29 AM, Paul Sandoz wrote: > Hi Martin, > > Have you tried using: > > MethodHandles.privateLookupIn(ConcurrentLinkedQueue.class, > MethodHandles.lookup()). > findVarHandle(…) > > ? > Thanks! privateLookupIn was not in my toolbox. I'm happy with: final VarHand

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-09 Thread Naoto Sato
Hi Sergei, java.base only supports Locale.US locale (and its parents Locale.ENGLISH/Locale.ROOT). I still see tests that use other locales, such as Locale.UK, Locale.FRENCH (eg. in TestDateTimeFormatterBuilder.java). Those test should also need to be separated. Naoto On 1/9/17 1:45 AM, Ser

Re: RFR of JDK-8172347: Refactoring src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java to improve testability of rmiregistry

2017-01-09 Thread Roger Riggs
Hi Hamlin, In addition to Mark's comments: How about this for the method comment: /** * Return a new RegistryImpl on the requested port and export it to serve registry requests. * A classloader is initialized from the system property "env.class.path" and * a security manager is set unl

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Paul Sandoz
Hi Martin, Have you tried using: MethodHandles.privateLookupIn(ConcurrentLinkedQueue.class, MethodHandles.lookup()). findVarHandle(…) ? I agree there is some inconsistency here, and we wanted to discourage the use of setAccessible. One problem is setAccessible conflates accessibility wit

RFR: 8055206: Update SecurityManager::checkPackageAccess to restrict non-exported JDK packages by default

2017-01-09 Thread Sean Mullan
Please review this JDK 9 change to make the SecurityManager::checkPackageAccess and checkPackageDefinition implementations restrict access to the same set of internal JDK packages as the module system. This overall change will improve security by making these two mechanisms consistent and red

Re: RFR of JDK-8172347: Refactoring src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java to improve testability of rmiregistry

2017-01-09 Thread Hamlin Li
Hi Mark, Thank you for reviewing, please check comments inline, and new webrev: http://cr.openjdk.java.net/~mli/8172347/webrev.02/. Thank you -Hamlin On 2017/1/9 6:36, Mark Sheppard wrote: Hi Hamlin, If I read the changes correctly, there would appear to be a side effect of your refactor e

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Paul Sandoz
> On 9 Jan 2017, at 05:36, Alan Bateman wrote: > > On 05/01/2017 19:01, Paul Sandoz wrote: > >> Hi, >> >> I encountered some circularity issues with security manager and VarHandles, >> specifically when attempting to use the new MethodHandles.privateLookupIn, >> so say ThreadLocalRandom has

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Uwe Schindler
Hi, It's also inconsistent, because the same type of code works with MethodHandles. If you make something accessible (and succeed doing this), it uses some special internal lookup that has no restrictions for unreflect. Uwe Am 9. Januar 2017 19:43:44 MEZ schrieb Martin Buchholz : >Relatedly,

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Martin Buchholz
Relatedly, I'm writing whitebox jtreg tests and would like to use VarHandles to access internal data structures. Because I have: * @modules java.base/java.util.concurrent:open I can use reflection with setAccessible to obtain a usable Field, but if I try to turn that into a VarHandle I get:

Re: RFR (JAXP) 8171243 : CatalogManager.catalogResolver throws FileSystemNotFoundException with jar

2017-01-09 Thread huizhe wang
On 1/9/2017 10:02 AM, Daniel Fuchs wrote: Hi Joe, BaseEntry.java: 213 * @return The URI created from the specified uri 214 * @throws IllegalArgumentException if the specified uri is null, 215 * or a URL can not be created based on the specified base and uri 216 */ 2

RFR: JDK-8172432,jar cleanup/update for module and mrm jar

2017-01-09 Thread Xueming Shen
Hi, Please review the following proposed changes for jar tool issue: https://bugs.openjdk.java.net/browse/JDK-8172432 webrev: http://cr.openjdk.java.net/~sherman/8172432/webrev http://cr.openjdk.java.net/~sherman/8172432/webrev_top/ (1) move the "packages" and "jarEntries" from "gl

Re: RFR (JAXP) 8171243 : CatalogManager.catalogResolver throws FileSystemNotFoundException with jar

2017-01-09 Thread Daniel Fuchs
Hi Joe, BaseEntry.java: 213 * @return The URI created from the specified uri 214 * @throws IllegalArgumentException if the specified uri is null, 215 * or a URL can not be created based on the specified base and uri 216 */ 217 URL verifyURI(String arg, URL base, Stri

RFR (JAXP) 8171243 : CatalogManager.catalogResolver throws FileSystemNotFoundException with jar

2017-01-09 Thread huizhe wang
Hi, The current Catalog API accepts file paths or URIs in a form of String to create Catalog or CatalogResolver in an effort to maintain consistency with the old Catalog API and other existing processors. However, that also introduced an ambiguity in the API, which is unwanted for a new API i

Re: [9] RFR: 8159058: SAXParseException when sending soap message

2017-01-09 Thread Lance Andersen
Hi Aleks, The changes seem reasonable. I am assuming that the JAXWS/SAAJ standalone tcks were run against the standalone RIs by the jaxws team? The JCK tests are a variation of the standalone tests so might be good to sanity check if not already done. Best Lance > On Jan 9, 2017, at 4:34 AM,

Re: Review Request JDK-8170772: ResourceBundle improper caching causes tools/javadoc tests intermittently

2017-01-09 Thread Naoto Sato
Either case is fine with me. I have the patch and wrote some additional test cases. Naoto On 1/9/17 9:02 AM, Mandy Chung wrote: On Jan 9, 2017, at 7:58 AM, Naoto Sato wrote: On 1/6/17 10:57 PM, Peter Levart wrote: Hi Mandy, Naoto, I think Mandy's original proposal has been pushed, but th

Re: Review Request JDK-8170772: ResourceBundle improper caching causes tools/javadoc tests intermittently

2017-01-09 Thread Mandy Chung
> On Jan 9, 2017, at 7:58 AM, Naoto Sato wrote: > > On 1/6/17 10:57 PM, Peter Levart wrote: >> Hi Mandy, Naoto, >> >> I think Mandy's original proposal has been pushed, but the >> simplification of CacheKey is still open: >> >> https://bugs.openjdk.java.net/browse/JDK-8171139 >> >> as well as

Re: Review Request JDK-8170772: ResourceBundle improper caching causes tools/javadoc tests intermittently

2017-01-09 Thread Naoto Sato
On 1/6/17 10:57 PM, Peter Levart wrote: Hi Mandy, Naoto, I think Mandy's original proposal has been pushed, but the simplification of CacheKey is still open: https://bugs.openjdk.java.net/browse/JDK-8171139 as well as the re-examination of clearCahe methods: https://bugs.openjdk.java.net/brow

Re: RFR of JDK-8172347: Refactoring src/java.rmi/share/classes/sun/rmi/registry/RegistryImpl.java to improve testability of rmiregistry

2017-01-09 Thread Mark Sheppard
Hi Hamlin, If I read the changes correctly, there would appear to be a side effect of your refactor extract method "run", such that the static member variable registry is no longer set in the main method, and is set in the run method ? Thus, invoking run multiple times (, whether that is intend

Re: jdk.internal.reflect.ReflectionFactory and SecurityManager

2017-01-09 Thread Alan Bateman
On 05/01/2017 19:01, Paul Sandoz wrote: Hi, I encountered some circularity issues with security manager and VarHandles, specifically when attempting to use the new MethodHandles.privateLookupIn, so say ThreadLocalRandom has access to fields in Thread [1]. When running with a security manager

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-09 Thread Rachna Goel
Hi Sergei, Just for curiosity, what is the purpose of moving package statement down. Sorry for pointing out, Copyright year needs to be updated to 2017. Thanks, Rachna On 09/01/17 3:15 PM, Sergei Kovalev wrote: Hi All, Re-sending request because I'm still looking for reviewers. In addition

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-09 Thread Sergei Kovalev
Hi All, Re-sending request because I'm still looking for reviewers. In addition for TEST.properties modification I'd like to put citation from Jon G. email. FWIW, I note your TEST.properties file looks malformed. It has 2 modules entries (line 4, line 7), which by the standard rules of Java

[9] RFR: 8159058: SAXParseException when sending soap message

2017-01-09 Thread Aleks Efimov
Hi, Please, help to review the JDK9 fix for the issue [1] in processing of default namespace attribute with empty values (xmlns="") by SAAJ implementation: http://cr.openjdk.java.net/~aefimov/8159058/9/00 Same fix was already integrated into JAXWS-RI and SAAJ-RI standalone projects. These ch