Re: Zombie FileHandler locks can exhaust all available log file locks.
Hi Jason, Looking at the diff for 6244047 - I see that, as you pointed out, the unwanted behavior described comes from the fact that the new code is using CREATE_NEW - which prevents the 'zombie lock files' from being reused. I am not an expert in file locks - but I wonder whether we could revert to using CREATE instead: wouldn't tryLock() later tell us if the file is used by another entity? Another possibility would be to combine CREATE_NEW and DELETE_ON_CLOSE, which according to StandardOpenOptions will attempt to delete the file on JVM termination if close() hasn't been called. This probably wouldn't help in case on VM crash, but it would help in the case demonstrated by your test below. I have however some reluctance because I see that we call FileChannel.close() in the case were the lock can't be obtained, and I'm not sure what that would do... Also StandardOpenOptions has some warning about using DELETE_ON_CLOSE for files that can be opened concurrently by other entities - so I'm not sure it would be appropriate. The last possibility I see would be to change the lock HashMap to take instances of a subclass of WeakReferenceFileHandler as values (instead of String), and add some code that attempts to close remove the lock file when the FileHandler is no longer referenced. Again - this will probably not help in the case of crash, and also adds the question on when the weak reference queue should be polled to ensure that the no longer referenced FileChannel are closed in a timely fashion. All in all - I feel our best options would be to revert to using CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything and live with the issue. Hopefully some nio experts will chime in ;-) On another track - we could also make MAX configurable - but that would just be shifting the issue around - wouldn't it? best regards, -- daniel On 6/20/14 10:54 PM, Jason Mehrens wrote: Daniel, Jim, In JDK8 the FileHandler file locking was changed to use FileChannel.open with CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due to safety concerns about writing to the same log file. The FileHandler has an upper bound of 100 as the number of file locks that can be attempted to be acquired. Given the right pattern, enough time, and enough failures it seems at it is possible for a program to end up in a state where the FileHandler is surrounded by zombie lock files refuses log or perform a clean up action. (A.K.A Farmer Rick Grimes). This means that the lck files have to manually be deleted or the FileHandler will just fail with an IOException every time it is created. A simple test to emulate crashing is to run (JDK7 vs. JDK8) the following code twice without deleting the log and lck files: public static void main(String[] args) throws Exception { System.out.println(System.getProperty(java.runtime.version)); ReferenceQueueFileHandler q = new ReferenceQueue(); for (int i=0; i100; i++) { WeakReferenceFileHandler h = new WeakReference( new FileHandler(./test_%u.log, 1, 2, true), q); while (q.poll() != h) { System.runFinalization(); System.gc(); System.runFinalization(); Thread.yield(); } } } I understand that if you can't trust that the file locking always works then there isn't much that can be done. Leaving the number of locks as unbounded isn't really an option either. Seems like there should be a way to identify zombie lock files (ATOMIC_MOVE) and delete them. Any thoughts on this? The source discussion on this can be found at http://stackoverflow.com/questions/24321098/is-java-util-logging-filehandler-in-java-8-broken Regards, Jason
RE: Zombie FileHandler locks can exhaust all available log file locks.
Daniel, My understanding is that changing CREATE_NEW to use CREATE would make it work like does in JDK7. Closing the lock files when the FileHandler is unreferenced I is probably the fix for JDK-6774110: lock file is not deleted when child logger is used. If we could prove that system FileLock is mandatory we could use the JDK7 behavior otherwise if they are advisory then use the JDK8 behavior. If we knew the boot time of the JVM host O/S we could delete all lock files older than the boot time + some constant. Jason Date: Mon, 23 Jun 2014 11:48:18 +0200 From: daniel.fu...@oracle.com To: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net Subject: Re: Zombie FileHandler locks can exhaust all available log file locks. Hi Jason, Looking at the diff for 6244047 - I see that, as you pointed out, the unwanted behavior described comes from the fact that the new code is using CREATE_NEW - which prevents the 'zombie lock files' from being reused. I am not an expert in file locks - but I wonder whether we could revert to using CREATE instead: wouldn't tryLock() later tell us if the file is used by another entity? Another possibility would be to combine CREATE_NEW and DELETE_ON_CLOSE, which according to StandardOpenOptions will attempt to delete the file on JVM termination if close() hasn't been called. This probably wouldn't help in case on VM crash, but it would help in the case demonstrated by your test below. I have however some reluctance because I see that we call FileChannel.close() in the case were the lock can't be obtained, and I'm not sure what that would do... Also StandardOpenOptions has some warning about using DELETE_ON_CLOSE for files that can be opened concurrently by other entities - so I'm not sure it would be appropriate. The last possibility I see would be to change the lock HashMap to take instances of a subclass of WeakReferenceFileHandler as values (instead of String), and add some code that attempts to close remove the lock file when the FileHandler is no longer referenced. Again - this will probably not help in the case of crash, and also adds the question on when the weak reference queue should be polled to ensure that the no longer referenced FileChannel are closed in a timely fashion. All in all - I feel our best options would be to revert to using CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything and live with the issue. Hopefully some nio experts will chime in ;-) On another track - we could also make MAX configurable - but that would just be shifting the issue around - wouldn't it? best regards, -- daniel On 6/20/14 10:54 PM, Jason Mehrens wrote: Daniel, Jim, In JDK8 the FileHandler file locking was changed to use FileChannel.open with CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due to safety concerns about writing to the same log file. The FileHandler has an upper bound of 100 as the number of file locks that can be attempted to be acquired. Given the right pattern, enough time, and enough failures it seems at it is possible for a program to end up in a state where the FileHandler is surrounded by zombie lock files refuses log or perform a clean up action. (A.K.A Farmer Rick Grimes). This means that the lck files have to manually be deleted or the FileHandler will just fail with an IOException every time it is created. A simple test to emulate crashing is to run (JDK7 vs. JDK8) the following code twice without deleting the log and lck files: public static void main(String[] args) throws Exception { System.out.println(System.getProperty(java.runtime.version)); ReferenceQueueFileHandler q = new ReferenceQueue(); for (int i=0; i100; i++) { WeakReferenceFileHandler h = new WeakReference( new FileHandler(./test_%u.log, 1, 2, true), q); while (q.poll() != h) { System.runFinalization(); System.gc(); System.runFinalization(); Thread.yield(); } } } I understand that if you can't trust that the file locking always works then there isn't much that can be done. Leaving the number of locks as unbounded isn't really an option either. Seems like there should be a way to identify zombie lock files (ATOMIC_MOVE) and delete them. Any thoughts on this? The source discussion on this can be found at http://stackoverflow.com/questions/24321098/is-java-util-logging-filehandler-in-java-8-broken Regards, Jason
Re: Zombie FileHandler locks can exhaust all available log file locks.
Hi Jason, On 6/23/14 2:51 PM, Jason Mehrens wrote: Daniel, My understanding is that changing CREATE_NEW to use CREATE would make it work like does in JDK7. Closing the lock files when the FileHandler is unreferenced I is probably the fix for JDK-6774110: lock file is not deleted when child logger is used. I'll see if I can dig more info about the reason for the change. Thanks for pointing at JDK-6774110 - I'll have a look at that as well. -- daniel If we could prove that system FileLock is mandatory we could use the JDK7 behavior otherwise if they are advisory then use the JDK8 behavior. If we knew the boot time of the JVM host O/S we could delete all lock files older than the boot time + some constant. Jason Date: Mon, 23 Jun 2014 11:48:18 +0200 From: daniel.fu...@oracle.com To: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net Subject: Re: Zombie FileHandler locks can exhaust all available log file locks. Hi Jason, Looking at the diff for 6244047 - I see that, as you pointed out, the unwanted behavior described comes from the fact that the new code is using CREATE_NEW - which prevents the 'zombie lock files' from being reused. I am not an expert in file locks - but I wonder whether we could revert to using CREATE instead: wouldn't tryLock() later tell us if the file is used by another entity? Another possibility would be to combine CREATE_NEW and DELETE_ON_CLOSE, which according to StandardOpenOptions will attempt to delete the file on JVM termination if close() hasn't been called. This probably wouldn't help in case on VM crash, but it would help in the case demonstrated by your test below. I have however some reluctance because I see that we call FileChannel.close() in the case were the lock can't be obtained, and I'm not sure what that would do... Also StandardOpenOptions has some warning about using DELETE_ON_CLOSE for files that can be opened concurrently by other entities - so I'm not sure it would be appropriate. The last possibility I see would be to change the lock HashMap to take instances of a subclass of WeakReferenceFileHandler as values (instead of String), and add some code that attempts to close remove the lock file when the FileHandler is no longer referenced. Again - this will probably not help in the case of crash, and also adds the question on when the weak reference queue should be polled to ensure that the no longer referenced FileChannel are closed in a timely fashion. All in all - I feel our best options would be to revert to using CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything and live with the issue. Hopefully some nio experts will chime in ;-) On another track - we could also make MAX configurable - but that would just be shifting the issue around - wouldn't it? best regards, -- daniel On 6/20/14 10:54 PM, Jason Mehrens wrote: Daniel, Jim, In JDK8 the FileHandler file locking was changed to use FileChannel.open with CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due to safety concerns about writing to the same log file. The FileHandler has an upper bound of 100 as the number of file locks that can be attempted to be acquired. Given the right pattern, enough time, and enough failures it seems at it is possible for a program to end up in a state where the FileHandler is surrounded by zombie lock files refuses log or perform a clean up action. (A.K.A Farmer Rick Grimes). This means that the lck files have to manually be deleted or the FileHandler will just fail with an IOException every time it is created. A simple test to emulate crashing is to run (JDK7 vs. JDK8) the following code twice without deleting the log and lck files: public static void main(String[] args) throws Exception { System.out.println(System.getProperty(java.runtime.version)); ReferenceQueueFileHandler q = new ReferenceQueue(); for (int i=0; i100; i++) { WeakReferenceFileHandler h = new WeakReference( new FileHandler(./test_%u.log, 1, 2, true), q); while (q.poll() != h) { System.runFinalization(); System.gc(); System.runFinalization(); Thread.yield(); } } } I understand that if you can't trust that the file locking always works then there isn't much that can be done. Leaving the number of locks as unbounded isn't really an option either. Seems like there should be a way to identify zombie lock files (ATOMIC_MOVE) and delete them. Any thoughts on this? The source discussion on this can be found at http://stackoverflow.com/questions/24321098/is-java-util-logging-filehandler-in-java-8-broken Regards, Jason
Re: Zombie FileHandler locks can exhaust all available log file locks.
Hi, I wonder whether the following patch would solve the issue. In the case where the lock file already exists, we check whether it's writable, and whether its directory is writable, and if so, we open it for 'write+append', otherwise, we continue with the next lock name. I we have manage to open the lock file, then we proceed with trying to lock it, as before. If locking succeeds - we use that file. If locking throws exception - we only fall through if we actually created the file, otherwise we proceed with the next lock name. Would that make sense? It passes the CheckLockLocationTest that was added for 6244047. Note: I also added a deleteOnExit() hook for good measure. -- daniel --- a/src/share/classes/java/util/logging/FileHandler.java +++ b/src/share/classes/java/util/logging/FileHandler.java @@ -25,6 +25,7 @@ package java.util.logging; +import static java.nio.file.StandardOpenOption.APPEND; import static java.nio.file.StandardOpenOption.CREATE_NEW; import static java.nio.file.StandardOpenOption.WRITE; @@ -35,6 +36,8 @@ import java.io.OutputStream; import java.nio.channels.FileChannel; import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.security.AccessController; import java.security.PrivilegedAction; @@ -434,24 +437,44 @@ continue; } +final Path lockFilePath = Paths.get(lockFileName); +boolean fileCreated; try { -lockFileChannel = FileChannel.open(Paths.get(lockFileName), +lockFileChannel = FileChannel.open(lockFilePath, CREATE_NEW, WRITE); +fileCreated = true; } catch (FileAlreadyExistsException ix) { +// This may be a zombie file left over by a previous +// execution. Reuse it - but since we didn't create +// it we won't delete it either. +if (Files.isRegularFile(lockFilePath) + Files.isWritable(lockFilePath) + Files.isWritable(lockFilePath.getParent())) { +lockFileChannel = FileChannel.open(lockFilePath, +WRITE, APPEND); +fileCreated = false; +} else { // try the next lock file name in the sequence continue; } +} boolean available; try { available = lockFileChannel.tryLock() != null; +if (available) { +// we got the lock - ensure that the lock file +// will be deleted on exit. +lockFilePath.toFile().deleteOnExit(); +} // We got the lock OK. } catch (IOException ix) { // We got an IOException while trying to get the lock. // This normally indicates that locking is not supported // on the target directory. We have to proceed without -// getting a lock. Drop through. -available = true; +// getting a lock. Drop through, but only if we did +// create the file... +available = fileCreated; } if (available) { // We got the lock. Remember it. On 6/23/14 3:18 PM, Daniel Fuchs wrote: Hi Jason, On 6/23/14 2:51 PM, Jason Mehrens wrote: Daniel, My understanding is that changing CREATE_NEW to use CREATE would make it work like does in JDK7. Closing the lock files when the FileHandler is unreferenced I is probably the fix for JDK-6774110: lock file is not deleted when child logger is used. I'll see if I can dig more info about the reason for the change. Thanks for pointing at JDK-6774110 - I'll have a look at that as well. -- daniel If we could prove that system FileLock is mandatory we could use the JDK7 behavior otherwise if they are advisory then use the JDK8 behavior. If we knew the boot time of the JVM host O/S we could delete all lock files older than the boot time + some constant. Jason Date: Mon, 23 Jun 2014 11:48:18 +0200 From: daniel.fu...@oracle.com To: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net Subject: Re: Zombie FileHandler locks can exhaust all available log file locks. Hi Jason, Looking at the diff for 6244047 - I see that, as you pointed out, the unwanted behavior described comes from the fact that the new code is using CREATE_NEW - which prevents the 'zombie lock files' from being reused. I am not an expert in file locks - but I wonder whether we could revert to using CREATE
Re: Zombie FileHandler locks can exhaust all available log file locks.
On 23/06/2014 10:48, Daniel Fuchs wrote: : All in all - I feel our best options would be to revert to using CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything and live with the issue. Hopefully some nio experts will chime in ;-) The logging use of FileLock is a bit unusual but looking at it now then I don't see the reason why it needs to use CREATE_NEW. I don't think DELETE_ON_CLOSE is suitable here as that work is for transient work files which isn't the case here. Instead it could use deleteOnExit to have some chance of removing it on a graceful exit. BTW: Do you know why locks is Map? Would a Set do? -Alan.
Re: Zombie FileHandler locks can exhaust all available log file locks.
On 6/23/14 4:53 PM, Alan Bateman wrote: On 23/06/2014 10:48, Daniel Fuchs wrote: : All in all - I feel our best options would be to revert to using CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything and live with the issue. Hopefully some nio experts will chime in ;-) The logging use of FileLock is a bit unusual but looking at it now then I don't see the reason why it needs to use CREATE_NEW. OK - thanks - in the discussion that ended up in the patch where CREATE_NEW was introduced Jason (I think it was Jason) pointed at https://bugs.openjdk.java.net/browse/JDK-4420020 I'm guessing here that maybe it would be wrong to use an already existing lock file if you don't have the rights to write to its directory - and that using CREATE_NEW ensures that you have all necessary access rights all along the path. I don't think DELETE_ON_CLOSE is suitable here as that work is for transient work files which isn't the case here. Instead it could use deleteOnExit to have some chance of removing it on a graceful exit. Right - I suggested a patch in another mail where I just use that. BTW: Do you know why locks is Map? Would a Set do? It certainly looks as if we could use a simple HashSet here. Thanks Alan! -- daniel -Alan.
Re: RFR JDK-5077522 : Duration.compare incorrect for some values
Thanks again Daniel and Lance! Joe On 6/21/2014 3:32 AM, Lance @ Oracle wrote: Agree this is better and cleaner! http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 tel:+1.781.442.2037 Oracle Java Engineering 1 Network Drive x-apple-data-detectors://34/0 Burlington, MA 01803 x-apple-data-detectors://34/0 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Sent from my iPad On Jun 21, 2014, at 4:27 AM, Daniel Fuchs daniel.fu...@oracle.com mailto:daniel.fu...@oracle.com wrote: Thanks Joe! This is much cleaner indeed :-) -- daniel On 6/21/14 4:36 AM, huizhe wang wrote: Thanks Daniel, Lance. On 6/20/2014 3:02 AM, Daniel Fuchs wrote: Hi Joe, Thanks for the detailed explanation. It really helps reviewing the fix! Glad to know it helps. I thought this part of spec could be unfamiliar to people. This looks reasonable to me. One minor nit is that you could turn: 769 BigInteger maxintAsBigInteger = BigInteger.valueOf((long) Integer.MAX_VALUE); Good catch! I was going to do so but then forgot. I also refactored the check method so that the checks can be done in one loop: 24 lines of code instead of the original 170. I feel good :-) The other changes are purely clean-up and in one case, JavaDoc. http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ http://cr.openjdk.java.net/%7Ejoehw/jdk9/5077522/webrev/ Best regards, Joe into a static final constant in the class. best regards, -- daniel On 6/17/14 9:19 PM, huizhe wang wrote: Hi, This is a long time compatibility issue: Duration.compare returns equal for INDETERMINATE relations defined in XML Schema standard (http://www.w3.org/TR/xmlschema-2/#duration-order) as listed in the following table: Relation P*1Y* P*364D* P*365D* P*366D* P*367D* P*1M* P*27D* P*28D* P*29D* P*30D* P*31D* P*32D* P*5M* P*149D* P*150D* P*151D* P*152D* P*153D* P*154D* The order-relation of two Duratoin values x and y is x y iff s+x s+y for each qualified datetime s listed below: * 1696-09-01T00:00:00Z * 1697-02-01T00:00:00Z * 1903-03-01T00:00:00Z * 1903-07-01T00:00:00Z The original implementation used Unix epoch, that is, 00:00:00 UTC on 1 January 1970, as s in the above calculation which violated the above specification. A patch during JDK 6 development added correct implementation of the spec, but it was unfortunately added after the original calculation using Epoch time. *The fix to the issue therefore is simply removing the calculation using Epoch time.* I also consolidated the tedious max field value checks into a method called checkMaxValue. *Patch:* http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ http://cr.openjdk.java.net/%7Ejoehw/jdk9/5077522/webrev/ Test: testCompareWithInderterminateRelation: this is a copy of the JCK test that tests INDETERMINATE relations. testVerifyOtherRelations: this is added to verify edge cases, e.g. +- 1 second to the original test cases. For example, to the original test: PT525600M is P365D P1Y, I added PT525599M59S, , P1Y, and PT527040M - P366D P1Y, PT527040M1S, , P1Y Below is the test result: Comparing P1Y and P365D: INDETERMINATE Comparing P1Y and P366D: INDETERMINATE Comparing P1M and P28D: INDETERMINATE Comparing P1M and P29D: INDETERMINATE Comparing P1M and P30D: INDETERMINATE Comparing P1M and P31D: INDETERMINATE Comparing P5M and P150D: INDETERMINATE Comparing P5M and P151D: INDETERMINATE Comparing P5M and P152D: INDETERMINATE Comparing P5M and P153D: INDETERMINATE Comparing PT2419200S and P1M: INDETERMINATE Comparing PT2678400S and P1M: INDETERMINATE Comparing PT31536000S and P1Y: INDETERMINATE Comparing PT31622400S and P1Y: INDETERMINATE Comparing PT525600M and P1Y: INDETERMINATE Comparing PT527040M and P1Y: INDETERMINATE Comparing PT8760H and P1Y: INDETERMINATE Comparing PT8784H and P1Y: INDETERMINATE Comparing P365D and P1Y: INDETERMINATE Comparing P1Y and P364D: expected: GREATER actual: GREATER Comparing P1Y and P367D: expected: LESSER actual: LESSER Comparing P1Y2D and P366D: expected: GREATER actual: GREATER Comparing P1M and P27D: expected: GREATER actual: GREATER Comparing P1M and P32D: expected: LESSER actual: LESSER Comparing P1M and P31DT1H: expected: LESSER actual: LESSER Comparing P5M and P149D: expected: GREATER actual: GREATER Comparing P5M and P154D: expected: LESSER actual: LESSER Comparing P5M and P153DT1H: expected: LESSER actual: LESSER Comparing PT2419199S and P1M: expected: LESSER actual: LESSER Comparing PT2678401S and P1M: expected: GREATER actual: GREATER Comparing PT31535999S and P1Y: expected: LESSER actual: LESSER Comparing PT31622401S and P1Y: expected: GREATER actual: GREATER Comparing PT525599M59S and P1Y: expected: LESSER actual: LESSER Comparing PT527040M1S and P1Y: expected: GREATER actual: GREATER Comparing PT8759H59M59S and P1Y: expected: LESSER actual:
Re: RFR: 8047721: @since should have JDK version
I would prefer that JCE1.2 be pulled out completely in the Cipher* classes. I will be sending you a separate note about JCE logistics. Thanks for doing this cleanup. Brad On 6/20/2014 11:46 AM, Henry Jen wrote: Hi, Please review a trivial webrev to add JDK version to @since in a format as Mark suggested[1]. http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/ [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html Appened is the diff as in the webrev. Cheers, Henry diff --git a/src/share/classes/java/lang/Package.java b/src/share/classes/java/lang/Package.java --- a/src/share/classes/java/lang/Package.java +++ b/src/share/classes/java/lang/Package.java @@ -107,6 +107,7 @@ * loader to be found. * * @see ClassLoader#definePackage + * @since 1.2 */ public class Package implements java.lang.reflect.AnnotatedElement { /** diff --git a/src/share/classes/javax/crypto/CipherInputStream.java b/src/share/classes/javax/crypto/CipherInputStream.java --- a/src/share/classes/javax/crypto/CipherInputStream.java +++ b/src/share/classes/javax/crypto/CipherInputStream.java @@ -170,7 +170,7 @@ * @return the next byte of data, or code-1/code if the end of the * stream is reached. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read() throws IOException { if (ostart = ofinish) { @@ -196,7 +196,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[]) throws IOException { return read(b, 0, b.length); @@ -217,7 +217,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[], int off, int len) throws IOException { if (ostart = ofinish) { @@ -254,7 +254,7 @@ * @param n the number of bytes to be skipped. * @return the actual number of bytes skipped. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public long skip(long n) throws IOException { int available = ofinish - ostart; @@ -277,7 +277,7 @@ * @return the number of bytes that can be read from this input stream * without blocking. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int available() throws IOException { return (ofinish - ostart); @@ -292,7 +292,7 @@ * stream. * * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void close() throws IOException { if (closed) { @@ -321,7 +321,7 @@ * codemark/code and codereset/code methods. * @see java.io.InputStream#mark(int) * @see java.io.InputStream#reset() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public boolean markSupported() { return false; diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java b/src/share/classes/javax/crypto/CipherOutputStream.java --- a/src/share/classes/javax/crypto/CipherOutputStream.java +++ b/src/share/classes/javax/crypto/CipherOutputStream.java @@ -114,7 +114,7 @@ * * @param b the codebyte/code. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(int b) throws IOException { ibuffer[0] = (byte) b; @@ -138,7 +138,7 @@ * @exception NullPointerException if codeb/code is null. * @exception IOException if an I/O error occurs. * @seejavax.crypto.CipherOutputStream#write(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[]) throws IOException { write(b, 0, b.length); @@ -152,7 +152,7 @@ * @param off the start offset in the data. * @param len the number of bytes to write. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[], int off, int len) throws IOException { obuffer = cipher.update(b, off, len); @@ -174,7 +174,7 @@ * the cipher's block size, no bytes will be written out. * * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void flush() throws IOException { if (obuffer != null) { @@ -198,7 +198,7 @@
Re: Ready for Review : 8042469 : Launcher changes for native memory tracking scalability enhancement
I've spun a new webrev based on the comments for webrev-03: http://cr.openjdk.java.net/~ntoda/8042469/webrev-04/ Changes are: 1) java.c a) add free(envName) line 1063 b) change from malloc() to JLI_MemAlloc() @ lines 1048 and 1056 Thanks -neil On 6/20/2014 4:45 PM, Kumar Srinivasan wrote: Neil, Generally looks good, yes JLI_* functions must be used, I missed that one. Are you going to post another iteration ? Kumar On 6/20/2014 4:27 PM, Neil Toda wrote: Thanks Joe. It would have checked for NULL for me. I'll use the JLI wrapper. -neil On 6/20/2014 4:04 PM, Joe Darcy wrote: Memory allocation in the launcher should use one of the JLI_MemAlloc wrappers, if possible. -Joe On 06/20/2014 09:50 AM, Neil Toda wrote: They should complain. Thanks Zhengyu. I'll make sure these are non-null. -neil On 6/20/2014 5:01 AM, Zhengyu Gu wrote: Neil, Thanks for quick implementation. java.c: Did not check return values of malloc(), I wonder if source code analyzers will complain. -Zhengyu On 6/19/2014 8:29 PM, Neil Toda wrote: Launcher support for modified Native Memory Tracking mechanism in JVM in JDK9. Webrev : http://cr.openjdk.java.net/~ntoda/8042469/webrev-03/ bug : https://bugs.openjdk.java.net/browse/JDK-8042469 CCC : http://ccc.us.oracle.com/8042469 Thanks. -neil
Re: ThreadLocalRandom clinit troubles
Martin, Thanks for filing. I was positive there was already a bug for this, but for the life of me I can't find it now. There's some other more minor cleanup that needs to take place, but seems like I've been in escalation/firefighting mode for more than a year now and it hasn't bubbled to the top. Brad On 6/21/2014 9:05 PM, Martin Buchholz wrote: While looking at NativePRNG, I filed https://bugs.openjdk.java.net/browse/JDK-8047769 SecureRandom should be more frugal with file descriptors If I run this java program on Linux public class SecureRandoms { public static void main(String[] args) throws Throwable { new java.security.SecureRandom(); } } it creates 6 file descriptors for /dev/random and /dev/urandom, as shown by: strace -q -ff -e open java SecureRandoms | grep /dev/ [pid 20769] open(/dev/random, O_RDONLY) = 5 [pid 20769] open(/dev/urandom, O_RDONLY) = 6 [pid 20769] open(/dev/random, O_RDONLY) = 7 [pid 20769] open(/dev/random, O_RDONLY) = 8 [pid 20769] open(/dev/urandom, O_RDONLY) = 9 [pid 20769] open(/dev/urandom, O_RDONLY) = 10 Looking at jdk/src/solaris/classes/sun/security/provider/NativePRNG.java it looks like 2 file descriptors are created for every variant of NativePRNG, whether or not they are ever used. Which is wasteful. In fact, you only ever need at most two file descriptors, one for /dev/random and one for /dev/urandom. Further, it would be nice if the file descriptors were closed when idle and lazily re-created. Especially /dev/random should typically be used at startup and never thereafter. On Fri, Jun 20, 2014 at 7:59 AM, Alan Bateman alan.bate...@oracle.com mailto:alan.bate...@oracle.com wrote: On 20/06/2014 15:02, Peter Levart wrote: And, as Martin pointed out, it seems to be used for tests that exercise particular responses from NameService API to test the behaviour of JDK classes. It would be a shame for those tests to go away. We've been talking about removing it for many years because it has been so troublesome. If we really need to having something for testing then I don't think it needs to be general purpose, we can get right of the lookup at least. -Alan.
JDK9 project: XML/JAXP Approachability / Ease of Use
Hi, We're planning on a jaxp project to address usability issues in the JAXP API. One of the complaints about the JAXP API is the number of lines of code that are needed to implement a simple task. Tasks that should take one or two lines often take ten or twelve lines instead. Consider the following example: File file = new File(FILEPATH + results.xml); DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setNamespaceAware(true); XPathFactory xf = XPathFactory.newInstance(); XPath xp = xf.newXPath(); xp.setNamespaceContext(new NamespaceContext() { @Override public String getNamespaceURI(String prefix) { return http://example.com/users/;; } @Override public String getPrefix(String namespaceURI) { return ns1; } @Override public Iterator getPrefixes(String namespaceURI) { ArrayList list = new ArrayList(); list.add(ns1); return list.iterator(); } }); try { DocumentBuilder builder = factory.newDocumentBuilder(); Document document = builder.parse(file); String s = (String) xp.evaluate( /ns1:Results/ns1:Row[ns1:USERID=2]/ns1:NAME[text()], document, XPathConstants.STRING); System.out.println(Company: + s); } catch (ParserConfigurationException e) { //creating DocumentBuilder } catch (SAXException ex) { //parsing } catch (IOException ex) { //parsing } catch (XPathExpressionException ex) { //xpath evaluation } The issues reflected in the above sample include: *1. Too many abstractions* As shown in the above sample, there are multiple layers of abstraction in the DOM and Xpath API: factory, builder, and document, XPathFactory and XPath. *2. Unnecessary Pluggability* The pluggability layer allows easily plugging in third party implementations. However, in many use cases where pluggability is not needed, it becomes the performance bottleneck for the applications. *3. Too many unrelated checked exceptions* There are four unrelated checked exceptions in the above example. None of them is recoverable. ParserConfigurationException actually reflects a design flaw in the DocumentBuilderFactory that allowed setting both a Schema and Schema Language. In practice, Exception is often used to avoid having to catch each of the checked exceptions. *4. Lack of integration* JAXP is an umbrella of several libraries. The sample code above demonstrated the lack of integration among them. First of all, A DOM Document and XPath have to be created separately. Secondly, as in the above case, the Document may be either namespace-aware or unaware, depending on the setting on the DOM Factory, which is unknown to XPath created later. This project may have two aspects: one to provide APIs to get straight to the objects such as DOM Document, and another to provide convenient methods for some common use cases. (*Note that, there is no intention to replace the existing API nor duplicate all of the features.) For the example above, it could potentially be done in a couple of lines (this is just an illustration on how existing APIs could be simplified and may not reflect what the API would look like): String company = XMLDocument.fromFile(FILEPATH + results.xml) .evalXPath(/Results/Row[USERID=2]/NAME[text()]); System.out.printf(Company: %s%n, company); We would love to hear from you. Any thoughts, concerns, experiences/complaints would be very welcome. Thanks, Joe
Review request for 8047904: Runtime.loadLibrary throws SecurityException when security manager is installed
The loadLibrary implementation is missing to wrap the call to File.getCanonicalPath method with doPrivileged block. Webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8047904/webrev.00/ thanks Mandy
Re: RFR: 8047721: @since should have JDK version
OK, I'll remove all @since JCE line, as the class already has @since 1.4 as Joe pointed out earlier. Uodated webrev at http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/ Cheers, Henry On 06/23/2014 10:04 AM, Bradford Wetmore wrote: I would prefer that JCE1.2 be pulled out completely in the Cipher* classes. I will be sending you a separate note about JCE logistics. Thanks for doing this cleanup. Brad On 6/20/2014 11:46 AM, Henry Jen wrote: Hi, Please review a trivial webrev to add JDK version to @since in a format as Mark suggested[1]. http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/ [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html Appened is the diff as in the webrev. Cheers, Henry diff --git a/src/share/classes/java/lang/Package.java b/src/share/classes/java/lang/Package.java --- a/src/share/classes/java/lang/Package.java +++ b/src/share/classes/java/lang/Package.java @@ -107,6 +107,7 @@ * loader to be found. * * @see ClassLoader#definePackage + * @since 1.2 */ public class Package implements java.lang.reflect.AnnotatedElement { /** diff --git a/src/share/classes/javax/crypto/CipherInputStream.java b/src/share/classes/javax/crypto/CipherInputStream.java --- a/src/share/classes/javax/crypto/CipherInputStream.java +++ b/src/share/classes/javax/crypto/CipherInputStream.java @@ -170,7 +170,7 @@ * @return the next byte of data, or code-1/code if the end of the * stream is reached. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read() throws IOException { if (ostart = ofinish) { @@ -196,7 +196,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[]) throws IOException { return read(b, 0, b.length); @@ -217,7 +217,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[], int off, int len) throws IOException { if (ostart = ofinish) { @@ -254,7 +254,7 @@ * @param n the number of bytes to be skipped. * @return the actual number of bytes skipped. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public long skip(long n) throws IOException { int available = ofinish - ostart; @@ -277,7 +277,7 @@ * @return the number of bytes that can be read from this input stream * without blocking. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int available() throws IOException { return (ofinish - ostart); @@ -292,7 +292,7 @@ * stream. * * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void close() throws IOException { if (closed) { @@ -321,7 +321,7 @@ * codemark/code and codereset/code methods. * @see java.io.InputStream#mark(int) * @see java.io.InputStream#reset() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public boolean markSupported() { return false; diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java b/src/share/classes/javax/crypto/CipherOutputStream.java --- a/src/share/classes/javax/crypto/CipherOutputStream.java +++ b/src/share/classes/javax/crypto/CipherOutputStream.java @@ -114,7 +114,7 @@ * * @param b the codebyte/code. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(int b) throws IOException { ibuffer[0] = (byte) b; @@ -138,7 +138,7 @@ * @exception NullPointerException if codeb/code is null. * @exception IOException if an I/O error occurs. * @seejavax.crypto.CipherOutputStream#write(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[]) throws IOException { write(b, 0, b.length); @@ -152,7 +152,7 @@ * @param off the start offset in the data. * @param len the number of bytes to write. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[], int off, int len) throws IOException { obuffer = cipher.update(b, off, len); @@ -174,7 +174,7 @@ * the cipher's block size, no bytes will be
Re: RFR: 8047721: @since should have JDK version
Looks good to me; thanks, -Joe On 06/23/2014 01:50 PM, Henry Jen wrote: OK, I'll remove all @since JCE line, as the class already has @since 1.4 as Joe pointed out earlier. Uodated webrev at http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/ Cheers, Henry On 06/23/2014 10:04 AM, Bradford Wetmore wrote: I would prefer that JCE1.2 be pulled out completely in the Cipher* classes. I will be sending you a separate note about JCE logistics. Thanks for doing this cleanup. Brad On 6/20/2014 11:46 AM, Henry Jen wrote: Hi, Please review a trivial webrev to add JDK version to @since in a format as Mark suggested[1]. http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/ [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html Appened is the diff as in the webrev. Cheers, Henry diff --git a/src/share/classes/java/lang/Package.java b/src/share/classes/java/lang/Package.java --- a/src/share/classes/java/lang/Package.java +++ b/src/share/classes/java/lang/Package.java @@ -107,6 +107,7 @@ * loader to be found. * * @see ClassLoader#definePackage + * @since 1.2 */ public class Package implements java.lang.reflect.AnnotatedElement { /** diff --git a/src/share/classes/javax/crypto/CipherInputStream.java b/src/share/classes/javax/crypto/CipherInputStream.java --- a/src/share/classes/javax/crypto/CipherInputStream.java +++ b/src/share/classes/javax/crypto/CipherInputStream.java @@ -170,7 +170,7 @@ * @return the next byte of data, or code-1/code if the end of the * stream is reached. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read() throws IOException { if (ostart = ofinish) { @@ -196,7 +196,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[]) throws IOException { return read(b, 0, b.length); @@ -217,7 +217,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[], int off, int len) throws IOException { if (ostart = ofinish) { @@ -254,7 +254,7 @@ * @param n the number of bytes to be skipped. * @return the actual number of bytes skipped. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public long skip(long n) throws IOException { int available = ofinish - ostart; @@ -277,7 +277,7 @@ * @return the number of bytes that can be read from this input stream * without blocking. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int available() throws IOException { return (ofinish - ostart); @@ -292,7 +292,7 @@ * stream. * * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void close() throws IOException { if (closed) { @@ -321,7 +321,7 @@ * codemark/code and codereset/code methods. * @see java.io.InputStream#mark(int) * @see java.io.InputStream#reset() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public boolean markSupported() { return false; diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java b/src/share/classes/javax/crypto/CipherOutputStream.java --- a/src/share/classes/javax/crypto/CipherOutputStream.java +++ b/src/share/classes/javax/crypto/CipherOutputStream.java @@ -114,7 +114,7 @@ * * @param b the codebyte/code. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(int b) throws IOException { ibuffer[0] = (byte) b; @@ -138,7 +138,7 @@ * @exception NullPointerException if codeb/code is null. * @exception IOException if an I/O error occurs. * @see javax.crypto.CipherOutputStream#write(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[]) throws IOException { write(b, 0, b.length); @@ -152,7 +152,7 @@ * @param off the start offset in the data. * @param len the number of bytes to write. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[], int off, int len) throws IOException { obuffer = cipher.update(b, off, len);
Re: RFR: 8047721: @since should have JDK version
JCE (Cipher) changes look good to me. Let me know if there's any problem with the point I mentioned in the other email. Brad On 6/23/2014 1:50 PM, Henry Jen wrote: OK, I'll remove all @since JCE line, as the class already has @since 1.4 as Joe pointed out earlier. Uodated webrev at http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/ Cheers, Henry On 06/23/2014 10:04 AM, Bradford Wetmore wrote: I would prefer that JCE1.2 be pulled out completely in the Cipher* classes. I will be sending you a separate note about JCE logistics. Thanks for doing this cleanup. Brad On 6/20/2014 11:46 AM, Henry Jen wrote: Hi, Please review a trivial webrev to add JDK version to @since in a format as Mark suggested[1]. http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/ [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html Appened is the diff as in the webrev. Cheers, Henry diff --git a/src/share/classes/java/lang/Package.java b/src/share/classes/java/lang/Package.java --- a/src/share/classes/java/lang/Package.java +++ b/src/share/classes/java/lang/Package.java @@ -107,6 +107,7 @@ * loader to be found. * * @see ClassLoader#definePackage + * @since 1.2 */ public class Package implements java.lang.reflect.AnnotatedElement { /** diff --git a/src/share/classes/javax/crypto/CipherInputStream.java b/src/share/classes/javax/crypto/CipherInputStream.java --- a/src/share/classes/javax/crypto/CipherInputStream.java +++ b/src/share/classes/javax/crypto/CipherInputStream.java @@ -170,7 +170,7 @@ * @return the next byte of data, or code-1/code if the end of the * stream is reached. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read() throws IOException { if (ostart = ofinish) { @@ -196,7 +196,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[]) throws IOException { return read(b, 0, b.length); @@ -217,7 +217,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[], int off, int len) throws IOException { if (ostart = ofinish) { @@ -254,7 +254,7 @@ * @param n the number of bytes to be skipped. * @return the actual number of bytes skipped. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public long skip(long n) throws IOException { int available = ofinish - ostart; @@ -277,7 +277,7 @@ * @return the number of bytes that can be read from this input stream * without blocking. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int available() throws IOException { return (ofinish - ostart); @@ -292,7 +292,7 @@ * stream. * * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void close() throws IOException { if (closed) { @@ -321,7 +321,7 @@ * codemark/code and codereset/code methods. * @see java.io.InputStream#mark(int) * @see java.io.InputStream#reset() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public boolean markSupported() { return false; diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java b/src/share/classes/javax/crypto/CipherOutputStream.java --- a/src/share/classes/javax/crypto/CipherOutputStream.java +++ b/src/share/classes/javax/crypto/CipherOutputStream.java @@ -114,7 +114,7 @@ * * @param b the codebyte/code. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(int b) throws IOException { ibuffer[0] = (byte) b; @@ -138,7 +138,7 @@ * @exception NullPointerException if codeb/code is null. * @exception IOException if an I/O error occurs. * @seejavax.crypto.CipherOutputStream#write(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[]) throws IOException { write(b, 0, b.length); @@ -152,7 +152,7 @@ * @param off the start offset in the data. * @param len the number of bytes to write. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void
Re: RFR: 8047721: @since should have JDK version
What's the rationale for removing the secondary version? Or I guess the question should really be: when are secondary versions useful? At least in the EE specs, the EE version plus the spec version are listed in many places like this. Cheers, Paul On Mon, Jun 23, 2014 at 3:50 PM, Henry Jen henry@oracle.com wrote: OK, I'll remove all @since JCE line, as the class already has @since 1.4 as Joe pointed out earlier. Uodated webrev at http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/ Cheers, Henry On 06/23/2014 10:04 AM, Bradford Wetmore wrote: I would prefer that JCE1.2 be pulled out completely in the Cipher* classes. I will be sending you a separate note about JCE logistics. Thanks for doing this cleanup. Brad On 6/20/2014 11:46 AM, Henry Jen wrote: Hi, Please review a trivial webrev to add JDK version to @since in a format as Mark suggested[1]. http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/ [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/ 000806.html Appened is the diff as in the webrev. Cheers, Henry diff --git a/src/share/classes/java/lang/Package.java b/src/share/classes/java/lang/Package.java --- a/src/share/classes/java/lang/Package.java +++ b/src/share/classes/java/lang/Package.java @@ -107,6 +107,7 @@ * loader to be found. * * @see ClassLoader#definePackage + * @since 1.2 */ public class Package implements java.lang.reflect.AnnotatedElement { /** diff --git a/src/share/classes/javax/crypto/CipherInputStream.java b/src/share/classes/javax/crypto/CipherInputStream.java --- a/src/share/classes/javax/crypto/CipherInputStream.java +++ b/src/share/classes/javax/crypto/CipherInputStream.java @@ -170,7 +170,7 @@ * @return the next byte of data, or code-1/code if the end of the * stream is reached. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read() throws IOException { if (ostart = ofinish) { @@ -196,7 +196,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[]) throws IOException { return read(b, 0, b.length); @@ -217,7 +217,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[], int off, int len) throws IOException { if (ostart = ofinish) { @@ -254,7 +254,7 @@ * @param n the number of bytes to be skipped. * @return the actual number of bytes skipped. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public long skip(long n) throws IOException { int available = ofinish - ostart; @@ -277,7 +277,7 @@ * @return the number of bytes that can be read from this input stream * without blocking. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int available() throws IOException { return (ofinish - ostart); @@ -292,7 +292,7 @@ * stream. * * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void close() throws IOException { if (closed) { @@ -321,7 +321,7 @@ * codemark/code and codereset/code methods. * @see java.io.InputStream#mark(int) * @see java.io.InputStream#reset() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public boolean markSupported() { return false; diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java b/src/share/classes/javax/crypto/CipherOutputStream.java --- a/src/share/classes/javax/crypto/CipherOutputStream.java +++ b/src/share/classes/javax/crypto/CipherOutputStream.java @@ -114,7 +114,7 @@ * * @param b the codebyte/code. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(int b) throws IOException { ibuffer[0] = (byte) b; @@ -138,7 +138,7 @@ * @exception NullPointerException if codeb/code is null. * @exception IOException if an I/O error occurs. * @seejavax.crypto.CipherOutputStream#write(byte[], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public void write(byte b[]) throws IOException { write(b, 0, b.length); @@
Re: RFR: 8047721: @since should have JDK version
Except for these two classes, none of the JCE APIs ever contained @since until the JCE was put into JDK 1.4 back in 2002. The unbundled JCE hasn't been shipped in probably almost a decade. None of the unbundled JSSE/JGSS should have them either. Carrying around this old information is just cruft, IMO. Brad On 6/23/2014 2:28 PM, Paul Benedict wrote: What's the rationale for removing the secondary version? Or I guess the question should really be: when are secondary versions useful? At least in the EE specs, the EE version plus the spec version are listed in many places like this. Cheers, Paul On Mon, Jun 23, 2014 at 3:50 PM, Henry Jen henry@oracle.com mailto:henry@oracle.com wrote: OK, I'll remove all @since JCE line, as the class already has @since 1.4 as Joe pointed out earlier. Uodated webrev at http://cr.openjdk.java.net/~__henryjen/jdk9/8047721/2/__webrev/ http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/ Cheers, Henry On 06/23/2014 10:04 AM, Bradford Wetmore wrote: I would prefer that JCE1.2 be pulled out completely in the Cipher* classes. I will be sending you a separate note about JCE logistics. Thanks for doing this cleanup. Brad On 6/20/2014 11:46 AM, Henry Jen wrote: Hi, Please review a trivial webrev to add JDK version to @since in a format as Mark suggested[1]. http://cr.openjdk.java.net/~__henryjen/jdk9/8047721/0/__webrev/ http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/ [1] http://mail.openjdk.java.net/__pipermail/jdk9-dev/2014-June/__000806.html http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html Appened is the diff as in the webrev. Cheers, Henry diff --git a/src/share/classes/java/lang/__Package.java b/src/share/classes/java/lang/__Package.java --- a/src/share/classes/java/lang/__Package.java +++ b/src/share/classes/java/lang/__Package.java @@ -107,6 +107,7 @@ * loader to be found. * * @see ClassLoader#definePackage + * @since 1.2 */ public class Package implements java.lang.reflect.__AnnotatedElement { /** diff --git a/src/share/classes/javax/__crypto/CipherInputStream.java b/src/share/classes/javax/__crypto/CipherInputStream.java --- a/src/share/classes/javax/__crypto/CipherInputStream.java +++ b/src/share/classes/javax/__crypto/CipherInputStream.java @@ -170,7 +170,7 @@ * @return the next byte of data, or code-1/code if the end of the * stream is reached. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read() throws IOException { if (ostart = ofinish) { @@ -196,7 +196,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read(byte[__], int, int) - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[]) throws IOException { return read(b, 0, b.length); @@ -217,7 +217,7 @@ * the stream has been reached. * @exception IOException if an I/O error occurs. * @seejava.io.InputStream#read() - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public int read(byte b[], int off, int len) throws IOException { if (ostart = ofinish) { @@ -254,7 +254,7 @@ * @param n the number of bytes to be skipped. * @return the actual number of bytes skipped. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2 */ public long skip(long n) throws IOException { int available = ofinish - ostart; @@ -277,7 +277,7 @@ * @return the number of bytes that can be read from this input stream * without blocking. * @exception IOException if an I/O error occurs. - * @since JCE1.2 + * @since 1.4, JCE1.2
Re: RFR 6642881: Improve performance of Class.getClassLoader()
Please review a change to the JDK code for adding classLoader field to the instances of java/lang/Class. This change restricts reflection from changing access to the classLoader field. In the spec, AccessibleObject.setAccessible() may throw SecurityException if the accessibility of an object may not be changed: http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) Only AccessibleObject.java has changes from the previous version of this change. open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/ bug link https://bugs.openjdk.java.net/browse/JDK-6642881 Thanks, Coleen On 6/19/14, 11:01 PM, David Holmes wrote: On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote: Hi Mandy, On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote: On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote: Hi, On 19 jun 2014, at 20:46, Coleen Phillimore coleen.phillim...@oracle.com wrote: On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote: Have you considered hiding the Class.classLoader field from reflection? I’m not sure it is necessary but I’m not to keen on the idea of people poking at this field with Unsafe (which should go away in 9 but …). I don't know how to hide the field from reflection. It's a private final field so you need to get priviledges to make it setAccessible. If you mean injecting it on the JVM side, the reason for this change is so that the JIT compilers can inline this call and not have to call into the JVM to get the class loader. There is sun.reflect.Reflection.registerFieldsToFilter() that hides a field from being found using reflection. It might very well be the case that private and final is enough, I haven’t thought this through 100%. On the other hand, is there a reason to give users access through the field instead of having to use Class.getClassLoader()? There are many getter methods that returns a private final field. I'm not sure if hiding the field is necessary nor a good precedence to set. Accessing a private field requires accessDeclaredMember permission although it's a different security check (vs getClassLoader permission). Arguably before this new classLoader field, one could call Class.getClassLoader0() via reflection to get a hold of class loader. Perhaps you are concerned that the accessDeclaredMember permission is too coarse-grained? I think the security team is looking into the improvement in that regards. I think I’m a bit worried about two things, first as you wrote, “accessDeclaredMember” isn’t the same as “getClassLoader”, but since you could get the class loader through getClassLoader0() that shouldn’t be a new issue. The second thing is that IIRC there are ways to set a final field after initialization. I’m not sure we need to care about that either if you need Unsafe to do it. Normal reflection can set a final field if you can successfully call setAccessible(true) on the Field object. David - cheers /Joel
RFR: 8042872: Fix raw and unchecked warnings in sun.applet
Hi, Please review another webrev to clean up rawtypes and unchecked warning, this time for sun.applet. The webrev applied to both jdk9/dev and jdk9/client cleanly, I am not sure which repo should the webrev go once pass review. Webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8042872/0/webrev/ Please also let me know any particular test I should do other than regular jprt. Cheers, Henry
RFR: 8047795: Collections.checkedList checking bypassed by List.replaceAll
Hello all; This changeset corrects a reported problem with the lists returned by Collections.checkedList(). Since Java 8 the replaceAll() method on checked lists has erroneously allowed the operator providing replacements to provide illegal replacement values which are then stored, unchecked into the wrapped list. This changeset adds a check on the proposed replacement value and throws a ClassCastException if the replacement value is incompatible with the list. Additionally the javadoc is updated to inform users that a ClassCastException may result if the proposed replacement is unacceptable. Note that this changeset takes the strategy of failing when the illegal value is encountered. Replacements of earlier items in the list are retained. jbsbug: https://bugs.openjdk.java.net/browse/JDK-8047795 webrev: http://cr.openjdk.java.net/~mduigou/JDK-8047795/0/webrev/ This change will be backported to Java 8. Mike
Re: RFR: 8042872: Fix raw and unchecked warnings in sun.applet
Hi Henry, The changes look good to me; thanks, -Joe On 06/23/2014 05:31 PM, Henry Jen wrote: Hi, Please review another webrev to clean up rawtypes and unchecked warning, this time for sun.applet. The webrev applied to both jdk9/dev and jdk9/client cleanly, I am not sure which repo should the webrev go once pass review. Webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8042872/0/webrev/ Please also let me know any particular test I should do other than regular jprt. Cheers, Henry
Re: RFR 6642881: Improve performance of Class.getClassLoader()
Coleen, On 6/23/2014 4:45 PM, Coleen Phillimore wrote: Please review a change to the JDK code for adding classLoader field to the instances of java/lang/Class. This change restricts reflection from changing access to the classLoader field. In the spec, AccessibleObject.setAccessible() may throw SecurityException if the accessibility of an object may not be changed: http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) Only AccessibleObject.java has changes from the previous version of this change. This change looks reasonable.As a side note: Joel mentions about the mechanism to hide class members from reflection. I discussed with Joel offline before he went on parental leave and suggest that we should revisit the two mechanisms that both effectively disallow access to private members in the future. Mandy open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/ bug link https://bugs.openjdk.java.net/browse/JDK-6642881 Thanks, Coleen On 6/19/14, 11:01 PM, David Holmes wrote: On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote: Hi Mandy, On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote: On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote: Hi, On 19 jun 2014, at 20:46, Coleen Phillimore coleen.phillim...@oracle.com wrote: On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote: Have you considered hiding the Class.classLoader field from reflection? I’m not sure it is necessary but I’m not to keen on the idea of people poking at this field with Unsafe (which should go away in 9 but …). I don't know how to hide the field from reflection. It's a private final field so you need to get priviledges to make it setAccessible. If you mean injecting it on the JVM side, the reason for this change is so that the JIT compilers can inline this call and not have to call into the JVM to get the class loader. There is sun.reflect.Reflection.registerFieldsToFilter() that hides a field from being found using reflection. It might very well be the case that private and final is enough, I haven’t thought this through 100%. On the other hand, is there a reason to give users access through the field instead of having to use Class.getClassLoader()? There are many getter methods that returns a private final field. I'm not sure if hiding the field is necessary nor a good precedence to set. Accessing a private field requires accessDeclaredMember permission although it's a different security check (vs getClassLoader permission). Arguably before this new classLoader field, one could call Class.getClassLoader0() via reflection to get a hold of class loader. Perhaps you are concerned that the accessDeclaredMember permission is too coarse-grained? I think the security team is looking into the improvement in that regards. I think I’m a bit worried about two things, first as you wrote, “accessDeclaredMember” isn’t the same as “getClassLoader”, but since you could get the class loader through getClassLoader0() that shouldn’t be a new issue. The second thing is that IIRC there are ways to set a final field after initialization. I’m not sure we need to care about that either if you need Unsafe to do it. Normal reflection can set a final field if you can successfully call setAccessible(true) on the Field object. David - cheers /Joel
Re: RFR 6642881: Improve performance of Class.getClassLoader()
On 6/23/14, 9:36 PM, Mandy Chung wrote: Coleen, On 6/23/2014 4:45 PM, Coleen Phillimore wrote: Please review a change to the JDK code for adding classLoader field to the instances of java/lang/Class. This change restricts reflection from changing access to the classLoader field. In the spec, AccessibleObject.setAccessible() may throw SecurityException if the accessibility of an object may not be changed: http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) Only AccessibleObject.java has changes from the previous version of this change. This change looks reasonable.As a side note: Joel mentions about the mechanism to hide class members from reflection. I discussed with Joel offline before he went on parental leave and suggest that we should revisit the two mechanisms that both effectively disallow access to private members in the future. Thanks, Mandy. Yes, let me know what you come up with and we can improve this. Thank you for the help fixing this bug. Coleen Mandy open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/ bug link https://bugs.openjdk.java.net/browse/JDK-6642881 Thanks, Coleen On 6/19/14, 11:01 PM, David Holmes wrote: On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote: Hi Mandy, On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote: On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote: Hi, On 19 jun 2014, at 20:46, Coleen Phillimore coleen.phillim...@oracle.com wrote: On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote: Have you considered hiding the Class.classLoader field from reflection? I’m not sure it is necessary but I’m not to keen on the idea of people poking at this field with Unsafe (which should go away in 9 but …). I don't know how to hide the field from reflection. It's a private final field so you need to get priviledges to make it setAccessible. If you mean injecting it on the JVM side, the reason for this change is so that the JIT compilers can inline this call and not have to call into the JVM to get the class loader. There is sun.reflect.Reflection.registerFieldsToFilter() that hides a field from being found using reflection. It might very well be the case that private and final is enough, I haven’t thought this through 100%. On the other hand, is there a reason to give users access through the field instead of having to use Class.getClassLoader()? There are many getter methods that returns a private final field. I'm not sure if hiding the field is necessary nor a good precedence to set. Accessing a private field requires accessDeclaredMember permission although it's a different security check (vs getClassLoader permission). Arguably before this new classLoader field, one could call Class.getClassLoader0() via reflection to get a hold of class loader. Perhaps you are concerned that the accessDeclaredMember permission is too coarse-grained? I think the security team is looking into the improvement in that regards. I think I’m a bit worried about two things, first as you wrote, “accessDeclaredMember” isn’t the same as “getClassLoader”, but since you could get the class loader through getClassLoader0() that shouldn’t be a new issue. The second thing is that IIRC there are ways to set a final field after initialization. I’m not sure we need to care about that either if you need Unsafe to do it. Normal reflection can set a final field if you can successfully call setAccessible(true) on the Field object. David - cheers /Joel
Re: RFR 6642881: Improve performance of Class.getClassLoader()
On 24/06/2014 11:44 AM, Coleen Phillimore wrote: On 6/23/14, 9:36 PM, Mandy Chung wrote: Coleen, On 6/23/2014 4:45 PM, Coleen Phillimore wrote: Please review a change to the JDK code for adding classLoader field to the instances of java/lang/Class. This change restricts reflection from changing access to the classLoader field. In the spec, AccessibleObject.setAccessible() may throw SecurityException if the accessibility of an object may not be changed: http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) Only AccessibleObject.java has changes from the previous version of this change. This change looks reasonable.As a side note: Joel mentions about the mechanism to hide class members from reflection. I discussed with Joel offline before he went on parental leave and suggest that we should revisit the two mechanisms that both effectively disallow access to private members in the future. Thanks, Mandy. Yes, let me know what you come up with and we can improve this. Thank you for the help fixing this bug. Note that completely hiding something from reflection is different to controlling its accessability via reflection. I think what is being done here is the right way to go. The changes look okay to me. Thanks, David Coleen Mandy open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/ bug link https://bugs.openjdk.java.net/browse/JDK-6642881 Thanks, Coleen On 6/19/14, 11:01 PM, David Holmes wrote: On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote: Hi Mandy, On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote: On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote: Hi, On 19 jun 2014, at 20:46, Coleen Phillimore coleen.phillim...@oracle.com wrote: On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote: Have you considered hiding the Class.classLoader field from reflection? I’m not sure it is necessary but I’m not to keen on the idea of people poking at this field with Unsafe (which should go away in 9 but …). I don't know how to hide the field from reflection. It's a private final field so you need to get priviledges to make it setAccessible. If you mean injecting it on the JVM side, the reason for this change is so that the JIT compilers can inline this call and not have to call into the JVM to get the class loader. There is sun.reflect.Reflection.registerFieldsToFilter() that hides a field from being found using reflection. It might very well be the case that private and final is enough, I haven’t thought this through 100%. On the other hand, is there a reason to give users access through the field instead of having to use Class.getClassLoader()? There are many getter methods that returns a private final field. I'm not sure if hiding the field is necessary nor a good precedence to set. Accessing a private field requires accessDeclaredMember permission although it's a different security check (vs getClassLoader permission). Arguably before this new classLoader field, one could call Class.getClassLoader0() via reflection to get a hold of class loader. Perhaps you are concerned that the accessDeclaredMember permission is too coarse-grained? I think the security team is looking into the improvement in that regards. I think I’m a bit worried about two things, first as you wrote, “accessDeclaredMember” isn’t the same as “getClassLoader”, but since you could get the class loader through getClassLoader0() that shouldn’t be a new issue. The second thing is that IIRC there are ways to set a final field after initialization. I’m not sure we need to care about that either if you need Unsafe to do it. Normal reflection can set a final field if you can successfully call setAccessible(true) on the Field object. David - cheers /Joel