December 2017 - core-libs-dev

[10] RFR 8187551: MessageFormat.setFormat(int, Format) AIOOBE not thrown when documented
by Nishit Jain 04 Dec '17

04 Dec '17

Hi, Please review the fix for JDK-8187551. Bug: https://bugs.openjdk.java.net/browse/JDK-8187551 Webrev: http://cr.openjdk.java.net/~nishjain/8187551/webrev.02/ Fix: As documented, updated MessageFormat.setFormat() to throw AIOOBE with invalid format element index. Thanks Martin Buchholz for filing the issue and suggesting the fix. Regards, Nishit Jain

3 4

Fwd: Re: RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved
by Peter Levart 04 Dec '17

04 Dec '17

Sending previous message to the list also (forgot to mention it in CC)... Hi Rogger, Interesting approach. Conditional finalization. You use finalization to support cases where user overrides finalize() and/or close() and Cleaner when he doesn't. I wonder if it is the right thing to use AltFinalizer when user overrides finalize() method. In that case the method is probably not empty and calls super.finalize() (if it is empty or doesn't call super, user probably doesn't want the finalization to close the stream) and so normal finalization applies. If you register AltFinalizer for such case, close() will be called twice. The logic should probably be 3-state: - if user overrides finalize() - do nothing; else - if user overrides close() - use AltFinalizer; else - use Cleaner Some additional comments: - FileInputStream.AltFinalizer#get could be written to not use exceptions for flow control. For example: static AltFinalizer get(FileInputStream fis) { return Stream.<Class<?>>iterate(fis.getClass(), cl -> cl != FileInputStream.class, Class::getSuperclass) .flatMap(clazz -> Stream.of(clazz.getDeclaredMethods())) .filter(m -> m.getParameterCount() == 0 && (m.getName().equals("close") || m.getName().equals("finalize"))) .findFirst() .map(m -> new AltFinalizer(fis)).orElse(null); } or, to support 3-state logic: static Optional<AltFinalizer> get(FileInputStream fis) { int flag = Stream. <Class<?>>iterate(fis.getClass(), cl -> cl != FileInputStream.class, Class::getSuperclass) .flatMap(clazz -> Stream.of(clazz.getDeclaredMethods())) .mapToInt(m -> m.getParameterCount() == 0 ? m.getName().equals("close") ? 1 : (m.getName().equals("finalize") ? 2 : 0) : 0) .reduce(0, (i, j) -> i | j); if ((flag & 2) != 0) { // overrides finalize() return Optional.empty(); } else if ((flag & 1) != 0) { // overrides close() return Optional.of(new AltFinalizer(fis)); } else { // overrides none return null; } } - similar for FileOutputStream.AltFinalizer That's all for now. Will try to check the rest later. Regards, Peter On 12/02/2017 03:38 AM, Roger Riggs wrote: > Please review a revision to the work on remove a dependency on > finalization from > FileInputStream, FileOutputStream, and add cleanup of closing file > descriptors in FileChannel. > > The previous version was too aggressive in removing the finalize > method and was considered to be insufficiently > backward compatible. > > For compatibility with previous versions, the close method will be > called when the FIS/FOS is unreachable > only when FIS/FOS has been subclassed to override close() or > finalize(). In other cases, the cleaner is used > to make sure the FileDescriptor is closed. > > Webrev: > http://cr.openjdk.java.net/~rriggs/webrev-fis-cleanup-8080225/ > > Issue: > https://bugs.openjdk.java.net/browse/JDK-8080225 > > CSR: > Relax FileInputStream/FileOutputStream requirement to use finalize > https://bugs.openjdk.java.net/browse/JDK-8187325 > > Thanks, Roger > > > >

2 1

Re: RFR 8080225 FileInput/OutputStream/FileChannel cleanup should be improved
by Peter Levart 04 Dec '17

04 Dec '17

