February 2018 - core-libs-dev

RFR: 8197893: Mistaken type check in CheckedEntrySet.toArray
by Martin Buchholz 14 Feb '18

14 Feb '18

http://cr.openjdk.java.net/~martin/webrevs/jdk/CheckedEntrySet-toArray/ https://bugs.openjdk.java.net/browse/JDK-8197893

2 1

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers
by David Holmes 14 Feb '18

14 Feb '18

Adding in core-libs-dev as there's nothing related to hotspot directly here. David On 14/02/2018 9:32 PM, Adam Farley8 wrote: > Hi All, > > Currently, diagnostic core files generated from OpenJDK seem to lump all > of the > native memory usages together, making it near-impossible for someone to > figure > out *what* is using all that memory in the event of a memory leak. > > The OpenJ9 VM has a feature which allows it to track the allocation of > native > memory for Direct Byte Buffers (DBBs), and to supply that information into > the > cores when they are generated. This makes it a *lot* easier to find out > what is using > all that native memory, making memory leak resolution less like some dark > art, and > more like logical debugging. > > To use this feature, there is a native method referenced in Unsafe.java. > To open > up this feature so that any VM can make use of it, the java code below > sets the > stage for it. This change starts letting people call DBB-specific methods > when > allocating native memory, and getting into the habit of using it. > > Thoughts? > > Best Regards > > Adam Farley > > P.S. Code: > > diff --git > a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template > b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template > --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template > +++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template > @@ -85,7 +85,7 @@ > // Paranoia > return; > } > - UNSAFE.freeMemory(address); > + UNSAFE.freeDBBMemory(address); > address = 0; > Bits.unreserveMemory(size, capacity); > } > @@ -118,7 +118,7 @@ > > long base = 0; > try { > - base = UNSAFE.allocateMemory(size); > + base = UNSAFE.allocateDBBMemory(size); > } catch (OutOfMemoryError x) { > Bits.unreserveMemory(size, cap); > throw x; > diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java > b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java > --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java > +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java > @@ -632,6 +632,26 @@ > } > > /** > + * Allocates a new block of native memory for DirectByteBuffers, of > the > + * given size in bytes. The contents of the memory are > uninitialized; > + * they will generally be garbage. The resulting native pointer will > + * never be zero, and will be aligned for all value types. Dispose > of > + * this memory by calling {@link #freeDBBMemory} or resize it with > + * {@link #reallocateDBBMemory}. > + * > + * @throws RuntimeException if the size is negative or too large > + * for the native size_t type > + * > + * @throws OutOfMemoryError if the allocation is refused by the > system > + * > + * @see #getByte(long) > + * @see #putByte(long, byte) > + */ > + public long allocateDBBMemory(long bytes) { > + return allocateMemory(bytes); > + } > + > + /** > * Resizes a new block of native memory, to the given size in bytes. > The > * contents of the new block past the size of the old block are > * uninitialized; they will generally be garbage. The resulting > native > @@ -687,6 +707,27 @@ > } > > /** > + * Resizes a new block of native memory for DirectByteBuffers, to the > + * given size in bytes. The contents of the new block past the size > of > + * the old block are uninitialized; they will generally be garbage. > The > + * resulting native pointer will be zero if and only if the requested > size > + * is zero. The resulting native pointer will be aligned for all > value > + * types. Dispose of this memory by calling {@link #freeDBBMemory}, > or > + * resize it with {@link #reallocateDBBMemory}. The address passed > to > + * this method may be null, in which case an allocation will be > performed. > + * > + * @throws RuntimeException if the size is negative or too large > + * for the native size_t type > + * > + * @throws OutOfMemoryError if the allocation is refused by the > system > + * > + * @see #allocateDBBMemory > + */ > + public long reallocateDBBMemory(long address, long bytes) { > + return reallocateMemory(address, bytes); > + } > + > + /** > * Sets all bytes in a given block of memory to a fixed value > * (usually zero). > * > @@ -918,6 +959,17 @@ > checkPointer(null, address); > } > > + /** > + * Disposes of a block of native memory, as obtained from {@link > + * #allocateDBBMemory} or {@link #reallocateDBBMemory}. The address > passed > + * to this method may be null, in which case no action is taken. > + * > + * @see #allocateDBBMemory > + */ > + public void freeDBBMemory(long address) { > + freeMemory(address); > + } > + > /// random queries > > /** > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU >

