April 2016 - core-libs-dev

RFR(XXS): 8149519: Investigate implementation of java.specification.version
by Volker Simonis 28 Jun '16

28 Jun '16

Hi, can I please have a review for this small fix: http://cr.openjdk.java.net/~simonis/webrevs/2016/8149519 https://bugs.openjdk.java.net/browse/JDK-8149519 Currently the value of the java.specification.version property comes from VERSION_SPECIFICATION in common/autoconf/spec.gmk.in. It is currently set to VERSION_NUMBER which is the same value which is also used for the java.version property. This is a bad idea, because VERSION_NUMBER is a dot separated sequence of numbers (e.g. 9.0.1) which is expected to change frequently (i.e. for every build and/or update version). If we are configuring with "--with-version-patch=1" for example, VERSION_NUMBER and java.version will be "9.0.0.1". But it makes no sense that VERSION_SPECIFICATION and java.specification.version have the same, dotted value. And it breaks a lot of legacy applications which parse java.specification.version as a float number. That code would still work if java.specification.version would be a concrete number (e.g. '9' or '10'). I suggest to set VERSION_SPECIFICATION to VERSION_MAJOR in common/autoconf/spec.gmk.in. This should be the "right value" until we get a specification change during a major release which hasn't happened for quite some time now. Regards, Volker

4 15

Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
by David Holmes 24 Jun '16

24 Jun '16

Hi Kalyan, This is not a hotspot issue so I'm moving this to core-libs, please drop hotspot from any replies. On 20/12/2013 6:26 AM, srikalyan wrote: > Hi all, I have been working on the bug JDK-8022321 > <https://bugs.openjdk.java.net/browse/JDK-8022321> , this is a sporadic > failure and the webrev is available here > http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReference… I'm really not sure what to make of this. We have a test that triggers an out-of-memory condition but the OOME can actually turn up in the ReferenceHandler thread causing it to terminate and the test to fail. We previously accounted for the non-obvious occurrences of OOME due to the Object.wait and the possible need to load the InterruptedException class - but still the OOME can appear where we don't want it. So finally you have just placed the whole for(;;) loop in a try/catch(OOME) that ignores the OOME. I'm certain that makes the test happy, but I'm not sure it is really what we want for the ReferenceHandler thread. If the OOME occurs while cleaning, or enqueuing then we will fail to clean and/or enqueue but there would be no indication that has occurred and I think that is a bigger problem than this test failing. There may be no way to make this test 100% reliable. In fact I'd suggest that no memory exhaustion test can be 100% reliable. David > * > **"Root Cause:Still not known"* > 2 places where there is a possibility for OOME > 1) Cleaner.clean() > 2) ReferenceQueue.enqueue() > > 1) The cleanup code in turn has 2 places where there is potential for > throwing OOME, > a) thunk Thread which is run from clean() method. This Runnable is > passed to Cleaner and appears in the following classes > java/nio/DirectByteBuffer.java > sun/misc/Perf.java > sun/nio/fs/NativeBuffer.java > sun/nio/ch/IOVecWrapper.java > sun/misc/Cleaner/ExitOnThrow.java > However none of the above overridden implementations ever create an > object in the clean() code. > b) new PrivilegedAction created in try catch Exception block of > clean() method but for this object to be created and to be held > responsible for OOME an Exception(other than OOME) has to be thrown. > > 2) No new heap objects are created in the enqueue method nor anywhere in > the deep call stack (VM.addFinalRefCount() etc) so this cannot be a > potential cause. > > *Experimental change to java.lang.Reference.java* : > - Put one more guard (try catch with OOME block) in the Reference > Handler Thread which may give the Reference Handler a chance to cleanup. > This is fixing the test failure (several 1000 runs with 0 failures) > - Without the above change the test fails atleast 3-5 times for every > 1000 run. > > *PS*: The code change is to a very critical part of JDK and i am fully > not aware of the consequences of the change, hence seeking expert help > here. Appreciate your time and inputs towards this. >

10 95

Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries
by KUBOTA Yuji 01 Jun '16