Hi Rogger, On 12/04/2017 02:17 PM, Peter Levart wrote: > Hi Rogger, > > Interesting approach. Conditional finalization. You use finalization > to support cases where user overrides finalize() and/or close() and > Cleaner when he doesn't. > > I wonder if it is the right thing to use AltFinalizer when user > overrides finalize() method. In that case the method is probably not > empty and calls super.finalize() (if it is empty or doesn't call > super, user probably doesn't want the finalization to close the > stream) and so normal finalization applies. If you register > AltFinalizer for such case, close() will be called twice. Ah, scrap that. I forgot that XXXStream.finalize() is now empty, so user overriding it and calling super does not in fact close the stream. You have to register AltFinalizer in that case. But now I wonder if the logic should still be 3-state and do the following: - if user overrides finalize() - use AltFinalizer to call both: first finalize() and then close(); else - if user overrides close() - use AltFinalizer to call close(); else - use Cleaner What do you think? Regards, Peter

2 4

RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151
by Xueming Shen 04 Dec '17

04 Dec '17

Hi, Please help review the change for JDK-8191918: issue: https://bugs.openjdk.java.net/browse/JDK-8191918 webrev: http://cr.openjdk.java.net/~sherman/8191918/webrev This is the backport/identical change we have already putback into earlier update releases for JDK-8189789. It includes two changes (1) to replace the change we put into jdk9 for #8184306 with the "official" change currently in zlib github repo (https://github.com/madler/zlib/issues/275) http://cr.openjdk.java.net/~sherman/8184306 https://bugs.openjdk.java.net/browse/JDK-8184306 (2) to change the way we handle the returned result Z_BUF_ERROR. Now the implementation expects that there might be bytes read from the input and bytes written to the output buffer when the deflateParams()/deflate() returns Z_BUF_ERROR. It is "interpreted as there is no enough buffer space for all output, but the deflater has decoded input bytes and write out output bytes as many as possible, come back with more available output space". See related github/zlib discussions at #275 [1] and #305[2] Thanks, Sherman [1] https://github.com/madler/zlib/issues/275 [2] https://github.com/madler/zlib/issues/305

3 4

RFR (XXS): 8192961: Remove some double semicolons
by Langer, Christoph 04 Dec '17

04 Dec '17

Hi, can I please have a review for this small cleanup. I spotted some double semicolons at a few import statements. Bug: https://bugs.openjdk.java.net/browse/JDK-8192961 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8192961.0/ Thanks Christoph

2 1

MR JarFile was: Scanning multi version jars?
by Greg Wilkins 03 Dec '17

03 Dec '17