5 8

RFR 8190378: Java EE and CORBA modules removal
by Lance Andersen 13 Feb '18

13 Feb '18

7 10

[11] RFR: 8197829: Optimize CLDRCalendarDataProviderImpl::retrieveInteger
by Claes Redestad 13 Feb '18

13 Feb '18

Hi, please review this improvement to the retrieveInteger method in CLDRCalendarDataProviderImpl. Bug: https://bugs.openjdk.java.net/browse/JDK-8197829 Webrev: http://cr.openjdk.java.net/~redestad/8197829/jdk.00/ This deals with a tiny startup regression that appeared in 10+36 on some applications and systems[1] Thanks! /Claes [1] Likely due to https://bugs.openjdk.java.net/browse/JDK-8190918

2 2

RFR: 8041626: [Event Request] Shutdown reason
by Robin Westberg 13 Feb '18

13 Feb '18

Hi all, Please review the following change that adds an event-based tracing event that is generated when the VM shuts down. The intent is to capture shutdowns that occur after the VM has been properly initialized (as initialization problems would most likely mean that the tracing framework hasn’t been properly started either). Issue: https://bugs.openjdk.java.net/browse/JDK-8041626 <https://bugs.openjdk.java.net/browse/JDK-8041626> Webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/> Testing: hs-tier1,hs-tier2,jdk-tier1,jdk-tier2 Best regards, Robin

5 14

IllegalAccessException trying to load a ResourceBundle in 9u181
by Vitaly Davidovich 13 Feb '18

13 Feb '18