01 Jun '16

2016-04-20 7:57 GMT+09:00 <mark.reinhold(a)oracle.com>: > I have to agree. I don't think it makes sense to add a known-vulnerable > encryption algorithm to the JDK. It might work perfectly well for one > use case but it will eventually be used by someone who doesn't take the > time to understand it, assumes that it provides strong encryption when > it doesn't, gets burned, and then blames Java. Yes, Mark. We never hope to make OpenJDK be blamed with our proposal. Mark, I am glad if you are interested in this proposal if using any strong encryption algorithm like Sherman's implementation (AES encryption). If so, we hope to continue discussion the better way to fill our needs and given comments for improving OpenJDK. Thanks, Yuji

3 4

Files.walk() is unusable because of AccessDeniedException
by Gilles Habran 26 May '16

26 May '16

Good morning, I am trying to use Files.walk() in a Java 8 way but it is broken. I am trying to create an app to manage files and size on all my Linux system (so I would walk from the root directory and show the 5 biggest files of each directory for example). *JDK-8039910* has been raised for this issue but it is closed (2014-11-22). Alan Bateman wrote that the method is working as intended but I don't see how this could be intended because at the moment, it is unusable in a Java 8 way and for me. When we use a terminal operations on the Stream returned by Files.walk(), we get a crash because of AccessDeniedException For example, here is the directory (rules.d) that makes everything crash : [9:51:20 - ghabran@arch:/tmp/chromium-pepper-flash]$ ll /usr/share/polkit-1 total 8.0K drwxr-xr-x 2 root root 4.0K Apr 14 09:18 actions *drwxr-x--- 2 root polkitd 4.0K Apr 14 09:18 rules.d* As you can see, as a simple user, I don't have access. It is not possible to apply a filter to check the attributes to manage the file/directory to see if we have the rights because an exception is thrown before we get to check the attributes. Here are several scenarios : 1) This doesn't crash (there seems to have no exception thrown). I get error code 0, no exception is printed on my terminal. try { Stream<Path> directoryTree = Files.walk(*Paths.get("/")*); directoryTree .filter(p -> { try { return Files.isRegularFile(p) && Files.isReadable(p); } catch (Exception e) { return false; } }); } catch (IOException ioe) { System.out.println(ioe); } 2) this raises the exception AccessDeniedException (this goes in the catch) try { Stream<Path> directoryTree = Files.walk( *Paths.get("/usr/share/polkit-1/rules.d")*); directoryTree .filter(p -> { try { return Files.isRegularFile(p) && Files.isReadable(p); } catch (Exception e) { return false; } }); *} catch (IOException ioe) {* * System.out.println(ioe);* *}* 3) this crashes with a stack trace : AccessDeniedException *(this doesn't go in the catch)* try { Stream<Path> directoryTree = Files.walk(*Paths.get("/")*); directoryTree .filter(p -> { try { return Files.isRegularFile(p) && Files.isReadable(p); } catch (Exception e) { return false; } })*.forEach(System.out::println);* } catch (IOException ioe) { System.out.println(ioe); } 4) this crashes but goes into the catch try { Stream<Path> directoryTree = Files.walk( *Paths.get("/usr/share/polkit-1/rules.d")*); directoryTree .filter(p -> { try { return Files.isRegularFile(p) && Files.isReadable(p); } catch (Exception e) { return false; } })*.forEach(System.out::println)*; *} catch (IOException ioe) {* * System.out.println(ioe);* *}* In the meantime, I will use the old way with walkFileTree and a FileVisitor. How am I supposed to walk() a directory tree in Java 8 without walk() ? I was looking forward to using Stream<Path> to do it but I can't. What should I do ? Thank you. Regards, Gilles

7 10

[9] RFR of 5100935: No way to access the 64-bit integer multiplication of 64-bit CPUs efficiently
by Brian Burkhalter 20 May '16

