November 2017 - core-libs-dev

Re: Manifest Related Bugs JDK-6372077, JDK-6202130, JDK-8176843, JDK-4842483, JDK-6443578, JDK-6910466, JDK-4935610, and JDK-4271239
by Sean Mullan 01 Mar '18

01 Mar '18

Hi Phillip, All of these bugs are in the core-libs area, so I am copying the core-libs-dev list since that is where they should be discussed and reviewed. I have -bcc-ed security-dev (where this was originally posted). --Sean On 10/2/17 1:24 PM, Philipp Kunz wrote: > Hi, > > While fixing JDK-6695402 I came across other related bugs to manifests > such as JDK-6372077, JDK-6202130, JDK-8176843, JDK-4842483, and > JDK-6443578 which all relate to manifest reading and writing. Somewhere > bug 7071055 is mentioned but I cannot find anywhere. Another group of > bugs, JDK-6910466, JDK-4935610, and JDK-4271239 concern the mandatory > manifest main attributes Manifest-Version or Signature-Version and at > first glance are duplicates. If you know of more known bugs, not > necessarily present in jira, I'd be glad to get notified. > > There are also some comments about utf handling and line breaking in the > code of Manifest: > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > > Furthermore, Attributes.map should declare appropriate type parameters: > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > The specification would also require that `header names must not start > with _, - or "From"` > (http://docs.oracle.com/javase/7/docs/technotes/guides/jar/jar.html#Section-…) > but I would opt not to add this as a hard restriction because I guess it > can be assumed that such names are in use now after having been working > for years. A warning to a logger might be conceivable such as in > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > Attribute values are never checked at all and invalid characters such as > line breaks are never detected except that when reading the manifest > again the values are cut off. > The tab character (U+0008) does not work in manifest values. > I suspect that there are also issues regarding the iteration order but I > haven't got a prove yet unlike for the other points above: > http://hg.openjdk.java.net/jdk10/master/file/a0116bcc65b7/src/java.base/sha… > There is duplicated or very similar code in Attributes and Manifest: > Attributes.write-Manifest.write-Attributes.writeMain and > Attributes.read-Manifest.read. > Resolving JDK-6202130 would have the additional benefit to be able to > view manifests with any utf-8 capable editor even if multi-byte > characters break across lines. > > Fixing these issues individually looks like more complicated work than > fixing them all at once, I guess, also because of a very low current > test coverage. So I'd suggest to add some thorough tests along with > fixing these issues. But before I start I would like to get some > guidance, assistance or at least confirmation on how to proceed. I'm new > to open jdk and have submitted only one patch so far. > > Is it ok to add tests for things that have worked before? > Is it ok to refactor duplicated code just for the added value to reduce > effort for testing? > I assume compatibility to and from existing manifests is the highest > priority, correct? This would also be the hard part in adding as > complete test coverage as possible. What would be acceptable criteria to > meet? > Why does Attributes not extend LinkedHashMap and why does Manifest not > extend HashMap? Any objection? > While I would not want to write code that looks slow or change more than > necessary one can never know before having performance actually > measured. Is there some way this is dealt with or should I wait for such > feedback until after patch submission? > > Would someone sponsor? > > Regards, > Philipp > > > > > ------------------------------------------------------------------------ > > > > Paratix GmbH > St Peterhofstatt 11 > 8001 Zürich > > +41 (0)76 397 79 35 > philipp.kunz(a)paratix.ch <mailto:philipp.kunz@paratix.ch>

5 12

