October 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

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(m): 8177290 add copy factory methods for unmodifiable List, Set, Map
by Stuart Marks 30 Nov '17

30 Nov '17

(also includes 8184690: add Collectors for collecting into unmodifiable List, Set, and Map) Hi all, Here's an updated webrev for this changeset; the previous review thread is here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-September/049261.… This webrev includes the following: * specification revisions to provide clearer definitions of "view" collections, "unmodifiable" collections, and "unmodifiable views" * new List.copyOf(), Set.copyOf(), and Map.copyOf() "copy factory" methods * new Collectors.toUnmodifiableList, Set, and Map methods * tests for the new API methods I've added some assertions that require some independence between the source collection (or map) and the result of the copyOf() method. I've made a small but significant change to Set.copyOf compared to the previous round. Previously, it specified that the first of any equal elements was preserved. Now, it is explicitly unspecified which of any equals elements is preserved. This is consistent with Set.addAll, Collectors.toSet, and the newly added Collectors.toUnmodifiableSet, none of which specify which of duplicate elements is preserved. (The outlier here is Stream.distinct, which specifies that the first element of any duplicates is preserved, if the stream is ordered.) I've also made some minor wording/editorial changes in response to suggestions from David Holmes and Roger Riggs. I've kept the wording changes that give emphasis to "unmodifiable" over "immutable." The term "immutable" is inextricably intertwined with "persistent" when it comes to data structures, and I believe we'll be explaining this forever if Java's "immutable" means something different from everybody else's. Webrev: http://cr.openjdk.java.net/~smarks/reviews/8177290/webrev.1/ Bugs: https://bugs.openjdk.java.net/browse/JDK-8177290 add copy factory methods for unmodifiable List, Set, Map https://bugs.openjdk.java.net/browse/JDK-8184690 add Collectors for collecting into unmodifiable List, Set, and Map Thanks, s'marks

14 40

ThreadPoolExecutor and finalization
by Roger Riggs 28 Nov '17

28 Nov '17

Hi, With the deprecation of Object.finalize its time to look at its uses too see if they can be removed or mitigated. The ThreadPoolExecutor overrides finalize to shutdown the pool. A simple but incomplete step is to retain the shutdown on finalize but remove it from the specification of ThreadPoolExecutor.finalize (and remove the override). [1] For those familiar with Executors please take as look at the situation and make a recommendation or suggestion. Thanks, Roger [1] https://bugs.openjdk.java.net/browse/JDK-8190324

9 40

RFR: 8187954 Update JAX-WS RI integration to latest version
by Jack Li 22 Nov '17

22 Nov '17

Hi, Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo. JBS: https://bugs.openjdk.java.net/browse/JDK-8187954 <https://bugs.openjdk.java.net/browse/JDK-8187954> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8187954/10/00/ <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8187954/10/00/> Summary of changes: jaxws/src/java.xml.bind/share/classes/javax/xml/bind/* JDK-8186946 - Fix accessibility and other issues in the java.xml.bind module jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/** JDK-8186314 - code at c.s.x.i.m.saaj.soap.MessageImpl must be modified to avoid crash after javac change And also contains the fixes for importing nodes for SOAPDocumentFragment Patch also contains several small bugfixes, not tracked in JBS. ---------------- Best regards Jack Li

3 13

[10] RFR 8186046 Minimal ConstantDynamic support
by Paul Sandoz 20 Nov '17

20 Nov '17

Hi, Please review the following patch for minimal dynamic constant support: http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support… <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support…> https://bugs.openjdk.java.net/browse/JDK-8186046 <https://bugs.openjdk.java.net/browse/JDK-8186046> https://bugs.openjdk.java.net/browse/JDK-8186209 <https://bugs.openjdk.java.net/browse/JDK-8186209> This patch is based on the JDK 10 unified HotSpot repository. Testing so far looks good. By minimal i mean just the support in the runtime for a dynamic constant pool entry to be referenced by a LDC instruction or a bootstrap method argument. Much of the work leverages the foundations built by invoke dynamic but is arguably simpler since resolution is less complex. A small set of bootstrap methods will be proposed as a follow on issue for 10 (these are currently being refined in the amber repository). Bootstrap method invocation has not changed (and the rules are the same for dynamic constants and indy). It is planned to enhance this in a further major release to support lazy resolution of bootstrap method arguments. The CSR for the VM specification is here: https://bugs.openjdk.java.net/browse/JDK-8189199 <https://bugs.openjdk.java.net/browse/JDK-8189199> the j.l.invoke package documentation was also updated but please consider the VM specification as the definitive "source of truth" (we may clean up this area further later on so it becomes more informative, and that may also apply to duplicative text on MethodHandles/VarHandles). Any AoT-related work will be deferred to a future release. — This patch only supports x64 platforms. There is a small set of changes specific to x64 (specifically to support null and primitives constants, as prior to this patch null was used as a sentinel for resolution and certain primitives types would never have been encountered, such as say byte). We will need to follow up with the SPARC platform and it is hoped/anticipated that OpenJDK members responsible for other platforms (namely ARM and PPC) will separately provide patches. — Many of tests rely on an experimental byte code API that supports the generation of byte code with dynamic constants. One test uses class file bytes produced from a modified version of asmtools. The modifications have now been pushed but a new version of asmtools need to be rolled into jtreg before the test can operate directly on asmtools information rather than embedding class file bytes directly in the test. — Paul.