20 May '16

Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-5100935 Patch: http://cr.openjdk.java.net/~bpb/5100935/webrev.00/ Summary: Add multiplyFull() and multiplyHigh() methods to java.lang.Math. This change addresses the Java specification and implementation only. The addition of compiler intrinsics for these methods will be tracked separately. Thanks, Brian

4 8

[9] RFR of 8023217: Additional floorDiv/floorMod/multiplyExact methods for java.lang.Math
by Brian Burkhalter 20 May '16

20 May '16

Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8023217 Patch: http://cr.openjdk.java.net/~bpb/8023217/webrev.00/ Summary: Add some additional methods of the same name as existing methods but with the commonly used parameter signature “long,int”: long multiplyExact(long,int) long floorDiv(long,int) int floorDiv(long,int) These methods also provide hooks for potential intrinsics. There might be room for improvement in their Java implementations. The modifications to the java.time classes are to fix warnings about “redundant cast to int.” Thanks, Brian

4 9

RFR 8029891 : Deadlock detected in java/lang/ClassLoader/deadlock/GetResource.java
by Brent Christian 18 May '16

18 May '16

Hi, Please review this fix, courtesy of Peter Levart (thanks!), that I would like to get in. https://bugs.openjdk.java.net/browse/JDK-8029891 http://cr.openjdk.java.net/~bchristi/8029891/webrev.0/ There is some discussion of it in the bug report, starting at 2014-12-31. The problem, as stated by Mandy: "System Properties is a hashtable that synchronizes on itself for any access. Currently System.getProperties returns the Properties instance accessed by the system in which any application code might synchronize on it (that's what the test is doing). The problem reported JDK-6977738 is about Properties.store method that was fixed not to synchronize on this instance. System property is a common way for changing the default setting and so it's impractical to expect the class loading code path not to call System.getProperty." This fix changes java.util.Properties to store its values in an internal ConcurrentHashMap, ignoring its Hashtable heritage. In this way, Properties can be "de-sychronized": all methods inherited from Hashtable are overridden, to remove synchronization, and delegate to the internal CHM. The serialized form is unchanged. An alternative approach considered would be for System.getProperties() to return a duplicate snapshot of the current Properties. This presents a compatibility risk to existing code that keeps a reference to the return value of System.getProperties() and expects to either read new properties added afterwards, or set properties on the cached copy. -Brent

7 41

We don't need jdk.internal.ref.Cleaner any more
by Peter Levart 16 May '16

16 May '16

Hi, sun.misc.Cleaner has been moved to internal package jdk.internal.ref recently [1] to clean-up sun.misc namespace. But now that: - we have comparable public API (java.lang.ref.Cleaner & Cleanable) [2] - we have an internal shared java.lang.ref.Cleaner instance (jdk.internal.ref.CleanerFactory.cleaner()) - sun.misc.Cleaner is not a special kind of Reference any more in the JVM [3] ...I think there's no reason to keep this special internal API any more. It can be replaced with public API. I propose to remove jdk.internal.ref.Cleaner class and replace its usages with java.lang.ref.Cleaner and friends [4]. What do you say? Regards, Peter [1] https://bugs.openjdk.java.net/browse/JDK-8148117 [2] https://bugs.openjdk.java.net/browse/JDK-8138696 [3] https://bugs.openjdk.java.net/browse/JDK-8143847 [4] http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.0…

11 72

Re: Fwd: RFR(m): 8140281 deprecate Optional.get()
by Jonathan Gibbons 13 May '16

13 May '16