All, Alan suggested this as a more appropriate forum for some issues I'm having with multi release jar files. My issues break down into two aspects: 1. How does a container know which classes within a jar should be scanned for annotations - and that is being discussed in the other thread: "Scanning multi version jars" 2. Confusion about the JarFile API - which I'd like to discuss in this thread. At the very least I think the javadoc on JarFile is wrong, but I also think the behaviour is very confusing Consider a MR jar with the following contents: example.jar ├── META-INF │ ├── MANIFEST.MF │ └── versions │ └── 9 │ └── org │ └── example │ ├── InBoth.class │ └── OnlyIn9.class └── org └── example ├── InBoth.class └── OnlyInBase.class where for debugging purposes I've made the contents of each file be its path within the jar. If I run the following code: JarFile jarFile = new JarFile(new File("/tmp/example.jar"), false, JarFile.OPEN_READ, Runtime.version()); for (Enumeration<JarEntry> e = jarFile.entries(); e.hasMoreElements(); ) { JarEntry entry0 = e.nextElement(); String name0 = entry0.getName(); String content0 = IO.toString(jarFile.getInputStream(entry0)); JarEntry entry1 = jarFile.getJarEntry(name0); String name1 = entry1.getName(); String content1 = IO.toString(jarFile.getInputStream(entry1)); System.err.printf("%n=== %s ===%n -> %s%n => %s%n => %s%n",name0,name1,content0,content1); } I get the following output: === META-INF/ === -> META-INF/ => => === META-INF/MANIFEST.MF === -> META-INF/MANIFEST.MF => Manifest-Version: 1.0 Multi-Release: true Created-By: 9 (Oracle Corporation) => Manifest-Version: 1.0 Multi-Release: true Created-By: 9 (Oracle Corporation) === org/ === -> org/ => => === org/example/ === -> org/example/ => => === org/example/OnlyInBase.class === -> org/example/OnlyInBase.class => org/example/OnlyInBase.class => org/example/OnlyInBase.class === org/example/InBoth.class === -> org/example/InBoth.class => org/example/InBoth.class => META-INF/versions/9/org/example/InBoth.class === META-INF/versions/ === -> META-INF/versions/ => => === META-INF/versions/9/ === -> META-INF/versions/9/ => => === META-INF/versions/9/org/ === -> META-INF/versions/9/org/ => => === META-INF/versions/9/org/example/ === -> META-INF/versions/9/org/example/ => => === META-INF/versions/9/org/example/OnlyIn9.class === -> META-INF/versions/9/org/example/OnlyIn9.class => META-INF/versions/9/org/example/OnlyIn9.class => META-INF/versions/9/org/example/OnlyIn9.class === META-INF/versions/9/org/example/InBoth.class === -> META-INF/versions/9/org/example/InBoth.class => META-INF/versions/9/org/example/InBoth.class => META-INF/versions/9/org/example/InBoth.class The issue I have are that sometimes the class is trying to hide the MR aspects, yet other times it is not. Specifically when iterating it returns JarEntry instances that ignore versions and always return the content to which they refer, yet if you obtain a JarEntry from the getJarEntry API, it behaves differently and may return the versioned content even if it has the un-versioned path. Specifically in the above example, if I obtain a JarEntry for org/example/InBoth.class from the enumerator, then it always returns be the base entry. But if I do entry=jarFile.getEntry(entry.getName()), I obtain a JarEntry that has the name of the base entry, but gives me the versioned content when used as a reference. More over, there is nothing in the JarEntry API that allows me to tell if it is versioned or not (no getVersion()) My expectations of the versioned JarFile API were that the enumeration should only return entries appropriate for the version. So when configured for java8, the above jar would enumerate over: META-INF/ META-INF/MANIFEST.MF org/ org/example/ org/example/OnlyInBase.class org/example/InBoth.class and for java9 the enumeration should return META-INF/ META-INF/MANIFEST.MF org/ org/example/ org/example/InBoth.class org/example/OnlyIn9.class Ie the existence of the META-INF/versions structure should be hidden unless a non versioned JarFile is instantiated. I had also expected that I would be able to query a JarEntry to ask what version it was for. The issue being discussed in the other thread about how containers can be portable in their scanning of MR jars. If the JarFile enumeration provided the behaviour above, then containers would not need to implement their own filtering and could just rely on the versioned JarFile enumeration. Currently as is, each container will have to implement their own logic to determine what class files in a jar should be scanned for annotations etc. regards On 15 September 2017 at 12:09, Greg Wilkins <gregw(a)webtide.com> wrote: > > Alan, > > thanks for correcting me on the API of JarFile - I can see it kind of > works, but in a very bizarre way (it gives different content for entries > obtained via the enumerator vs the getJarEntry API, even though both > entries report the same name). But I'll discuss that elsewhere. > > > > The main issue still remains is that it is entirely unclear what files we > should scan. I understand the nuanced point that you are trying to make, > ie "that it depends" on if the class is public or private, if it is an API > change, if it is an alternate implementation rather than a new version of > the same library etc. etc. I also totally understand that there are > intended uses and unintended uses for this feature. > > However, as an implementer of an application container, it does not matter > if I understand the nuances of MR jars and intended usage. What matters is > do the developers of the 3rd party jars that will be deployed in my > container understand those nuances? We have to look at jars that are > supplied by third parties, with various levels of understanding, perhaps > with some tricky clever ideas how to mess with the system, and we have to > decide which classes we are going to scan for annotations. > > This is NOT a performance issue. It is a consistency/portability issue. > We have to make exactly the same decisions as all the other application > containers out there, else 3rd party library jars will act differently on > different containers. > > Thus it looks like we need some kind of heuristic to guess what the 3rd > party developer intended when they used the MR feature. Some approaches > will need us to scan all the outer and inner classes to determine if the > inner classes are referenced and if they are public or private. > > The heuristic could then be to analyse an inner class IFF it is public and > referenced. Or perhaps that should be if it is public OR referenced? > > Alternately, can we just have an heuristic based only on the index. If > Foo exists as a versioned class, then only similarly versioned Foo$Bar > classes should be scanned and base Foo$Bar classes will be ignored? > > All of these are possible. But we need an official documented (perhaps > tool enforced) policy so that all containers can implement the same > heuristic so that we can have portability. > > Ideally, the containers would not need to implement this heuristic, as it > would be implemented in the enumerator of JarFile. Unfortunately that is > not the case and the enumerator returns all the entries regardless of > version. So containers must implement their own enumeration and we need > to make sure we all implement it the same! > > regards > > > > > > On 14 September 2017 at 20:44, Alan Bateman <Alan.Bateman(a)oracle.com> > wrote: > >> On 14/09/2017 10:58, Weijun Wang wrote: >> >>> : >>> I know an MR jar allows you to shadow a class file with a >>> release-specific one, but what if the new release has removed an old class? >>> It will not appear in the release-specific directory but still exists in >>> the root. Should we describe this in the MANIFEST? >>> >>> A MR JAR is not intended to support multiple versions of the same >> library, instead the versioned sections are for classes that take advantage >> of newer language or API features. They help with the migration from using >> JDK internal APIs to supported/standard APIs for example. So I don't think >> it should be complicated by an additional list of entries to "hide" in the >> base or overlaid version sections. >> >> Greg's mail doesn't say if Bar is public so I can't tell if his example >> involves an attempted API change or not. Assuming Bar is not public then >> compiling the 9 version of Foo.java will generate Foo.class and no >> Foo$Bar.class. This doesn't mean it's completely orphaned of course as >> there may be other classes in the base section, and in the same package, >> that were compiled with references to Bar. The `jar` tool could do some >> additional validation to catch these references and so avoid >> IncompatibleClassChangeError at runtime (as might arise if >> getEnclosingClass were invoked on the inner class). That would help with >> Greg's annotation scanning scenario too. >> >> -Alan >> > > > > -- > Greg Wilkins <gregw(a)webtide.com> CTO http://webtide.com > -- Greg Wilkins <gregw(a)webtide.com> CTO http://webtide.com

