Re: [PATCH] JDK-8054565: FilterOutputStream.close may throw IOException if called twice and underlying flush or close fails
To close the loop; find below the complete reviewed changes. http://cr.openjdk.java.net/~chegar/8054565/webrev.00/webrev/ Since there is a minor spec change, I will submit a CCC to cover it. Unless any unforeseen issues arise from the CCC, you can assume that this is final. -Chris.
JDK 9 RFR of 8067669: Documentation for methods in Number incomplete regarding too large values.
Please review at your convenience:
Issue: https://bugs.openjdk.java.net/browse/JDK-8067669
Patch: http://cr.openjdk.java.net/~bpb/8067669/webrev.00/
Summary:
Add some verbiage.
I would advocate either changing the verbiage either to the content in the
patch or something better, or resolving the issue as “Not an Issue.”
Thanks,
Brian
On Dec 19, 2014, at 3:34 PM, Brian Burkhalter brian.burkhal...@oracle.com
wrote:
On Dec 19, 2014, at 3:26 PM, joe darcy joe.da...@oracle.com wrote:
I don't really think the current text is problematic; however, if it is to
be changed, I recommending including a citation to the Narrowing primitive
conversion section of the Java Language Specification. This can be
accomplished using the @jls javadoc tag; the syntax is something like
* @jls 5.1.3. Narrowing Primitive Conversion
Examples of the use of @jls can be found elsewhere in the core libraries.
This is already in the class level documentation which is why I left it out.
No harm adding it to the method docs however.
For the exact wording how about
... The particular semantics of the conversion operation from the specified
number to a {@code long} are defined in subclasses. The operation may
involve a narrowing conversion, rounding, or truncation.
Seems reasonable to me.
[9] RFR (M): 8067344: Adjust java/lang/invoke/LFCaching/LFGarbageCollectedTest.java for recent changes in java.lang.invoke
http://cr.openjdk.java.net/~vlivanov/8067344/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8067344 LFGarbageCollectedTest should be adjusted after JDK-8057020. There are a couple of problems with the test. (1) Existing logic to test that LambdaForm instance is collected isn't stable enough. Consequent System.GCs can hinder reference enqueueing. To speed up the test, I added -XX:SoftRefLRUPolicyMSPerMB=0 and limited the heap by -Xmx64m. (2) MethodType-based invoker caches are deliberately left strongly reachable. So, they should be skipped in the test. (3) Added additional diagnostic output to simplify failure analysis (test case details, method handle type and LambdaForm, heap dump (optional, -DHEAP_DUMP=true)). Testing: failing test. Thanks! Best regards, Vladimir Ivanov
JDK 9 RFR of 8064463: BigDecimal should populate NumberFormatException message
Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8064463 Patch: http://cr.openjdk.java.net/~bpb/8064463/webrev.00/ Summary: Ensure that the message strings of all NumberFormatExceptions are populated with appropriate content, excluding where the NFE merely wraps another exception in its cause. It was suggested in the issue description to include the contents of the supplied character array in the exception message. This is not done in this patch for the reason that the resulting string could be quite large. If it seems reasonable to include the characters the patch can be so amended. Thanks, Brian
Re: RFR: JDK-8047769 SecureRandom should be more frugal with file descriptors
Hi Peter,
Looks like you have a committer status, will you be pushing this?
On 12/18/2014 5:23 AM, Peter Levart wrote:
Hi,
I propose a patch for the following issue:
https://bugs.openjdk.java.net/browse/JDK-8047769
Here's the webrev:
http://cr.openjdk.java.net/~plevart/jdk9-dev/FileInputStreamPool.8047769/webrev.01/
Looks good. A few minor comments.
In a couple places, there are a few lines 80 chars. (It's a pet peeve
of mine, this makes side-by-side reviews difficult without a wide
monitor. I realize not everyone shares this view, but they're probably
not working on a laptop screen regularly.)
Do you need to close the InputStream when last holder is GC'd, or do we
just let the FileInputStream's finalizer take care of that?
Both of these current calls are contained within a
AccessContrller.doPriviledged, the checkRead() seems redundant.
In your test case, if assertions are not enabled, the tests at 46/50/51
are noops, e.g. when run by hand. Generally should directly throw
Exceptions.
The patch uses a package-private FileInputStreamPool class that caches
open FileInputStream(s) so that at most one file descriptor is open for
a particular file. Reading from shared unbuffered FileInputStream in
multiple threads should be safe, right?
I would think (hope?) so, but I am not 100% sure. I tracked it down
into libjava native code:
io_util.c
=
fd = GET_FD(this, fid);
if (fd == -1) {
JNU_ThrowIOException(env, Stream Closed);
nread = -1;
} else {
nread = IO_Read(fd, buf, len);
which is then defined to handleRead, which calls something called
RESTARTABLE in io_util_md.c. Assuming I'm looking at the right code.
Core-libs folks?
sun/security/provider/PolicyFile/GrantAllPermToExtWhenNoPolicy.java:
Make sure that when no system policy and user policy files exist, the
built-in default policy will be used, which - amongst other things -
grants standard extensions the AllPermission.
sun/security/provider/PolicyParser/ExtDirsChange.java: standard
extensions path is hard-coded in default system policy file
sun/security/provider/PolicyParser/PrincipalExpansionError.java: parser
incorrectly ignores a principal if the principal name expands to nothing.
...they are unrelated to this patch - seems to be caused by recent
layout changes for modular runtime images.
Hopefully you saw my previous response. Repeating:
---begin---
They've been failing for a while:
https://bugs.openjdk.java.net/browse/JDK-8039280
These tests are all /manual so they don't show up in our regular runs
of JPRT/jtreg, which include -a.
-a | -automatic | -automagic
Any test with /manual will not be run
---end---
Thanks,
Brad