Since the justification for this change appears to be that the IDEs might help write people write bad code, why not look to the IDEs to generate a warning when using Optional.get outside of the context of Optional.isPresent? In other words, is this really such a good change? -- Jon On 04/25/2016 04:14 PM, Stuart Marks wrote: > One of the changes in the langtools webrev > > http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/ > > is in the compiler. Please review. > > Thanks, > > s'marks > > > -------- Forwarded Message -------- > Subject: RFR(m): 8140281 deprecate Optional.get() > Date: Mon, 25 Apr 2016 16:05:13 -0700 > From: Stuart Marks <stuart.marks(a)oracle.com> > To: core-libs-dev <core-libs-dev(a)openjdk.java.net> > > Hi all, > > Please review these webrevs that deprecate Optional.get() and to > replace it with Optional.getWhenPresent(). The corresponding changes > are also applied to OptionalDouble.getAsDouble(), > OptionalInt.getAsInt(), and OptionalLong.getAsLong(). > > Unlike most deprecations, this isn't about the function or the utility > of some API, it's about the name. The solution is basically to rename > the API. The problem is that "get" shows up as the "obvious" choice in > things like IDE code completion, leading to code that mishandles empty > Optionals. Typical Stack Overflow discourse runs something like this: > > Q: what do I do with this Optional thing > > A: just call get() > > Q: thanks, it works! > > Of course, it works until it doesn't. > > Examining the JDK's use of Optional.get(), I didn't see very many > cases that called get() without first checking for the presence of a > value. But I did see quite a number of cases like this: > > if (opt.isPresent()) { > doSomething(opt.get()); > } else { > doSomethingElse(); > } > > In many of these cases, the code could be refactored to use other > Optional methods such as filter(), map(), or ifPresent(). > > In any case this reinforces the contention that use of get() leads to > poor code. > > For this changeset, in just about all cases I've simply replaced the > call to get() with a call to getWhenPresent(). In a couple cases I > replaced the stream calls > > .filter(Optional::isPresent).map(Optional::get) > > with > > .flatMap(Optional::stream) > > which I hope will become the new idiom for unwrapping a stream of > Optionals. > > While many cases could be cleaned up further, I didn't change them. > The reasons are that I didn't want to spend too much time putting code > cleanup into the critical path of this changeset (I'd be happy to help > later); doing so would create potential conflicts with code coming in > from the Jigsaw forest; and there are non-obvious places where > converting from a conditional to one of the lambda-based methods could > cause performance problems at startup. > > There are also a few cases where simplification is prevented because > it would end up causing the resulting lambda expressions to throw > checked exceptions. :-( > > Webrevs here: > > http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/ > > http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/ > > Thanks, > > s'marks

6 17

RFR [9] 8153737: Unsupported Module
by Chris Hegarty 11 May '16

11 May '16

Enough technical debt has been paid down that we can now create the new JDK-specific module as proposed by JEP 260 [1], named jdk.unsupported. This module will initially contain, and export, the sun.misc package, and will eventually export the sun.reflect package too ( once it has been sanitized ). The jdk.unsupported module will be present in full JRE and JDK images. http://cr.openjdk.java.net/~chegar/8153737/ https://bugs.openjdk.java.net/browse/JDK-8153737 Summary of the changes: - Restructure, what remains of, the sources in sun/misc to reflect the new modular layout. There are still a few non-Critical APIs in sun.misc that will be addressed later. - Add dependencies from several JDK modules to jdk.unsupported. All dependencies have issues tracking them, and should be removed in 9. - The test library in the top-level repo provides a convenience to tests to retrieve Unsafe. This has been updated to the "real" internal Unsafe, so there are many many tests that require to be updated, mostly the @modules tags. ( The Unsafe dependency in the library should probably be revisited at some point, as it requires many tests, not wishing to use Unsafe, to require an explicit dependency on it ) Note: with recent work from Mikael [2], sun.misc.Unsafe is now just a delegate for the "real" internal Unsafe, so it is time to move the tests to use it directly. - Cleanup any tests unnecessarily/incorrectly declaring a dependency on sun.misc ( this could have been unnecessarily added, been refactored out at one point, or the library upon which the tests depends refactored, that the dependency is no longer required ). I'm sure further cleanup could be done, but this is sufficient for now. - A small number of test in hotspot/test/compiler/unsafe, which are generated, have been updated, along with the generation script, to depend on jdk.unsupported/sun.misc ( they explicitly test the sun.misc version ) All 'core', 'pit', and 'hotspot' testsets have been successfully run on Mac, Linux, Windows, and Solaris. -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8132928 [2] https://bugs.openjdk.java.net/browse/JDK-8149159

6 15