7 17

JDK-8067661: transferTo proposal for Appendable
by Patrick Reinhart 15 Nov '17

15 Nov '17

Hi Chris & Pavel, Based on the transferTo Method on the InputStream I would propose to introduce a default method on the Readable interface. In that way the method can be used for all existing implementations of Readable and still be implemented in a special way by a individual implementer. /** * Reads all characters from this readable and writes the characters to * the given appendable in the order that they are read. On return, this * readable will be at end its data. * <p> * This method may block indefinitely reading from the readable, or * writing to the appendable. The behavior for the case where the readable * and/or appendable is <i>asynchronously closed</i>, or the thread * interrupted during the transfer, is highly readable and appendable * specific, and therefore not specified. * <p> * If an I/O error occurs reading from the readable or writing to the * appendable, then it may do so after some characters have been read or * written. Consequently the readable may not be at end of its data and * one, or both participants may be in an inconsistent state. It is strongly * recommended that both readable and appendable be promptly closed * if needed and an I/O error occurs. * * @param out the appendable, non-null * @return the number of characters transferred * @throws IOException if an I/O error occurs when reading or writing * @throws NullPointerException if {@code out} is {@code null} * * @since 1.9 */ default long transferTo(Appendable out) throws IOException { .... } -Patrick

5 26

RFR JDK-8185582, Update Zip implementation to use Cleaner, not finalizers
by Xueming Shen 05 Nov '17

05 Nov '17

Hi, Please help review the change for JDK-8185582. issue: https://bugs.openjdk.java.net/browse/JDK-8185582 webrev: http://cr.openjdk.java.net/~sherman/8185582/webrev/ csr: https://bugs.openjdk.java.net/browse/JDK-8187485 The proposed change here is to replace the finalize() cleanup mechanism with the newly introduced java.lang.ref.Cleaner for java.util.zip package (Deflater, Inflater, ZipFile and ZipFile.ZipFileInputStrean/ZipFileInflaterInputStream). Since it's an "incompatible" change, both behavioral and source incompatibility, to remove the public finalize() from these classes, a corresponding CSR is also created and need review as well. Thanks, Sherman

8 51

[PROPOSAL][JDK10] Introduce Executable.getParameterType(index)
by Christoph Dreis 03 Nov '17

03 Nov '17

Hey, while discussing JDK-8029019 & JDK-8189266 Claes came up with the idea to introduce non-copying access to the underlying parameter types of an Executable [1]. While there is a (for a good reason?) package-private Method.getSharedParameterTypes() already I thought of an alternative that doesn't open up the parameterTypes array to the outside world and actually solves the use-case where we wanted an allocation-free alternative for Method.getParameterTypes()[index]. Here is a draft of what I'm thinking of: Executable: /** * Returns the {@code Class} object that represents the formal * parameter type, at the given index, of the executable * represented by this object. The given index starts at 0 and * represents the declaration order of the parameters. * * * @param index index to look for the parameter type * @return the parameter type at the given index for the executable this object * represents * @throws IndexOutOfBoundsException if the parameter type * at the given index of the underlying executable could not be found */ public abstract Class<?> getParameterType(int index); Method & Constructor: /** * {@inheritDoc} * @throws IndexOutOfBoundsException {@inheritDoc} * @since 18.3 */ public Class<?> getParameterType(int index) { if (index < 0 || index > getParameterCount()) { throw new IndexOutOfBoundsException("No parameter found on index " + index); } return parameterTypes[index]; } As I don't know what the official @since should look like in the future, do not pay too much attention on that. I'm not happy with the documentation wording though. What do you think about that? Is someone willing to sponsor this given it's considered worthwhile. Cheers, Christoph [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049520.htm l

5 16

[PATCH] 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics
by Chris Dennis 02 Nov '17

02 Nov '17

Hi Paul (et al) Like all things API there are wrinkles here when it comes to implementing. This patch isn’t final, there appears to be no existing test coverage for these classes beyond testing the compensating summation used in the double implementation, and I left off adding any until it was decided how much parameter validation we want (since that’s the only testing that can really occur here). The wrinkles relate to the issues around instances that have suffered overflow in count and/or sum which the existing implementation doesn’t defend against: * I chose to ignore all parameters if 'count == 0’ and just leave the instance empty. I held off from validating min, max and count however. This obviously 'breaks things’ (beyond how broken they would already be) if count == 0 only due to overflow. * I chose to fail if count is negative. * I chose to enforce that max and min are logically consistent with count and sum, this can break the moment either one of the overflows as well (I’m also pretty sure that the FPU logic is going to suffer here too - there might be some magic I can do with a pessimistic bit of rounding on the FPU multiplication result). I intentionally went with the most aggressive parameter validation here to establish one end of what could be implemented. There are arguments for this and also arguments for the other extreme (no validation at all). Personally I’m in favor of the current version where the creation will fail if the inputs are only possible through overflow (modulo fixing the FPU precision issues above) even though it’s at odds with approach of the existing implementation. Would appreciate thoughts/feedback. Thanks. Chris P.S. I presume tests would be required for the parameter checking (once it is finalized)?

8 23