Hi all, I'm not sure if core-libs is the right mailing list for jigsaw/modules questions these days (rather than jigsaw-dev), so please feel free to forward this there if it's the more appropriate list. I have the following code carried over from java 8 (actually much earlier than that, but that's beside the point): final Resource rb = ResourceBundle.getBundle("sun.security.util.AuthResources"); In 9u181, this fails with: Exception in thread "main" java.util.MissingResourceException: Can't find bundle for base name sun.security.util.AuthResources, locale en_US at java.base/java.util.ResourceBundle. throwMissingResourceException(ResourceBundle.java:2045) at java.base/java.util.ResourceBundle.getBundleImpl( ResourceBundle.java:1679) at java.base/java.util.ResourceBundle.getBundleImpl( ResourceBundle.java:1583) at java.base/java.util.ResourceBundle.getBundleImpl( ResourceBundle.java:1546) at java.base/java.util.ResourceBundle.getBundle( ResourceBundle.java:838) <…> Caused by: java.lang.IllegalAccessException: unnamed module can't load sun.security.util.AuthResources in module java.base at java.base/java.util.ResourceBundle$Control. newBundle(ResourceBundle.java:3167) at java.base/java.util.ResourceBundle.loadBundle( ResourceBundle.java:1984) at java.base/java.util.ResourceBundle.findBundle( ResourceBundle.java:1766) at java.base/java.util.ResourceBundle.findBundle( ResourceBundle.java:1718) at java.base/java.util.ResourceBundle.findBundle( ResourceBundle.java:1718) at java.base/java.util.ResourceBundle.getBundleImpl( ResourceBundle.java:1652) ... 4 more Not surprising given encapsulation. So I tried running with "--add-exports java.base/sun.security.util=ALL-UNNAMED" but that still fails, which *is* surprising based on my understanding. I also tried (for good measure) with a similar add-opens but no luck there either. I feel like I must be missing something obvious here, or doing something silly. What's the right way to allow the getBundle call to succeed here? Thanks

4 7

RFR 8197462 : Inconsistent exception messages for invalid capturing group names
by Ivan Gerasimov 13 Feb '18

13 Feb '18

Hello! Capturing group name can be used in a regular expression in two contexts: When introducing a group (?<name>...) or when referring it \k<name>. If the name is invalid (i.e. does not start with a Latin letter, or contains wrong chars) then we may see different error messages, some of which look confusing. Here are examples of the messages produced by the current JDK: Unknown look-behind group near index 3 (?<>) ^ named capturing group is missing trailing '>' near index 4 \\k<> ^ Unknown look-behind group near index 4 (?<.>) ^ (named capturing group <.> does not exit near index 4 \\k<.> ^ named capturing group is missing trailing '>' near index 4 (?<a.>) ^ named capturing group is missing trailing '>' near index 4 \\k<a.> ^ In particular, this diversity is caused by that the internal Pattern.groupname() function lacks a check for the very first character of the name. So that when \k<name> is parsed, the first char is always accepted, no matter what it was. Some cleanup was also done along the way. Would you please help review the fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8197462 WEBREV: http://cr.openjdk.java.net/~igerasim/8197462/00/webrev/ Thanks in advance! -- With kind regards, Ivan Gerasimov

2 1

RFR: 8196770: Add JNDI test com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java
by Chris Yin 13 Feb '18

13 Feb '18

Please review the added JNDI test com/sun/jndi/ldap/blits/AddTests/AddNewEntry.java, thanks It’s to verify capability to add a new entry to the directory using the ADD operation. This test will use LDAPServer to playback dump ldap message to simulate test environment. bug: https://bugs.openjdk.java.net/browse/JDK-8196770 webrev: http://cr.openjdk.java.net/~xiaofeya/8196770/webrev.00/ Regards Chris

2 2

RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java
by Chris Yin 13 Feb '18

13 Feb '18

Please review the added JNDI test javax/naming/dns/AttributeTests/GetAny.java, thanks It’s to cover the case that we can get the attributes of a DNS entry using special qualifiers. This test will use DNSServer to playback dump dns message (which created by DNSTracer when run test against real dns server) to simulate test environment. bug: https://bugs.openjdk.java.net/browse/JDK-8195976 <https://bugs.openjdk.java.net/browse/JDK-8195976> webrev: http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/ <http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/> Regards Chris

3 5

Re: RFR 8184289: Obsolete -XX:+UnsyncloadClass and -XX:+MustCallLoadClassInternal options
by David Holmes 13 Feb '18

13 Feb '18

Hi Harold, Adding core-libs-dev so they are aware of the ClassLoader change. On 10/02/2018 5:44 AM, harold seigel wrote: > Hi David, > > Thanks for reviewing this. > > Please see updated webrev: > http://cr.openjdk.java.net/~hseigel/bug_8184289.2/webrev/index.html This all seems fine to me. I agree there is question mark over the SystemDictionary code now only used for the null/boot loader: 848 if ((class_loader.is_null())) { 849 if (k == NULL && HAS_PENDING_EXCEPTION 850 && PENDING_EXCEPTION->is_a(SystemDictionary::LinkageError_klass())) { 851 MutexLocker mu(SystemDictionary_lock, THREAD); 852 InstanceKlass* check = find_class(d_hash, name, dictionary); 853 if (check != NULL) { 854 // Klass is already loaded, so just use it 855 k = check; 856 CLEAR_PENDING_EXCEPTION; 857 guarantee((!class_loader.is_null()), "dup definition for bootstrap loader?"); 858 } 859 } This seems to be a negative test, that we should in fact never get in this situation when dealing with the boot loader. But in that case we don't need lines 855 and 856 anymore and the code would just collapse to: 848 if ((class_loader.is_null())) { 849 if (k == NULL && HAS_PENDING_EXCEPTION 850 && PENDING_EXCEPTION->is_a(SystemDictionary::LinkageError_klass())) { 851 MutexLocker mu(SystemDictionary_lock, THREAD); 852 guarantee(find_class(d_hash, name, dictionary) == NULL, 853 "dup definition for bootstrap loader?"); 854 } 855 } and in that case I'd probably rather see this as an assertion than a guarantee, and the whole block can be debug-only. Thanks, David ----- > And, please see in-line comments. > > > On 2/8/2018 5:42 PM, David Holmes wrote: >> Hi Harold, >> >> First I'm pretty sure this one can't be pushed until the version bump >> arrives in jdk/hs :) > I hope the version bump arrives soon. >> >> On 9/02/2018 6:53 AM, harold seigel wrote: >>> Hi, >>> >>> Please review this JDK-11 change to obsolete the UnsyncloadClass and >>> MustCallLoadClassInternal options. With this change, these options >>> are still accepted on the command line but have no affect other than >>> to generate these warning messages: >>> >>> Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option >>> UnsyncloadClass; support was removed in 11.0 >>> >>> Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option >>> MustCallLoadClassInternal; support was removed in 11.0 >>> >>> Open Webrev: >>> http://cr.openjdk.java.net/~hseigel/bug_8184289/webrev/index.html >> >> Overall looks good. Tricky to untangle things in the SD especially! >> >> src/hotspot/share/classfile/systemDictionary.cpp >> >> Looking at this change, and the comment: >> >> - // For UnsyncloadClass only >> // If they got a linkageError, check if a parallel class load >> succeeded. >> // If it did, then for bytecode resolution the specification >> requires >> // that we return the same result we did for the other thread, >> i.e. the >> // successfully loaded InstanceKlass >> // Should not get here for classloaders that support parallelism >> // with the new cleaner mechanism, even with >> AllowParallelDefineClass >> // Bootstrap goes through here to allow for an extra guarantee >> check >> ! if (UnsyncloadClass || (class_loader.is_null())) { >> >> It's not clear why all the "UnsyncLoadClass only" stuff is also being >> done for the "Bootstrap" (? bootloader?) case. But in any case the >> comment block doesn't read correctly now as this is all, and only, >> about the bootstrap case. I'd suggest: >> >> - // For UnsyncloadClass only >> + // For bootloader only: >> // If they got a linkageError, check if a parallel class load >> succeeded. >> // If it did, then for bytecode resolution the specification >> requires >> // that we return the same result we did for the other thread, >> i.e. the >> // successfully loaded InstanceKlass >> // Should not get here for classloaders that support parallelism >> // with the new cleaner mechanism, even with >> AllowParallelDefineClass >> - // Bootstrap goes through here to allow for an extra guarantee >> check > Done. >> >> >> Question: is ClassLoader.loadClassInternal now obsolete as well? > Yes. Thanks for pointing that out. The new webrev contains significant > changes needed to remove loadClassInternal. >> >> --- >> >> src/hotspot/share/runtime/arguments.cpp >> >> Is there some reason to leave the expiration version unset? Do you >> think the obsoletion warning may be needed for a couple of releases ?? > I figured whoever expires the option can put in the version. >> >> --- >> >> test/hotspot/jtreg/runtime/CommandLine/ObsoleteFlagErrorMessage.java >> >> You don't need to add anything here. This is not a test of all >> obsolete flags, it is just testing some specific handling of obsolete >> flags. > I reverted this change. > > Thanks! Harold >> >> Thanks, >> David >> ----- >> >>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8184289 >>> >>> The change was tested with Mach5 tiers 1-5 and the NSK parallel class >>> loading tests. Also, JDK's containing the change were built on all >>> Mach5 platforms. >>> >>> Thanks, Harold >>> >

5 7