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