2 2

RFR 8186961 Class.getFields() does not return fields of previously visited super interfaces/classes.
by Paul Sandoz 01 Dec '17

01 Dec '17

Hi, Please review: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-g… <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186961-iface-static-fields-g…> There is a bug lurking, perhaps for a while, where diamond patterns for interface hierarchies can result in the incorrect reporting of fields when using reflection, see the test case. Since reflection data is cached i don’t see an advantage to retaining a set of of traversed interfaces, which is the root cause of the issue. The code is optimized for common cases and will for less common cases collect fields of interfaces into a temporary linked hash set (to preserve the order, perhaps not strictly necessary but i did not want to change that behaviour). Thanks, Paul.

3 9

RFR (JDK10/JAXP) 8191938: Fix lint warnings in JAXP repo: a few Deprecation warrnings and enable -Xlint:all
by Joe Wang 01 Dec '17

01 Dec '17

Hi, Please review a fix for a few more deprecation warnings. Compiling with -Xlint:all showed that these were the last few warnings. We can then enable -Xlint:all for the java.xml module. JBS: https://bugs.openjdk.java.net/browse/JDK-8191938 webrevs: http://cr.openjdk.java.net/~joehw/jdk10/8191938/webrev/ Thanks, Joe

5 11

RFR: 8189102: All tools should support -?, -h and --help
by Lindenmaier, Goetz 01 Dec '17

01 Dec '17

Hi, please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477 See the webrev for a detailed description of the changes. If required, I'll make break-out changes to be reviewed separately. I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html Best regards, Goetz.

6 23