[10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line
by Vladimir Kozlov 13 Dec '17

13 Dec '17

http://cr.openjdk.java.net/~kvn/8191788/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8191788 This is redo of JDK-8190975 [1] fix which added jdk.internal.vm.compiler to tests which have --limit-modules option. Unfortunately tests start failing on SPARC where Graal (jdk.internal.vm.compiler) is not available. JDK-8191653 [2] reversed 8190975 changes to fix that problem. This fix adds jdk.internal.vm.compiler to --limit-modules list in JVM only when Graal is used: when it is explicitly specified with -Djvmci.Compiler=graal or in default case and when UseJVMCICompiler is true. I tested the fix with failed tests from JDK-8190975 which are mostly AppCDS tests in test/runtime/appcds/jigsaw and also test/jdk/java/lang/String/concat/WithSecurityManager.java I think this fix may break upgradeable status of Graal (for Oracle Labs version of Graal). But it should be fine since it is only used with --limit-modules which is not used by Labs. Thanks, Vladimir [1] http://hg.openjdk.java.net/jdk/hs/rev/8deb7919d118 [2] http://hg.openjdk.java.net/jdk/hs/rev/b38d8aadcada

7 20

Incorrect validation of DST in java.util.SimpleTimeZone
by Venkateswara R Chintala 11 Dec '17

11 Dec '17

Hi, In a multi-threaded environment, when java.util.SimpleTimeZone object is used to create a default timezone, there can be a race condition between the methods java.util.Timezone.getDefault() and java.util.Timezone.getDefaultRef() which can result in inconsistency of cache that is used to validate a particular time/date in DST. When a thread is cloning a default timezone object (SimpleTimeZone) and at the same time if a different thread modifies the time/year values, then the cache values (cacheYear, cacheStart, cacheEnd) can become inconsistent which leads to incorrect DST determination. We considered two approaches to fix the issue. 1)Synchronize access to cloning default timezone object when cache is being modified. 2)Invalidate the cache while returning the clone. We preferred the second option as synchronization is more expensive. We have attached the patch and jtreg testcase. Please review. -- Regards -Venkat

6 25

Change in properties for logging: deliberate?
by Jeremy Manson 07 Dec '17

07 Dec '17

Hey folks, I can't find reference to this in a relnote, so I thought I would ask. If we have the following files: logging.properties: # Set the level of the root logger .level = INFO # Send the root logger's output to the console .handlers = java.util.logging.ConsoleHandler Test.java: public class Test { public static void main(String[] args) { java.util.logging.Logger.getAnonymousLogger().info("Hi!"); } } In Java 8, we will see console output, and in Java 9, we won't. # Java 8 $ java -Djava.util.logging.config.file=logging.properties Test Nov 08, 2017 5:02:05 PM Test main INFO: Hi! # Java 9 $ java -Djava.util.logging.config.file=logging.properties Test I thought that, perhaps, a decision was made to have the empty string no longer mean the root logger, so I took it off of .handlers: logging.properties: # Set the level of the root logger .level = INFO # Send the root logger's output to the console handlers = java.util.logging.ConsoleHandler Success in the sense that this got me output in Java 9. However, .level still seems to work; if I then have: # Set the level of the root logger .level = WARNING # Send the root logger's output to the console handlers = java.util.logging.ConsoleHandler it sets the root logger's level to WARNING. I don't see any of this in a relnote anywhere, and I don't see an obvious bug about it anywhere. There seem to have been a fair few changes to java.util.logging in Java 9. Was a documented decision made to have .handlers stop meaning handlers on the root logger, but to allow .level to continue meaning level on the root logger? Or do I just have the wrong end of the stick on this? Thanks! Jeremy

5 17

RFR 8187222 : ClassLoader.getSystemClassLoader not clear if recursive initialization leads to ISE or unspecified error
by Brent Christian 05 Dec '17

05 Dec '17

Hi, Please review the following change: Bug: https://bugs.openjdk.java.net/browse/JDK-8187222 Webrev: http://cr.openjdk.java.net/~bchristi/8187222/webrev.00/ The method description of ClassLoader.getSystemClassLoader() states (in regards to setting a custom system classloader): "If circular initialization of the system class loader is detected then an unspecified error or exception is thrown." This ambiguity in the method description can and should be removed. The method also has a @throws tag: * @throws IllegalStateException * If invoked recursively during the construction of the class * loader specified by the "{@code java.system.class.loader}" * property. JDK 8 threw an IllegalStateException. This changed to an InternalError in 9b111[1]. Throwing an IllegalStateException conforms to the @throws tag, and returns to the previous behavior of JDK 8 and (earlier on in) 9. Also, as Alan points out in the bug, an InternalError looks like a bug in the JDK itself, whereas IllegalStateException better reflects that the issue likely lies in the custom classloader. Automated build & test job passes cleanly. Thanks, -Brent 1. https://bugs.openjdk.java.net/browse/JDK-8142968

3 10

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

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