Re: RFR 9 8031708: Windows x86 build failure: JNU_ThrowOutOfMemoryError undefined
Hi Chris, Sorry that I did not check my previous change carefully on Windows. Thanks for fixing the issue. The change looks good to me. -Dan On 01/14/2014 06:06 AM, Chris Hegarty wrote: On 14 Jan 2014, at 14:03, Alan Bateman alan.bate...@oracle.com wrote: On 14/01/2014 13:56, Chris Hegarty wrote: A recent change, JDK-8029007 introduced JNU_ThrowOutOfMemoryError into MessageUtils. The Windows x86 build subsequently fails as follows: Warning (shows the root cause): c:/jprt/T/P1/113753.chhegar/s/jdk/src/share/native/sun/misc/MessageUtils.c(58) : warning C4013: 'JNU_ThrowOutOfMemoryError' undefined; assuming extern returning int This looks okay, I assume there must be a different version of VC++ being used here. Thanks of the review Alan. A warning about the implicit definition is outputted on all platforms, but only seems fatal during linking on Windows x86. I assume it is related to different compiler/linker versions. Either way, the fix is trivial and gets us back up and running on Windows. -Chris. -Alan
Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions
Thanks, Chris. Right, I don't find any usage of getThreadStateValues, so it is simpler to just remove it. -Dan On 01/09/2014 11:49 PM, Chris Hegarty wrote: Looks ok to me. I presume getThreadStateValues is simply no longer needed. -Chris. On 10 Jan 2014, at 06:31, Dan Xu dan...@oracle.com wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 -Dan
Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions
On 01/10/2014 02:32 AM, Alan Bateman wrote: On 10/01/2014 06:31, Dan Xu wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 This looks good, the only one that isn't clear (to me) is the GetStringLength usage in MessageUtil.c where I don't think it is possible to ever get 0. This may be a case where you need to use ExceptionOccured instead. -Alan. According to jni.cpp, GetStringLength() will always return positive value or 0. For simplicity, I will change = 0 to == 0. Thanks, Alan. -Dan
Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions
Hi Roger, My macro is a little different from yours, which compares with -1 instead of NULL. I also see CHECK_EXCEPTION macro. Thanks for adding them, which are useful when I cannot decide the pending exception state by just using return values. As for the style, actually I prefer the (!pointer) to (pointer == NULL) because it is more concise and also make me avoid the typo like (pointer = NULL), which I cannot find from the compilation. Thanks! -Dan On 01/10/2014 08:40 AM, roger riggs wrote: Hi Dan, Just pushed are macros in jni_util.h to do the same function as your new macros. Please update to use the common macros instead of introducing new ones. Style wise, I would avoid mixing binary operators (!) with pointers. it is clearer to compare with NULL. (The CHECK_NULL macro will do the check and return). (Not a Reviewer) Thanks, Roger On 1/10/2014 1:31 AM, Dan Xu wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 -Dan
Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions
Yes, you are right. I just found this macro. It looks very handy to use. Thanks! -Dan On 01/10/2014 09:59 AM, roger riggs wrote: Hi Dan, One other comment, instead of changing the return type of the setStaticIntField just return and the caller should check for exceptions and return. See jni.h: CHECK_EXCEPTION(env) Roger On 1/10/2014 11:40 AM, roger riggs wrote: Hi Dan, Just pushed are macros in jni_util.h to do the same function as your new macros. Please update to use the common macros instead of introducing new ones. Style wise, I would avoid mixing binary operators (!) with pointers. it is clearer to compare with NULL. (The CHECK_NULL macro will do the check and return). (Not a Reviewer) Thanks, Roger On 1/10/2014 1:31 AM, Dan Xu wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 -Dan
Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions
Thank you for all the feedback. I have updated my changes to use CHECK_EXCEPTION_RETURN and CHECK_EXCEPTION macro recently added into jni_util.h. I also removed else block in function setStaticIntField() in Version.c since (*env)-GetStaticFieldID will throw a same exception if the field cannot be found. Here is the new webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.01/. Thanks! -Dan On 01/10/2014 10:21 AM, Mike Duigou wrote: On Jan 10 2014, at 10:09 , Chris Hegarty chris.hega...@oracle.com wrote: On 10 Jan 2014, at 18:05, Dan Xu dan...@oracle.com wrote: Hi Roger, My macro is a little different from yours, which compares with -1 instead of NULL. I also see CHECK_EXCEPTION macro. Thanks for adding them, which are useful when I cannot decide the pending exception state by just using return values. As for the style, actually I prefer the (!pointer) to (pointer == NULL) because it is more concise and also make me avoid the typo like (pointer = NULL), which I cannot find from the compilation. Thanks! There's always yoda conditions https://en.wikipedia.org/wiki/Yoda_conditions, (NULL == pointer), but that's not likely to make anyone (besides me) happy. Mike Not that it matters, but my preference is to == NULL. -Chris. -Dan On 01/10/2014 08:40 AM, roger riggs wrote: Hi Dan, Just pushed are macros in jni_util.h to do the same function as your new macros. Please update to use the common macros instead of introducing new ones. Style wise, I would avoid mixing binary operators (!) with pointers. it is clearer to compare with NULL. (The CHECK_NULL macro will do the check and return). (Not a Reviewer) Thanks, Roger On 1/10/2014 1:31 AM, Dan Xu wrote: Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 -Dan
RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions
Hi All, Please review the fix for JNI pending exception issues reported in jdk-8029007. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8029007 -Dan
Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions
Thank you, Alan. I will add this change into my fix and push it today. Thanks! -Dan On 01/08/2014 01:24 AM, Alan Bateman wrote: On 08/01/2014 00:50, Dan Xu wrote: Hi All, Thanks for your good review. I have dropped the change in FileSystemPreferences.java , and created the new webrev which only changes FileSystemPreferences.c. Please help review it. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev.01/ When changing FileSystemPreferences.c, I noticed the code like JNU_GetStringPlatformChars(env, java_fname, JNI_FALSE). Because JNI_FALSE is passed into this function, I am wondering why our code still release string by calling JNU_ReleaseStringPlatformChars(env, java_fname, fname). Thanks! This is the isCopy parameter to allow the function return whether it has returned a copy or not. So the JNU_ReleaseStringPlatformChars is needed. However the JNI_FALSE isn't right, it does happens to work because JNI_FALSE is defined as 0, same as NULL. Do you mind changing these to NULL as part of the change so that it's a bit more obvious? (no need to publish a new webrev if there are no other changes). Otherwise looks fine to me. -Alan.
Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions
Hi Alan, Btw, I am wondering whether I need pass a variable to see if the returned char* is a copy or not, and basing on that to do the release. For example, jboolean isCopy; const char *fname = JNU_GetStringPlatformChars(env, java_fname, isCopy); if (isCopy == JNI_TRUE) JNU_ReleaseStringPlatformChars(env, java_fname, fname); But according to the definition of JNU_GetStringPlatformChars(), it seems always to set *isCopy to JNI_TRUE currently. Because nativeGetStringPlatformChars() does nothing now. Thanks, -Dan On Wed 08 Jan 2014 09:56:40 AM PST, Dan Xu wrote: Thank you, Alan. I will add this change into my fix and push it today. Thanks! -Dan On 01/08/2014 01:24 AM, Alan Bateman wrote: On 08/01/2014 00:50, Dan Xu wrote: Hi All, Thanks for your good review. I have dropped the change in FileSystemPreferences.java , and created the new webrev which only changes FileSystemPreferences.c. Please help review it. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev.01/ When changing FileSystemPreferences.c, I noticed the code like JNU_GetStringPlatformChars(env, java_fname, JNI_FALSE). Because JNI_FALSE is passed into this function, I am wondering why our code still release string by calling JNU_ReleaseStringPlatformChars(env, java_fname, fname). Thanks! This is the isCopy parameter to allow the function return whether it has returned a copy or not. So the JNU_ReleaseStringPlatformChars is needed. However the JNI_FALSE isn't right, it does happens to work because JNI_FALSE is defined as 0, same as NULL. Do you mind changing these to NULL as part of the change so that it's a bit more obvious? (no need to publish a new webrev if there are no other changes). Otherwise looks fine to me. -Alan.
Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions
Hi All, Thanks for your good review. I have dropped the change in FileSystemPreferences.java , and created the new webrev which only changes FileSystemPreferences.c. Please help review it. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev.01/ When changing FileSystemPreferences.c, I noticed the code like JNU_GetStringPlatformChars(env, java_fname, JNI_FALSE). Because JNI_FALSE is passed into this function, I am wondering why our code still release string by calling JNU_ReleaseStringPlatformChars(env, java_fname, fname). Thanks! -Dan On 01/07/2014 01:36 AM, Alan Bateman wrote: On 06/01/2014 22:29, Dan Xu wrote: Hi All, Please review the simple fix for JNI pending exceptions in FileSystemPreferences.c. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8028726 Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/ The update to FIleSystemPreferences.c looks okay but if I read it correctly then lockFile0 can never return NULL without a pending exception (meaning that the change to FileSystemPreferences.java could mask an underlying bug if it existed). One passing comment is that this native methods could be completely eliminated here by changing FileSystemPreferences to lock the file via a FileChannel (use a FileLock as the lock handle). Also the chmod usage can be eliminated by mkdirs with Files.createDirectory and specify the permission files when creating the directory. I realize this is beyond the scope of what you are doing here (but an opportunity none the less). -Alan
RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions
Hi All, Please review the simple fix for JNI pending exceptions in FileSystemPreferences.c. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8028726 Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/ -Dan
Re: RFR: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions
Hi Lance, Thanks for your review. My understanding towards the for loop in lockFile() of FileSystemPreferences.java is that it retries if anything fails. So I don't touch L914 to keep its old logic un-changed. Please let me know if you have some good suggestions. Thanks! -Dan On 01/06/2014 02:35 PM, Lance Andersen - Oracle wrote: Dan, Looks OK, but line 914 which you did not change, notice the comments not sure if that is common in this code but seemed a bit off to me: 914 //// If at first, you don't succeed... On Jan 6, 2014, at 5:29 PM, Dan Xu wrote: Hi All, Please review the simple fix for JNI pending exceptions in FileSystemPreferences.c. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8028726 Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/ -Dan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: JDK-8022219, Intermittent test failures in java/util/zip/ZipFile
Hi Alan and Chris, Thanks for your review. Unfortunately, my jtreg is still an old version. And when I tried to use /lib/testlibrary, my jprt job also failed. -Dan On 12/10/2013 03:04 AM, Chris Hegarty wrote: On 10 Dec 2013, at 09:10, Alan Bateman alan.bate...@oracle.com wrote: On 08/12/2013 17:29, Dan Xu wrote: Hi All, Please review the fix towards the intermittent test failure in java/util/zip/ZipFile. It is a similar failure that I encountered before, due to the interferences from other software or daemon services in Windows platforms. The fix is to use the method from FileUtils test library to handle the problem at its best effort. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8022219 Webrev: http://cr.openjdk.java.net/~dxu/8022219/webrev/ http://cr.openjdk.java.net/%7Edxu/8022219/webrev/ -Dan This looks okay to me although I think you should be able to use /lib/testlibrary as the @library value (assuming your jtreg is somewhat recent). +1. -Chris. -Alan.
RFR: JDK-8022219,Intermittent test failures in java/util/zip/ZipFile
Hi All, Please review the fix towards the intermittent test failure in java/util/zip/ZipFile. It is a similar failure that I encountered before, due to the interferences from other software or daemon services in Windows platforms. The fix is to use the method from FileUtils test library to handle the problem at its best effort. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8022219 Webrev: http://cr.openjdk.java.net/~dxu/8022219/webrev/ http://cr.openjdk.java.net/%7Edxu/8022219/webrev/ -Dan
hg: jdk8/tl/jdk: 8028628: java/nio/channels/FileChannel/Size.java failed once in the same binary run
Changeset: 81708985c0a2 Author:dxu Date: 2013-11-21 14:23 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/81708985c0a2 8028628: java/nio/channels/FileChannel/Size.java failed once in the same binary run Reviewed-by: alanb, chegar, mchung, lancea ! test/java/nio/channels/FileChannel/Size.java
hg: jdk8/tl/jdk: 7065902: (file) test/java/nio/file/Files/Misc.java fails on Solaris 11 when run as root
Changeset: a74d6aa51654 Author:dxu Date: 2013-11-21 14:16 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a74d6aa51654 7065902: (file) test/java/nio/file/Files/Misc.java fails on Solaris 11 when run as root Reviewed-by: alanb ! test/java/nio/file/Files/Misc.java
RFR: JDK-7065902 - (file) test/java/nio/file/Files/Misc.java fails on Solaris 11 when run as root
Hi All, Please review the simple fix towards test/java/nio/file/Files/Misc.java. It only fails when it is run by root. As Alan commented in the bug, the root has all permissions, so the test assumptions are wrong. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-7065902 Webrev: http://cr.openjdk.java.net/~dxu/7065902/webrev/ -Dan
Re: RFR: JDK-8028628 - java/nio/channels/FileChannel/Size.java failed once in the same binary run
On 11/21/2013 05:41 AM, Alan Bateman wrote: On 21/11/2013 01:09, Dan Xu wrote: Hi All, I have updated my fix based on your suggestions. I have changed to create testing files in the working directory, moved those static member variables into local method variables, and used try-with-resources to read and write the testing files. After the change, the file delete is no longer important. So I just do the clean-up with deleteOnExit() for simplicity. If the test fails, it is better to keep the test file to give more clue. Therefore, I don't put the file clean-up into finally block. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8028628/webrev01/ -Dan Just one thing about testLargeFile, I see that it additionally creates a file-mapping and it's not clear that this is needed (I don't see anything in JDK-4563125 to explain this). I suspect this can be removed. Otherwise it looks okay to me. -Alan. Hi Alan, I think the map is used to enlarge the size of the largeFile to testSize + 10. In initTestFile(), it initiates the file with size 10. My understanding is that it avoids writing large amount of data into the largeFile by using the map. Thanks! -Dan
Re: RFR: JDK-8028628 - java/nio/channels/FileChannel/Size.java failed once in the same binary run
Hi Alan, When looking at the FileChannel.map() method, it seems that it will not write e 4 billion times. Instead, it only extends the file size to testSize+10. It uses ftruncate() in linux and uses SetFilePointer() in Windows, which causes the file offset different on different platforms. But the performance should be good. On windows, this test runs for around 0.14 seconds in jprt machines. Thanks! -Dan On Thu 21 Nov 2013 11:51:28 AM PST, Alan Bateman wrote: On 21/11/2013 17:02, Dan Xu wrote: Hi Alan, I think the map is used to enlarge the size of the largeFile to testSize + 10. In initTestFile(), it initiates the file with size 10. My understanding is that it avoids writing large amount of data into the largeFile by using the map. Thanks! -Dan Okay, I guess I was really just wondering about the reliability on Windows as sometimes tests that use memory mapped files are troublesome when it comes to clean-up after the test. Also as this test hasn't run on Windows before then I wonder about how long initTestFile will take to create the 4GB file. You've probably measured already but if you need a speed up then you could change initTestFile to use a FIleChannel and create the file with the SPARSE option. Then just position to size-1 and write 1 byte. That should be faster than writing e 4 billion times. -Alan.
Re: RFR: JDK-8028628 - java/nio/channels/FileChannel/Size.java failed once in the same binary run
Hi All, I have updated my fix based on your suggestions. I have changed to create testing files in the working directory, moved those static member variables into local method variables, and used try-with-resources to read and write the testing files. After the change, the file delete is no longer important. So I just do the clean-up with deleteOnExit() for simplicity. If the test fails, it is better to keep the test file to give more clue. Therefore, I don't put the file clean-up into finally block. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8028628/webrev01/ -Dan On 11/20/2013 04:08 AM, Alan Bateman wrote: On 19/11/2013 23:57, Dan Xu wrote: Hi All, Please review the simple fix towards Size.java testcase. It failed once on windows platform in the recent same binary run, which is mostly due to some interferences and the special delete handling on windows. In the fix, I remove the delete operation in initTestFile() method because FileOutputStream will truncate the file content and it is not necessary to delete it first. Thanks! Bug:https://bugs.openjdk.java.net/browse/JDK-8028628 Webrev: http://cr.openjdk.java.net/~dxu/8028628/webrev/ http://cr.openjdk.java.net/%7Edxu/8028628/webrev/ This does look like a case where the test is needlessly deleting and re-creating the file (although still annoying to have interference from virus checkers or other background services). As you point out, FileOutputStream will truncate an existing file so it's not needed. So I think your changes to remove the exist/delete from the init method is good. If you have the cycles then there are probably a few clean-ups that could be done on this test. I don't think blah needs to be static, it could use try-with-resources and delete blah in the finally block. Also test2 looks historical, it may be that this can be enabled on Linux and Windows now (the bug/comments seem to date from JDK 1.4). -Alan
RFR: JDK-8028631 - Improve the test coverage to the pathname handling on unix-like platforms
Hi All, We have java/io/pathNames/GeneralWin32.java testcase to do the general exhaustive test of pathname handling on windows. I am adding a new test GeneralSolaris.java to test the pathname handling in unix-like platforms. In the changes, I also make sure the test can run successfully in concurrency mode. Please help review it. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8028631 Webrev: http://cr.openjdk.java.net/~dxu/8028631/webrev/ -Dan
hg: jdk8/tl/jdk: 8028631: Improve the test coverage to the pathname handling on unix-like platforms
Changeset: f496565c4eec Author:dxu Date: 2013-11-19 13:22 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f496565c4eec 8028631: Improve the test coverage to the pathname handling on unix-like platforms Summary: Add GeneralSolaris.java testcase and fix the concurrency issue Reviewed-by: lancea, chegar, alanb ! test/java/io/pathNames/General.java + test/java/io/pathNames/GeneralSolaris.java ! test/java/io/pathNames/GeneralWin32.java
RFR: JDK-8028628 - java/nio/channels/FileChannel/Size.java failed once in the same binary run
Hi All, Please review the simple fix towards Size.java testcase. It failed once on windows platform in the recent same binary run, which is mostly due to some interferences and the special delete handling on windows. In the fix, I remove the delete operation in initTestFile() method because FileOutputStream will truncate the file content and it is not necessary to delete it first. Thanks! Bug:https://bugs.openjdk.java.net/browse/JDK-8028628 Webrev: http://cr.openjdk.java.net/~dxu/8028628/webrev/ http://cr.openjdk.java.net/%7Edxu/8028628/webrev/ -Dan
Re: RFR: JDK-8028628 - java/nio/channels/FileChannel/Size.java failed once in the same binary run
Hi Mandy, I think that writing to the current directory and writing to the temp directory will get the same interference in this testcase. Because the interference is mostly coming from the anti-virus software or some windows system services. Any file changes in the file system may trigger them. Due to the interference, if a test deletes a file and then immediately create a file with the same name, the create operation may fail with access denied exception. I have described it in detail when discussing Chris's webrev, http://openjdk.5641.n7.nabble.com/RFR-8022213-Intermittent-test-failures-in-java-net-URLClassLoader-Add-jdk-testlibrary-FileUtils-java-td165561.html. Thanks! -Dan On 11/19/2013 07:08 PM, Mandy Chung wrote: On 11/19/2013 3:57 PM, Dan Xu wrote: Hi All, Please review the simple fix towards Size.java testcase. It failed once on windows platform in the recent same binary run, which is mostly due to some interferences and the special delete handling on windows. In the fix, I remove the delete operation in initTestFile() method because FileOutputStream will truncate the file content and it is not necessary to delete it first. Thanks! Bug:https://bugs.openjdk.java.net/browse/JDK-8028628 Webrev: http://cr.openjdk.java.net/~dxu/8028628/webrev/ http://cr.openjdk.java.net/%7Edxu/8028628/webrev/ The patch itself is okay. Would you think if writing to a file in the current directory rather than temp is less prone to the interference? Mandy
Re: RFR doc only typo in java.io.DataInput
Hi Roger, The change looks good. -Dan On 11/07/2013 02:29 PM, roger riggs wrote: Please review this straightforward typo correction: Webrev: http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/ Thanks, Roger
Re: RFR: 8022213 Intermittent test failures in java/net/URLClassLoader (Add jdk/testlibrary/FileUtils.java)
On 11/07/2013 11:04 AM, Alan Bateman wrote: On 07/11/2013 14:59, Chris Hegarty wrote: Given both Michael and Alan's comments. I've update the webrev: http://cr.openjdk.java.net/~chegar/fileUtils.01/webrev/ 1) more descriptive method names 2) deleteXXX methods return if interrupted, leaving the interrupt status set 3) Use Files.copy with REPLACE_EXISTING 4) Use SimpleFileVisitor, rather than FileVisitor This looks better although interrupting the sleep means that the deleteXXX will quietly terminate with the interrupt status set (which could be awkward to diagnose if used with tests that are also using Thread.interrupt). An alternative might be to just throw the IOException with InterruptedException as the cause. -Alan. Hi Chris, In the method, deleteFileWithRetry0(), it assumes that if any other process is accessing the same file, the delete operation, Files.delete(), will throw out IOException on Windows. But I don't see this assumption is always true when I investigated this issue on intermittent test failures. When Files.delete() method is called, it finally calls DeleteFile or RemoveDirectory functions based on whether the target is a file or directory. And these Windows APIs only mark the target for deletion on close and return immediately without waiting the operation to be completed. If another process is accessing the file in the meantime, the delete operation does not occur and the target file stays at delete-pending status until that open handle is closed. It basically implies that DeleteFile and RemoveDirectory is like an async operation. Therefore, we cannot assume that the file/directory is deleted after Files.delete() returns or File.delete() returns true. When checking those intermittently test failures, I find the test normally succeeds on the Files.delete() call. But due to the interference of Anti-virus or other Windows daemon services, the target file changes to delete-pending status. And the immediately following operation fails due the target file still exists, but our tests assume the target file is already gone. Because the delete-pending status of a file usually last for a very short time which depends on the interference source, such failures normally happens when we recursively delete a folder or delete-and-create a file with the same file name at a high frequency. It is basically a Windows API design or implementation issue. I have logged an enhancement, JDK-8024496, to solve it from Java library layer. Currently, I have two strategies in mind. One is to make the delete operation blocking, which means to make sure the file/directory is deleted before the return. The other is to make sure the delete-pending file does not lead to a failure of subsequent file operations. But they both has pros and cons. Thank! -Dan
hg: jdk8/tl/jdk: 8025698: (fs) Typo in exception thrown by encode() in UnixPath.java
Changeset: 6ea1f9d8ec78 Author:dxu Date: 2013-11-06 13:25 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6ea1f9d8ec78 8025698: (fs) Typo in exception thrown by encode() in UnixPath.java Reviewed-by: dxu, mduigou, henryjen, weijun Contributed-by: ajuc...@gmail.com ! src/solaris/classes/sun/nio/fs/UnixPath.java
hg: jdk8/tl/jdk: 8027612: java/io/File/MaxPathLength.java fails intermittently in the clean-up stage
Changeset: 6d1f3ba68db2 Author:dxu Date: 2013-11-04 15:48 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6d1f3ba68db2 8027612: java/io/File/MaxPathLength.java fails intermittently in the clean-up stage Reviewed-by: chegar ! test/java/io/File/MaxPathLength.java
hg: jdk8/tl/jdk: 8027624: com/sun/crypto/provider/KeyFactory/TestProviderLeak.java unstable again
Changeset: 6a6faa00acc4 Author:dxu Date: 2013-11-01 14:40 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6a6faa00acc4 8027624: com/sun/crypto/provider/KeyFactory/TestProviderLeak.java unstable again Reviewed-by: wetmore ! test/com/sun/crypto/provider/KeyFactory/TestProviderLeak.java
Re: [test-only] JDK 8 RFR 8027625: test/java/math/BigInteger/ExtremeShiftingTests.java needs @run tag to specify heap size
Looks good to me. -Dan On 11/01/2013 05:07 PM, Brian Burkhalter wrote: Please review at your convenience: Issue: https://bugs.openjdk.java.net/browse/JDK-8027625 Patch: --- a/test/java/math/BigInteger/ExtremeShiftingTests.java Fri Nov 01 14:40:03 2013 -0700 +++ b/test/java/math/BigInteger/ExtremeShiftingTests.java Fri Nov 01 17:03:04 2013 -0700 @@ -25,6 +25,7 @@ * @test * @bug 6371401 * @summary Tests of shiftLeft and shiftRight on Integer.MIN_VALUE + * @run main/othervm -Xmx512m ExtremeShiftingTests * @author Joseph D. Darcy */ import java.math.BigInteger; Thanks, Brian
hg: jdk8/tl/jdk: 8027155: test/java/io/File/NulFile.java failing when test run in othervm mode
Changeset: e93de88661ab Author:dxu Date: 2013-10-31 11:52 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e93de88661ab 8027155: test/java/io/File/NulFile.java failing when test run in othervm mode Reviewed-by: mchung, alanb ! test/java/io/File/NulFile.java
RFR: JDK-8027155 - test/java/io/File/NulFile.java failing when test run in othervm mode
Hi All, Here is a simple change to fix the test failure reported in JDK-8027155. Due to the code change in JDK-8025128, this test also needs to be updated. Bug: https://bugs.openjdk.java.net/browse/JDK-8027155 Webrev: http://cr.openjdk.java.net/~dxu/8027155/webrev/ Thanks, -Dan
hg: jdk8/tl/jdk: 7122887: JDK ignores Gnome3 proxy settings
Changeset: c9562ac9b812 Author:dxu Date: 2013-10-23 22:30 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c9562ac9b812 7122887: JDK ignores Gnome3 proxy settings Summary: Fix GConf and add to use GProxyResolver to handle network proxy resolution Reviewed-by: chegar ! src/solaris/native/sun/net/spi/DefaultProxySelector.c ! test/java/net/ProxySelector/SystemProxies.java
hg: jdk8/tl/jdk: 8025712: (props) Possible memory leak in java_props_md.c / ParseLocale
Changeset: cb373cf43294 Author:dxu Date: 2013-10-11 09:47 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cb373cf43294 8025712: (props) Possible memory leak in java_props_md.c / ParseLocale Reviewed-by: naoto, chegar ! src/solaris/native/java/lang/java_props_md.c
Re: RFR: JDK-8025712, , (props) Possible memory leak in java_props_md.c / ParseLocale
Good catch, Chris. Btw, according to the description, it seems the block in #ifndef __linux__ is the workaround only for Solaris. Shall we use a more strict macro here instead of #ifndef __linux__? Thanks! -Dan On 10/10/2013 02:22 AM, Chris Hegarty wrote: Dan, Your changes look fine to me. While looking at this I notice that there is another potential leak. If the malloc for temp fails, then we need to free lc ( for MAC only ), right? -Chris. On 10/09/2013 07:51 PM, Dan Xu wrote: Hi All, This fix is to solve the memory leak issue in ParseLocale() function of jdk/src/solaris/native/java/lang/java_props_md.c. Because the locale, lc, is copied into temp, it is not necessary to do the strdup(), which leads to the memery leak. The fix simply removes the line of strdup operation. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8025712/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8025712 -Dan
Re: RFR: JDK-8025712, , (props) Possible memory leak in java_props_md.c / ParseLocale
Thanks for your clarification, Naoto. Here is the updated webrev, http://cr.openjdk.java.net/~dxu/8025712/webrev.01/. Please help review it. -Dan On 10/10/2013 03:14 PM, Naoto Sato wrote: You could, but that part only relates to the @euro locales handling, which is at this moment almost obsolete, let alone for MacOSX platform. So the current code should work fine. Naoto On 10/10/13 1:23 PM, Dan Xu wrote: Good catch, Chris. Btw, according to the description, it seems the block in #ifndef __linux__ is the workaround only for Solaris. Shall we use a more strict macro here instead of #ifndef __linux__? Thanks! -Dan On 10/10/2013 02:22 AM, Chris Hegarty wrote: Dan, Your changes look fine to me. While looking at this I notice that there is another potential leak. If the malloc for temp fails, then we need to free lc ( for MAC only ), right? -Chris. On 10/09/2013 07:51 PM, Dan Xu wrote: Hi All, This fix is to solve the memory leak issue in ParseLocale() function of jdk/src/solaris/native/java/lang/java_props_md.c. Because the locale, lc, is copied into temp, it is not necessary to do the strdup(), which leads to the memery leak. The fix simply removes the line of strdup operation. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8025712/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8025712 -Dan
RFR: JDK-8025712,,(props) Possible memory leak in java_props_md.c / ParseLocale
Hi All, This fix is to solve the memory leak issue in ParseLocale() function of jdk/src/solaris/native/java/lang/java_props_md.c. Because the locale, lc, is copied into temp, it is not necessary to do the strdup(), which leads to the memery leak. The fix simply removes the line of strdup operation. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8025712/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8025712 -Dan
RFR: JDK-8026061 - File.createTempFile fails if prefix is absolute path
Hi, This is a backport request towards 7u-dev repository on bug JDK-8025128 that I fixed several days ago. It is going to keep the method, File.createTempFile(), backward compatible. Please help review it. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8026061 Webrev: http://cr.openjdk.java.net/~dxu/8026061/webrev/ -Dan
Re: RFR: JDK-8026061 - File.createTempFile fails if prefix is absolute path
Sean and Alan, Thanks for the instructions. I just sent out the request to jdk7u-dev mailing list. -Dan On Tue 08 Oct 2013 12:49:08 PM PDT, Seán Coffey wrote: Yes - no problem pushing the changeset for Dan once the approval request is logged and approved. regards, Sean. On 08/10/2013 20:43, Alan Bateman wrote: On 08/10/2013 19:24, Dan Xu wrote: Hi, This is a backport request towards 7u-dev repository on bug JDK-8025128 that I fixed several days ago. It is going to keep the method, File.createTempFile(), backward compatible. Please help review it. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8026061 Webrev: http://cr.openjdk.java.net/~dxu/8026061/webrev/ -Dan This looks like it's the same patch that is in jdk8 so it doesn't need to be reviewed again. Instead you just need to send an approval request to the jdk7u-dev list to request approval to push it there. The approval template page [1] has the list of things that need to be included in the request. One other thing to point out is that you don't have committer role on jdk7u so you are going to need a sponsor. I'm sure Seán or one of the other maintainers will help on this. -Alan [1] http://openjdk.java.net/projects/jdk7u/approval-template.html
RFR: JDK-8025128,File.createTempFile fails if prefix is absolute path
Hi, Recently I made changes in TempDirectory.generateFile() method to prevent using invalidparameters to create temporary files. But such tight control will bring a compatibility issue. This change is going to solve theseproblems. Please help review it. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8025128 webrev: http://cr.openjdk.java.net/~dxu/8025128/webrev/ http://cr.openjdk.java.net/%7Edxu/8025128/webrev/ -Dan
Re: RFR: JDK-8025610 : (xs) Optional constructor and Optional.of should explicitly document NPE
It looks good. -Dan On 09/27/2013 03:53 PM, Mike Duigou wrote: Hello all; As pointed out by Roger Riggs, Optional.of (and the private Optional(T) constructor) don't explicitly document throwing NPE in response to a null value despite describing value as non-null. This changeset improves the documentation to add an @throws NPE tag to the javadoc. http://cr.openjdk.java.net/~mduigou/JDK-8025610/0/webrev/ Thanks, Mike
hg: jdk8/tl/jdk: 8025128: File.createTempFile fails if prefix is absolute path
Changeset: 754db1268be1 Author:dxu Date: 2013-09-27 17:09 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/754db1268be1 8025128: File.createTempFile fails if prefix is absolute path Summary: Use only the file name from the supplied prefix for backward compatibility Reviewed-by: alanb, chegar ! src/share/classes/java/io/File.java ! test/java/io/File/createTempFile/SpecialTempFile.java
hg: jdk8/tl/jdk: 8023765: Improve MaxPathLength.java testcase and reduce its test load; ...
Changeset: 5b01c851bb1d Author:dxu Date: 2013-08-30 16:45 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5b01c851bb1d 8023765: Improve MaxPathLength.java testcase and reduce its test load 7160013: java/io/File/MaxPathLength.java fails Reviewed-by: alanb ! test/ProblemList.txt ! test/java/io/File/MaxPathLength.java
hg: jdk8/tl/jdk: 4792059: test/java/io/pathNames/GeneralSolaris.java fails on symbolic links
Changeset: 5bf4f285 Author:dxu Date: 2013-08-29 10:43 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5bf4f285 4792059: test/java/io/pathNames/GeneralSolaris.java fails on symbolic links Summary: Exclude the possible usage of linked files or directories in the test Reviewed-by: alanb ! test/java/io/pathNames/General.java
Re: RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load
On 08/29/2013 12:20 AM, Alan Bateman wrote: On 29/08/2013 00:49, Dan Xu wrote: : Stuart brought a very good point for this test failure in the main bug, JDK-7160013. According to his comment, Bottom line is that a Windows daemon (not an anti-virus scanner) may hold a file in delete-pending state for a short amount of time after it's been deleted. While it's in this state, if you ask if the file exists, the system will say that it does not. However, if you try to create a new file with the same name, you get an access denied error. And I re-checked remarks in the Windows APIs used for deleting file in [1] and directory in [2]. According to the spec, these APIs only mark a file/directory for deletion on close. And the delete operation does not occur until the last handle to it is closed. Therefore, if a Windows daemon opens a handle towards those newly-created directories (I think anti-virus on-access scanner may do a similar thing), the delete operation will be put in delete-pending state, and the next immediate call to create the same directory structure will fail. Based on the above information, I updated this testcase to make it create different directory structure in each loop, so that the next-create operation will not conflict with previous pending delete operation. I also follow Alan's suggestion to use Files.deleteIfExists in the latest change. Please review it at http://cr.openjdk.java.net/~dxu/8023765/webrev.01/. Thanks! If we want stable and fast test execution on Windows machines then I think we have to remove these sources of interference. In my view this means disabling the services that snoop on your usage so that they aren't opening files that are trying to move or remove. It also means configuring the AV software to exclude some area of the file system that we can use as the test work tree. Anyway, the patch looks okay, it's just the new comment on L60 might be a bit mis-leading. -Alan. I tried to run this test on all platforms by removing L61. But it fails on the second while loop on Solaris and Mac platforms. The reason is that the testing path, which contains 20-level directory, is too long for them to handle. That is why I added a new comment in L60. I agree that we need a clean testing environment. David DeHaven found Application Experience service interference at the end of last year, but unfortunately that service cannot be stopped in Windows. And I am also wondering whether we need clarify the Windows special behaviour in our spec for the file/directory delete methods. For example, in File.delete(), it currently says, 1019 * @return codetrue/code if and only if the file or directory is 1020 * successfully deleted; codefalse/code otherwise Thanks! -Dan
Re: RFR: JDK-4792059 -- test/java/io/pathNames/GeneralSolaris.java fails on symbolic links
Thank you, Alan. I have updated my webrev to http://cr.openjdk.java.net/~dxu/4792059/webrev.01/. -Dan On 08/28/2013 04:05 AM, Alan Bateman wrote: On 28/08/2013 00:20, Dan Xu wrote: Hi, When GeneralSolaris testcase follows symbolic link to pick up an existing file or directory for testing, it will fail the assertion in check()method because the file path canonicalization process will result in the real path not a path containing symbolic link. I enforce this test not to use any symbolic link as a testing file ordirectory to avoid this problem. Please help review my fix. Thanks! bug: https://bugs.openjdk.java.net/browse/JDK-4792059 webrev: http://cr.openjdk.java.net/~dxu/4792059/webrev/ -Dan You can drop the exists check as Files.is can't return true if the file doesn't exist. Otherwise looks okay to me. -Alan.
Re: RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load
On 08/27/2013 07:15 AM, Dan Xu wrote: On 08/27/2013 12:12 AM, Alan Bateman wrote: On 27/08/2013 01:18, Dan Xu wrote: Hi All, MaxPathLength.javais a troublesome testcase, and fails intermittently in the nightly test. And it also runs for a long time, especially on Windows platforms. Inorder to improve the test stability, I remove its unnecessary test iterations, and use NIOdelete method todo the clean-up to make the potential failureseasier for diagnosis. Please review thechanges. Thanks! bug: https://bugs.openjdk.java.net/browse/JDK-8023765 webrev: http://cr.openjdk.java.net/~dxu/8023765/webrev/ -Dan The double to quickly skip over the names to MAX_LENGTH/2 looks reasonable. I guess fileCreated should really be fileExists as it may be deleted and then deleted. An alternative here would be to use Files.deleteIfExists as that would avoid the need to introduce flags to track whether the directory and exist exists. -Alan Hi Alan, Those flag names are a little misleading. Sorry about that. fileCreated and dirCreated are actually tracking the existence of new file and directories. If the new file gets deleted, I marked the flag to false in the code. And at the end, I also change the recorded file path after the rename operation. I agree that using deleteIfExists is a good alternative. In my original thought, I plan to monitor every step and make sure all file operations happen as expected. Thanks! -Dan Stuart brought a very good point for this test failure in the main bug, JDK-7160013. According to his comment, Bottom line is that a Windows daemon (not an anti-virus scanner) may hold a file in delete-pending state for a short amount of time after it's been deleted. While it's in this state, if you ask if the file exists, the system will say that it does not. However, if you try to create a new file with the same name, you get an access denied error. And I re-checked remarks in the Windows APIs used for deleting file in [1] and directory in [2]. According to the spec, these APIs only mark a file/directory for deletion on close. And the delete operation does not occur until the last handle to it is closed. Therefore, if a Windows daemon opens a handle towards those newly-created directories (I think anti-virus on-access scanner may do a similar thing), the delete operation will be put in delete-pending state, and the next immediate call to create the same directory structure will fail. Based on the above information, I updated this testcase to make it create different directory structure in each loop, so that the next-create operation will not conflict with previous pending delete operation. I also follow Alan's suggestion to use Files.deleteIfExists in the latest change. Please review it at http://cr.openjdk.java.net/~dxu/8023765/webrev.01/. Thanks! -Dan [1] http://msdn.microsoft.com/en-us/library/windows/desktop/aa363915%28v=vs.85%29.aspx [2] http://msdn.microsoft.com/en-us/library/windows/desktop/aa365488%28v=vs.85%29.aspx
Re: RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load
On 08/27/2013 12:12 AM, Alan Bateman wrote: On 27/08/2013 01:18, Dan Xu wrote: Hi All, MaxPathLength.javais a troublesome testcase, and fails intermittently in the nightly test. And it also runs for a long time, especially on Windows platforms. Inorder to improve the test stability, I remove its unnecessary test iterations, and use NIOdelete method todo the clean-up to make the potential failureseasier for diagnosis. Please review thechanges. Thanks! bug: https://bugs.openjdk.java.net/browse/JDK-8023765 webrev: http://cr.openjdk.java.net/~dxu/8023765/webrev/ -Dan The double to quickly skip over the names to MAX_LENGTH/2 looks reasonable. I guess fileCreated should really be fileExists as it may be deleted and then deleted. An alternative here would be to use Files.deleteIfExists as that would avoid the need to introduce flags to track whether the directory and exist exists. -Alan Hi Alan, Those flag names are a little misleading. Sorry about that. fileCreated and dirCreated are actually tracking the existence of new file and directories. If the new file gets deleted, I marked the flag to false in the code. And at the end, I also change the recorded file path after the rename operation. I agree that using deleteIfExists is a good alternative. In my original thought, I plan to monitor every step and make sure all file operations happen as expected. Thanks! -Dan
RFR: JDK-4792059 -- test/java/io/pathNames/GeneralSolaris.java fails on symbolic links
Hi, When GeneralSolaris testcase follows symbolic link to pick up an existing file or directory for testing, it will fail the assertion in check()method because the file path canonicalization process will result in the real path not a path containing symbolic link. I enforce this test not to use any symbolic link as a testing file ordirectory to avoid this problem. Please help review my fix. Thanks! bug: https://bugs.openjdk.java.net/browse/JDK-4792059 webrev: http://cr.openjdk.java.net/~dxu/4792059/webrev/ -Dan
RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load
Hi All, MaxPathLength.javais a troublesome testcase, and fails intermittently in the nightly test. And it also runs for a long time, especially on Windows platforms. Inorder to improve the test stability, I remove its unnecessary test iterations, and use NIOdelete method todo the clean-up to make the potential failureseasier for diagnosis. Please review thechanges. Thanks! bug: https://bugs.openjdk.java.net/browse/JDK-8023765 webrev: http://cr.openjdk.java.net/~dxu/8023765/webrev/ -Dan
hg: jdk8/tl/jdk: 8023430: Replace File.mkdirs with Files.createDirectories to get MaxPathLength.java failure details
Changeset: 8a7d9cc2f41c Author:dxu Date: 2013-08-22 11:43 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/8a7d9cc2f41c 8023430: Replace File.mkdirs with Files.createDirectories to get MaxPathLength.java failure details Reviewed-by: alanb ! test/ProblemList.txt ! test/java/io/File/MaxPathLength.java
Re: RFR: JDK-8023430 - Replace File.mkdirs with Files.createDirectories to get MaxPathLength.java failure details
On 08/20/2013 11:18 PM, Alan Bateman wrote: On 21/08/2013 01:04, Dan Xu wrote: Hi, MaxPathLength.java testcase failed intermittently. And File.mkdirs() does not throw any exceptions when it fails, which makes it even harder for the diagnosis. As Alan suggested, I'd like to change it to Files.createDirectories to get detailed exceptions once a failure happens. Thanks! webrev: http://cr.openjdk.java.net/~dxu/8023430/webrev/ bug: http://bugs.sun.com/view_bug.do?bug_id=8023430 I think this is the right thing to do to find out what is going on. With the patch then you can remove couldMakeTestDirectory as the exists check and throwing the exception is no longer needed. -Alan. Thanks, Alan. I have updated my webrev at, http://cr.openjdk.java.net/~dxu/8023430/webrev.01/. Please take a look! -Dan
RFR: JDK-8023430 - Replace File.mkdirs with Files.createDirectories to get MaxPathLength.java failure details
Hi, MaxPathLength.java testcase failed intermittently. And File.mkdirs() does not throw any exceptions when it fails, which makes it even harder for the diagnosis. As Alan suggested, I'd like to change it to Files.createDirectories to get detailed exceptions once a failure happens. Thanks! webrev: http://cr.openjdk.java.net/~dxu/8023430/webrev/ bug: http://bugs.sun.com/view_bug.do?bug_id=8023430 -Dan
hg: jdk8/tl/jdk: 8023203: Wrap RandomAccessFile.seek native method into a Java helper method
Changeset: 2fd841fccb2e Author:dxu Date: 2013-08-19 12:38 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2fd841fccb2e 8023203: Wrap RandomAccessFile.seek native method into a Java helper method Reviewed-by: alanb, chegar ! make/java/java/mapfile-vers ! makefiles/mapfiles/libjava/mapfile-vers ! src/share/classes/java/io/RandomAccessFile.java ! src/share/native/java/io/RandomAccessFile.c
RFR: JDK-8023203 - Wrap RandomAccessFile.seek native method into a Java helper method
HiAll, Please review the simple change for JDK-8023203 - Wrap RandomAccessFile.seek native method into a Java helper method. Itadds a helper java method and makes it the only place that can invoke the native method directly. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8023203/webrev/ Bug: http://bugs.sun.com/view_bug.do?bug_id=8023203 -Dan
Re: RFR [6883354] File.mkdirs() method doesn't behave well when given /../
On 08/16/2013 03:40 AM, Alan Bateman wrote: On 12/08/2013 17:02, Ivan Gerasimov wrote: Hello everybody! Would you please help review a small change to File.mkdirs() method? The current implementation of the method, when past an argument dir1/../dir2 only tries to create dir2. mkdir -p 'dir1/../dir2' command on Linux creates both dir1 and dir2. java.nio.file.Files.createDirectories() also creates both dir1 and dir2. The proposed fix makes File.mkdirs() method to behave in the same way on all the platforms except for Windows. The problem with Windows is that it reports 'dir1/..' as existent even if dir1 does not exist. Because of that Files.createDirectories() doesn't work this way on Windows either. Proposed fix: - makes File.mkdirs() match the behavior of 'mkdir -p' on Linux, Solaris and MacOS, and - doesn't change its behavior on Windows. http://cr.openjdk.java.net/~igerasim/6883354/0/webrev/ This is old code so we have to be cautious about changing it (and understanding why it used the canonical path in the first place). What would you think about adding a few tests to cover more types of file path, Windows drive-relative and directory-relative paths in particular as these are cases where we would need to confident that it doesn't break. As an aside, Dan Xu and I were chatting recently about just replacing most of the java.io.File implementation to just use java.nio.file. This specific one is a case where it might be easier to just change the method to Files.createDirectories(toPath()). -Alan. Hi Ivan, As you mentioned, the code behaves differently on Windows platform, so you cannot use the same test for all platforms. Your proposed Mkdir.java will fail on windows. Thanks! -Dan
hg: jdk8/tl/jdk: 4858457: File.getCanonicalPath() throws IOException when invoked with nul (win)
Changeset: 5bb196aa183c Author:dxu Date: 2013-08-15 12:36 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5bb196aa183c 4858457: File.getCanonicalPath() throws IOException when invoked with nul (win) Reviewed-by: alanb ! src/windows/native/java/io/canonicalize_md.c ! test/java/io/File/WinDeviceName.java
hg: jdk8/tl/jdk: 8017109: Cleanup overrides warning in src/solaris/classes/sun/print/AttributeClass.java
Changeset: 0d27309a87e0 Author:dxu Date: 2013-08-15 14:11 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0d27309a87e0 8017109: Cleanup overrides warning in src/solaris/classes/sun/print/AttributeClass.java Reviewed-by: jgodinez ! src/solaris/classes/sun/print/AttributeClass.java
Re: RFR: JDK-8022178: System.console() throws IOE on some Windows
Looks good to me. -Dan On 08/14/2013 11:35 AM, Alan Bateman wrote: On 14/08/2013 19:35, Xueming Shen wrote: Hi, Please help review the trivial change for 8022178. http://cr.openjdk.java.net/~sherman/console/webrev System.console() is not specified to throw an IOE. It is supposed to return a null silently if there is no system console or anything goes wrong to get one. The Windows implementation obviously has some leftover code from the old/original implementation to throw a IOE if it fails to get the std in/out handler in certain use scenario. The proposed change is to simply remove the IOE throwing code and return silently. It does look like left over code, the change looks fine to me. -Alan
Re: RFR: JDK-4858457 -- File.getCanonicalPath() throws IOException when invoked with nul (win)
Hi Alan, Thanks for pointing out. I have updated my fix in the new webrev, http://cr.openjdk.java.net/~dxu/4858457/webrev.01/. Please take a look! -Dan On 08/14/2013 08:22 AM, Alan Bateman wrote: On 08/08/2013 21:44, Dan Xu wrote: Hi All, Windows platforms have reserved a few names for device usages, and nul is one of them. And Windows API, such as _wfullpath() and GetFullPathNameW, returns its full path with the prefix \\.\, which represents win32 device namespaces. For example, nul, whose full path name is \\.\nul, is a valid device name. But our current Canonicalization logicfails to generate its canonicalpath and throws an IOException. This fix is going to address the problem. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/4858457/webrev.00/ Bug: http://bugs.sun.com/view_bug.do?bug_id=4858457 -Dan The change looks okay to me, I just wonder if it might make sense to combine the test with WinDeviceName so that there is only one set of inputs to test. -Alan
Re: RFR: JDK-8021977 -- Open File Using Java.IO May Throw IOException On Windows
Thank you. I will use the updated synopsis for my pushtoday. -Dan On 08/09/2013 08:41 AM, mark.reinh...@oracle.com wrote: 2013/8/8 9:26 -0700, dan...@oracle.com: Webrev: http://cr.openjdk.java.net/~dxu/8021977/webrev.00/ Bug: http://bugs.sun.com/view_bug.do?bug_id=8021977 Looks good, but the synopsis is rather horrid (Java.IO?). Please use the updated synopsis I just pasted into the bug report: Opening a file using java.io can throw IOException on Windows. - Mark
hg: jdk8/tl/jdk: 8021977: Opening a file using java.io can throw IOException on Windows
Changeset: 03822f2389bf Author:dxu Date: 2013-08-09 10:55 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/03822f2389bf 8021977: Opening a file using java.io can throw IOException on Windows Summary: Remove IOException related error-handling code for backward compatibility Reviewed-by: alanb, lancea, mr ! src/windows/native/java/io/io_util_md.c
RFR: JDK-4858457 -- File.getCanonicalPath() throws IOException when invoked with nul (win)
Hi All, Windows platforms have reserved a few names for device usages, and nul is one of them. And Windows API, such as _wfullpath() and GetFullPathNameW, returns its full path with the prefix \\.\, which represents win32 device namespaces. For example, nul, whose full path name is \\.\nul, is a valid device name. But our current Canonicalization logicfails to generate its canonicalpath and throws an IOException. This fix is going to address the problem. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/4858457/webrev.00/ Bug: http://bugs.sun.com/view_bug.do?bug_id=4858457 -Dan
RFR: JDK-8021977 -- Open File Using Java.IO May Throw IOException On Windows
Hi All, Please review a simple bug fix for JDK8021977. For the backward compatibility, I have to remove the code that might throw out IOExceptionin the native side. The issue has never been reported.But it exists in a possible code path. And it is not easy to write a testcase for this obvious but corner issue. I am relying on the existing IO testcases for testing. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8021977/webrev.00/ Bug: http://bugs.sun.com/view_bug.do?bug_id=8021977 -Dan
RFR: JDK-8022554 -- Fix Warnings in sun.invoke.anon Package
Hi All, Please review the simple warning fix in src/share/classes/sun/invoke/anon/ConstantPoolPatch.java. webrev: http://cr.openjdk.java.net/~dxu/8022554/webrev/ Thanks, -Dan
Re: RFR: JDK-8022554 -- Fix Warnings in sun.invoke.anon Package
Thanks for your review!
I was thinking of that. But without Class?[] on the left, the compiler
just worked fine.
Here is a simple example,
//Main.java
import java.util.*;
public class Main {
public static final Class[] TEST_CLASS = new Class?[16];
public static final List[] TEST_MAP = new ArrayList?[16];
}
After compiling with javac Xlint:all Main.java, no warnings are printed
out. Is it a compiler issue? Thanks!
-Dan
On 08/07/2013 11:11 AM, Joe Darcy wrote:
I agree with Mike; Class?[] should be used on both sides.
Cheers,
-Joe
On 08/07/2013 10:53 AM, Mike Duigou wrote:
Why not have Class?[] on the left side as well?
Mike
On Aug 7 2013, at 10:49 , Dan Xu wrote:
Hi All,
Please review the simple warning fix in
src/share/classes/sun/invoke/anon/ConstantPoolPatch.java.
webrev: http://cr.openjdk.java.net/~dxu/8022554/webrev/
Thanks,
-Dan
Re: RFR: JDK-8022554 -- Fix Warnings in sun.invoke.anon Package
I see, Thanks! I have updated my changeto
http://cr.openjdk.java.net/~dxu/8022554/webrev1/.
-Dan
On 08/07/2013 11:46 AM, Joe Darcy wrote:
Hi Dan,
Even if the compiler does not complain, using Class or Class[] is
using a raw type and raw types should generally be viewed as
unacceptable in modern code.
Cheers,
-Joe
On 08/07/2013 11:36 AM, Dan Xu wrote:
Thanks for your review!
I was thinking of that. But without Class?[] on the left, the
compiler just worked fine.
Here is a simple example,
//Main.java
import java.util.*;
public class Main {
public static final Class[] TEST_CLASS = new Class?[16];
public static final List[] TEST_MAP = new ArrayList?[16];
}
After compiling with javac Xlint:all Main.java, no warnings are
printed out. Is it a compiler issue? Thanks!
-Dan
On 08/07/2013 11:11 AM, Joe Darcy wrote:
I agree with Mike; Class?[] should be used on both sides.
Cheers,
-Joe
On 08/07/2013 10:53 AM, Mike Duigou wrote:
Why not have Class?[] on the left side as well?
Mike
On Aug 7 2013, at 10:49 , Dan Xu wrote:
Hi All,
Please review the simple warning fix in
src/share/classes/sun/invoke/anon/ConstantPoolPatch.java.
webrev: http://cr.openjdk.java.net/~dxu/8022554/webrev/
Thanks,
-Dan
Re: RFR: JDK-8022554 -- Fix Warnings in sun.invoke.anon Package
Thank you for your review!
-Dan
On 08/07/2013 12:08 PM, Remi Forax wrote:
On 08/07/2013 08:59 PM, Joe Darcy wrote:
Amended version approved to go back.
Thanks,
-Joe
As one of the guys involved in the design of this API :)
I'm Ok with this change.
Rémi
On 08/07/2013 11:54 AM, Dan Xu wrote:
I see, Thanks! I have updated my changeto
http://cr.openjdk.java.net/~dxu/8022554/webrev1/.
-Dan
On 08/07/2013 11:46 AM, Joe Darcy wrote:
Hi Dan,
Even if the compiler does not complain, using Class or Class[]
is using a raw type and raw types should generally be viewed as
unacceptable in modern code.
Cheers,
-Joe
On 08/07/2013 11:36 AM, Dan Xu wrote:
Thanks for your review!
I was thinking of that. But without Class?[] on the left, the
compiler just worked fine.
Here is a simple example,
//Main.java
import java.util.*;
public class Main {
public static final Class[] TEST_CLASS = new Class?[16];
public static final List[] TEST_MAP = new ArrayList?[16];
}
After compiling with javac Xlint:all Main.java, no warnings are
printed out. Is it a compiler issue? Thanks!
-Dan
On 08/07/2013 11:11 AM, Joe Darcy wrote:
I agree with Mike; Class?[] should be used on both sides.
Cheers,
-Joe
On 08/07/2013 10:53 AM, Mike Duigou wrote:
Why not have Class?[] on the left side as well?
Mike
On Aug 7 2013, at 10:49 , Dan Xu wrote:
Hi All,
Please review the simple warning fix in
src/share/classes/sun/invoke/anon/ConstantPoolPatch.java.
webrev: http://cr.openjdk.java.net/~dxu/8022554/webrev/
Thanks,
-Dan
hg: jdk8/tl/jdk: 8022554: Fix Warnings in sun.invoke.anon Package
Changeset: d1c82d5bee3f Author:dxu Date: 2013-08-07 12:13 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d1c82d5bee3f 8022554: Fix Warnings in sun.invoke.anon Package Reviewed-by: darcy, mduigou, lancea ! src/share/classes/sun/invoke/anon/ConstantPoolPatch.java
hg: jdk8/tl/jdk: 8022410: Fix Javac Warnings in com.sun.security.auth Package
Changeset: 4b8b811059db Author:dxu Date: 2013-08-06 14:33 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4b8b811059db 8022410: Fix Javac Warnings in com.sun.security.auth Package Reviewed-by: darcy ! src/share/classes/com/sun/security/auth/PolicyFile.java ! src/share/classes/com/sun/security/auth/SubjectCodeSource.java
hg: jdk8/tl/jdk: 8022478: Fix Warnings In sun.net.www.protocol.http Package
Changeset: 1d21ff5c2b3f Author:dxu Date: 2013-08-06 18:16 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1d21ff5c2b3f 8022478: Fix Warnings In sun.net.www.protocol.http Package Reviewed-by: darcy ! src/share/classes/sun/net/www/protocol/http/AuthCacheValue.java ! src/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java
Re: Review request: JDK-8016579 (process) IOException thrown by ProcessBuilder.start() method is incorrectly encoded (v.3)
I see. Thanks for your good explanation. The change looks good to me. -Dan On 07/18/2013 12:56 AM, Alexey Utkin wrote: On 7/17/2013 10:29 PM, Dan Xu wrote: I don't know the difference between (*env)-NewStringUTF(env, buf) and JNU_NewStringPlatform(env, buf). Does the JNU_NewStringPlatform() function currently fail to process non-western locale? According to the code, JNU_NewStringPlatform() is also trying to use the right encoding for the give string. JNU_NewStringPlatform() is using the String(byte[] bytes) constructor. Javadoc says: Constructs a new |String| by decoding the specified array of bytes using the platform's default charset ... The behavior of this constructor when the given bytes are not valid in the default charset is unspecified. In the most of cases the code page is ISO-8859-1 by hysterical reasons (Win9x/WinMe) of user does not change the Java system property. Currently in Java for Windows (in libraries and JVM, but not in AWT - I Artem fixed it there) we got stupid decoding pipe: win32_UTF16(aka Windows Unicode)(JNI - 16bit WCHAR) - ISO-8859-1(JVM - 8bit char) - UTF16(Java - 16bit jchar). It can be compared with TV stream HDTV-MPG4-HDTV. Instead, (*env)-NewStringUTF(env, buf) call goes by pipe win32_UTF16(aka Windows Unicode)(JNI) - UTF8 (JVM multibyte 8bit!)- UTF16(Java) without information loss and has no dependance from user's fantasy about system locale. Regards, -uta Other changes look good to me. Thanks for fixing getLastErrorString function in io_util_md.c file. -Dan On 07/17/2013 08:28 AM, Alexey Utkin wrote: Thanks, Here is new version with Dan's correction: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.02/ Regards, -uta On 7/16/2013 11:07 PM, Dan Xu wrote: I agree. Is jdk_util.c or jni_util.c or any file under native/common/ a good place? The fix looks good to me. One small comment is to get the return value of swprintf() in win32Error(). If the return value is -1, an error situation needs to be handled even though it seems very rare. If it is not -1, then get the actual length of utf16_javaMessage. And pass the length of utf16_javaMessage to WideCharToMultiByte() instead of MESSAGE_LENGTH to make the code more efficient. Thanks! Accepted. -Dan On 07/16/2013 10:49 AM, Martin Buchholz wrote: Looks OK to me. As ever, we continue to not have good places within the JDK itself for C-level infrastructure - win32Error is not specific to the process api, and should really be pulled into some common directory - but I don't know of any such. On Tue, Jul 16, 2013 at 9:04 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Here is new version of fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.01/ On 7/15/2013 9:08 PM, Martin Buchholz wrote: Superficial review: Looks good mostly. Historically, switching windows code to use W APIs has been a big TODO, but was waiting for Win98 de-support. In java.lang we have passed the half-way: the error reporting sub-system is in the ASCII world. Please spell correctly: MESAGE_LENGTH Thanks, I missed it. Fixed. If errno and GetLastError are two separate error notification systems, how do you know which one corresponded to the last failure? E.g. if the last failure only set errno, won't the error message be via GetLastError(), which is likely to be stale? As Dan mentioned, the os_lasterror was a copy of JVM os::lasterror call. The error message procedure is used for CreatePipe CreateProcessW GetExitCodeProcess WaitForMultipleObjects fail report. You are right, all the calls return the problem by GetLastMessage call. The function is changed (reduced). Here is new version of fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.01/ Regards, -uta On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Bug description: https://jbs.oracle.com/bugs/browse/JDK-8016579 http://bugs.sun.com/view_bug.do?bug_id=8016579 Here is the suggested fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.00/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.00/ Summary: We have THREE locales in action: 1. Thread default locale - dictates UNICODE-to-8bit conversion 2. OS locale that defines the message localization 3. The file name locale Each locale could be an extended locale, that means that text cannot be mapped to 8bit sequence without multibyte encoding. VM is ready for that, if text is UTF-8
Re: Review request: JDK-8016579 (process) IOException thrown by ProcessBuilder.start() method is incorrectly encoded (v.3)
I don't know the difference between (*env)-NewStringUTF(env, buf) and JNU_NewStringPlatform(env, buf). Does the JNU_NewStringPlatform() function currently fail to process non-western locale? According to the code, JNU_NewStringPlatform() is also trying to use the right encoding for the give string. Other changes look good to me. Thanks for fixing getLastErrorString function in io_util_md.c file. -Dan On 07/17/2013 08:28 AM, Alexey Utkin wrote: Thanks, Here is new version with Dan's correction: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.02/ Regards, -uta On 7/16/2013 11:07 PM, Dan Xu wrote: I agree. Is jdk_util.c or jni_util.c or any file under native/common/ a good place? The fix looks good to me. One small comment is to get the return value of swprintf() in win32Error(). If the return value is -1, an error situation needs to be handled even though it seems very rare. If it is not -1, then get the actual length of utf16_javaMessage. And pass the length of utf16_javaMessage to WideCharToMultiByte() instead of MESSAGE_LENGTH to make the code more efficient. Thanks! Accepted. -Dan On 07/16/2013 10:49 AM, Martin Buchholz wrote: Looks OK to me. As ever, we continue to not have good places within the JDK itself for C-level infrastructure - win32Error is not specific to the process api, and should really be pulled into some common directory - but I don't know of any such. On Tue, Jul 16, 2013 at 9:04 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Here is new version of fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.01/ On 7/15/2013 9:08 PM, Martin Buchholz wrote: Superficial review: Looks good mostly. Historically, switching windows code to use W APIs has been a big TODO, but was waiting for Win98 de-support. In java.lang we have passed the half-way: the error reporting sub-system is in the ASCII world. Please spell correctly: MESAGE_LENGTH Thanks, I missed it. Fixed. If errno and GetLastError are two separate error notification systems, how do you know which one corresponded to the last failure? E.g. if the last failure only set errno, won't the error message be via GetLastError(), which is likely to be stale? As Dan mentioned, the os_lasterror was a copy of JVM os::lasterror call. The error message procedure is used for CreatePipe CreateProcessW GetExitCodeProcess WaitForMultipleObjects fail report. You are right, all the calls return the problem by GetLastMessage call. The function is changed (reduced). Here is new version of fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.01/ Regards, -uta On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Bug description: https://jbs.oracle.com/bugs/browse/JDK-8016579 http://bugs.sun.com/view_bug.do?bug_id=8016579 Here is the suggested fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.00/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.00/ Summary: We have THREE locales in action: 1. Thread default locale - dictates UNICODE-to-8bit conversion 2. OS locale that defines the message localization 3. The file name locale Each locale could be an extended locale, that means that text cannot be mapped to 8bit sequence without multibyte encoding. VM is ready for that, if text is UTF-8. The suggested fix does the work right from the beginning. Unicode version of JVM call: hotspot/src/os/windows/vm/os_windows.cpp: size_t os::lasterror(char* buf, size_t len) was used as prototype for Unicode error message getter. It has to be fixed accordingly as well as jdk/src/windows/native/java/io/io_util_md.c size_t getLastErrorString(char *buf, size_t len) The bug contains the attachment https://jbs.oracle.com/bugs/secure/attachment/14581/JDK-8016579.txt that summarize the fix result in comparison with original implementation. Regards, -uta
Re: Review request: JDK-8016579 (process) IOException thrown by ProcessBuilder.start() method is incorrectly encoded (v.2)
I agree. Is jdk_util.c or jni_util.c or any file under native/common/ a good place? The fix looks good to me. One small comment is to get the return value of swprintf() in win32Error(). If the return value is -1, an error situation needs to be handled even though it seems very rare. If it is not -1, then get the actual length of utf16_javaMessage. And pass the length of utf16_javaMessage to WideCharToMultiByte() instead of MESSAGE_LENGTH to make the code more efficient. Thanks! -Dan On 07/16/2013 10:49 AM, Martin Buchholz wrote: Looks OK to me. As ever, we continue to not have good places within the JDK itself for C-level infrastructure - win32Error is not specific to the process api, and should really be pulled into some common directory - but I don't know of any such. On Tue, Jul 16, 2013 at 9:04 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Here is new version of fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.01/ On 7/15/2013 9:08 PM, Martin Buchholz wrote: Superficial review: Looks good mostly. Historically, switching windows code to use W APIs has been a big TODO, but was waiting for Win98 de-support. In java.lang we have passed the half-way: the error reporting sub-system is in the ASCII world. Please spell correctly: MESAGE_LENGTH Thanks, I missed it. Fixed. If errno and GetLastError are two separate error notification systems, how do you know which one corresponded to the last failure? E.g. if the last failure only set errno, won't the error message be via GetLastError(), which is likely to be stale? As Dan mentioned, the os_lasterror was a copy of JVM os::lasterror call. The error message procedure is used for CreatePipe CreateProcessW GetExitCodeProcess WaitForMultipleObjects fail report. You are right, all the calls return the problem by GetLastMessage call. The function is changed (reduced). Here is new version of fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.01/ Regards, -uta On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Bug description: https://jbs.oracle.com/bugs/browse/JDK-8016579 http://bugs.sun.com/view_bug.do?bug_id=8016579 Here is the suggested fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.00/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.00/ Summary: We have THREE locales in action: 1. Thread default locale - dictates UNICODE-to-8bit conversion 2. OS locale that defines the message localization 3. The file name locale Each locale could be an extended locale, that means that text cannot be mapped to 8bit sequence without multibyte encoding. VM is ready for that, if text is UTF-8. The suggested fix does the work right from the beginning. Unicode version of JVM call: hotspot/src/os/windows/vm/os_windows.cpp: size_t os::lasterror(char* buf, size_t len) was used as prototype for Unicode error message getter. It has to be fixed accordingly as well as jdk/src/windows/native/java/io/io_util_md.c size_t getLastErrorString(char *buf, size_t len) The bug contains the attachment https://jbs.oracle.com/bugs/secure/attachment/14581/JDK-8016579.txt that summarize the fix result in comparison with original implementation. Regards, -uta
Re: Review request: JDK-8016579 (process) IOException thrown by ProcessBuilder.start() method is incorrectly encoded
I guess Alexey made the changes based on the existing implementation of JVM_GetLastErrorString() in hotspot, which is os::lasterror(char* buf, size_t len) in os_windows.cpp. One comment about the code at line 105 in win32Error(). 105 const int errnum = GetLastError(); Since in os_lasterror() function, both GetLastError() and errno are used conditionally to get the error code, will the same logic used here to get the value for errnum? Otherwise, it is possible that an error code and message are wrongly mapped and printed out. I need to point out that this issue is not brought up by this change, and also exists in the old code. Thanks! -Dan On 07/15/2013 10:08 AM, Martin Buchholz wrote: Superficial review: Looks good mostly. Historically, switching windows code to use W APIs has been a big TODO, but was waiting for Win98 de-support. Please spell correctly: MESAGE_LENGTH If errno and GetLastError are two separate error notification systems, how do you know which one corresponded to the last failure? E.g. if the last failure only set errno, won't the error message be via GetLastError(), which is likely to be stale? On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin alexey.ut...@oracle.com mailto:alexey.ut...@oracle.com wrote: Bug description: https://jbs.oracle.com/bugs/browse/JDK-8016579 http://bugs.sun.com/view_bug.do?bug_id=8016579 Here is the suggested fix: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.00/ http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.00/ Summary: We have THREE locales in action: 1. Thread default locale - dictates UNICODE-to-8bit conversion 2. OS locale that defines the message localization 3. The file name locale Each locale could be an extended locale, that means that text cannot be mapped to 8bit sequence without multibyte encoding. VM is ready for that, if text is UTF-8. The suggested fix does the work right from the beginning. Unicode version of JVM call: hotspot/src/os/windows/vm/os_windows.cpp: size_t os::lasterror(char* buf, size_t len) was used as prototype for Unicode error message getter. It has to be fixed accordingly as well as jdk/src/windows/native/java/io/io_util_md.c size_t getLastErrorString(char *buf, size_t len) The bug contains the attachment https://jbs.oracle.com/bugs/secure/attachment/14581/JDK-8016579.txt that summarize the fix result in comparison with original implementation. Regards, -uta
hg: jdk8/tl/jdk: 8017212: File.createTempFile requires unnecessary read permission
Changeset: 10d2a4b1e576 Author:dxu Date: 2013-07-11 13:40 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/10d2a4b1e576 8017212: File.createTempFile requires unnecessary read permission Summary: Directly call FileSystem method to check a file existence. Also reviewed by tom.haw...@oracle.com Reviewed-by: alanb ! src/share/classes/java/io/File.java + test/java/io/File/CheckPermission.java ! test/java/io/File/NulFile.java ! test/java/io/File/createTempFile/SpecialTempFile.java
Re: Build error with GCC4.8 on Fedora19
Adding build-dev mailing list. On 07/08/2013 09:54 PM, Yasu wrote: Sorry, I forgot to attach a patch: -- diff -r 52454985425d makefiles/CompileNativeLibraries.gmk --- a/makefiles/CompileNativeLibraries.gmk Mon Jul 08 19:24:22 2013 -0700 +++ b/makefiles/CompileNativeLibraries.gmk Tue Jul 09 13:41:12 2013 +0900 @@ -1987,7 +1987,7 @@ ifneq ($(OPENJDK_TARGET_OS),macosx) - SCTP_WERROR := -Werror + SCTP_WERROR := -Werror -Wno-error=unused-parameter ifeq ($(OPENJDK_TARGET_CPU_ARCH), ppc) SCTP_WERROR := endif
hg: jdk8/tl/jdk: 6469160: (fmt) general (%g) formatting of zero (0.0) with precision 0 or 1 throws ArrayOutOfBoundsException
Changeset: eabcb85fcabc Author:bpb Date: 2013-06-24 14:17 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/eabcb85fcabc 6469160: (fmt) general (%g) formatting of zero (0.0) with precision 0 or 1 throws ArrayOutOfBoundsException Summary: For zero value ensure than an unpadded zero character is passed to Formatter.addZeros() Reviewed-by: iris, darcy Contributed-by: Brian Burkhalter brian.burkhal...@oracle.com ! src/share/classes/java/util/Formatter.java ! src/share/classes/sun/misc/FloatingDecimal.java ! test/java/util/Formatter/Basic-X.java.template ! test/java/util/Formatter/Basic.java ! test/java/util/Formatter/BasicBigDecimal.java ! test/java/util/Formatter/BasicDouble.java ! test/java/util/Formatter/BasicDoubleObject.java ! test/java/util/Formatter/BasicFloat.java ! test/java/util/Formatter/BasicFloatObject.java
hg: jdk8/tl/jdk: 8016592: Clean-up Javac Overrides Warnings In javax/management/NotificationBroadcasterSupport.java
Changeset: f6d72c4f6bf1 Author:dxu Date: 2013-06-19 13:00 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f6d72c4f6bf1 8016592: Clean-up Javac Overrides Warnings In javax/management/NotificationBroadcasterSupport.java Summary: Add hashCode() methods to ListenerInfo and WildcardListenerInfo classes Reviewed-by: dfuchs, alanb, sjiang, chegar ! src/share/classes/javax/management/NotificationBroadcasterSupport.java
Re: RFR: JDK-8013827 and JDK-8011950, , java.io.File.createTempFile enters infinite loop when passed invalid data
On 06/03/2013 03:23 AM, Alan Bateman wrote: On 02/06/2013 03:16, Dan Xu wrote: : As for the SpecialTempFile testcase, I wonder whether these regression tests may happen to be used to test earlier JDK versions. In that case, the thread pool will help the test framework find the test failure easily. Otherwise, I agree it adds extra overhead into this test. My concern is mostly the timeout as we've had so many problems with tests failing intermittently when the machine is very busy. In this case, I could imagine this failing when there are tests running concurrently in a pool VMs and at the same time competing with AV software on Windows that is sucking the life out of the system. In general, it's not always possible to write a test that behaves well when run an unpatched-JDK. Deadlocks, crashes, and hangs are just some examples where a test might timeout or jtreg needs to spin up a new VM to continue the test execution. So while important for one-off verification then it's probably not worth spending too much effort trying to get it to behaves well on an JDK that doesn't have the fix. -Alan. Hi Alan, Thank you for the clarification. I have updated the test and if format at webrev, http://cr.openjdk.java.net/~dxu/8013827/webrev.02/. Please review it. -Dan
hg: jdk8/tl/jdk: 8013827: File.createTempFile hangs with temp file starting with 'com1.4'; ...
Changeset: 4a66dd1d7eea Author:dxu Date: 2013-06-10 11:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4a66dd1d7eea 8013827: File.createTempFile hangs with temp file starting with 'com1.4' 8011950: java.io.File.createTempFile enters infinite loop when passed invalid data Reviewed-by: alanb ! src/share/classes/java/io/File.java ! src/windows/native/java/io/WinNTFileSystem_md.c ! test/java/io/File/CreateNewFile.java ! test/java/io/File/NulFile.java + test/java/io/File/createTempFile/SpecialTempFile.java
Re: RFR: JDK-8013827 and JDK-8011950, , java.io.File.createTempFile enters infinite loop when passed invalid data
On 06/01/2013 03:05 AM, Alan Bateman wrote: On 31/05/2013 19:15, Dan Xu wrote: On 05/29/2013 04:54 AM, Alan Bateman wrote: Thanks for taking this one. The changes mean that IAE is now thrown for cases where it wasn't thrown previously and I'm wondering if this is worth doing. It might be simpler to just throw IOException on the grounds that creating the file will fail. One other thing to consider is that \ is a valid name of a file on some platforms. I suspect you'll need to delegate to the FileSystem to check the suffix/prefix. Alternatively in TempDirectory.generateFile you could create a File from prefix/n/suffix and call getName to check that its matches. If not then throw an exception. -Alan. Thanks for your good review. I have updated my changes. And it is uploaded at, http://cr.openjdk.java.net/~dxu/8013827/webrev.01/. This updated changes look good, minor nit in that the missing space in two occurrences of if(. For the test SpecialTempFile then I wonder if the 2s is sufficient, I could imagine this test failing intermittently when under load. Is a thread pool even needed because with the fix then createFileTemp should either work or throw an IOException (no hang). -Alan Thanks, Alan. I am going to fix the format issue on if(. As for the SpecialTempFile testcase, I wonder whether these regression tests may happen to be used to test earlier JDK versions. In that case, the thread pool will help the test framework find the test failure easily. Otherwise, I agree it adds extra overhead into this test. -Dan
Re: RFR: JDK-8013827 and JDK-8011950, , java.io.File.createTempFile enters infinite loop when passed invalid data
On 05/29/2013 04:54 AM, Alan Bateman wrote: On 28/05/2013 19:39, Dan Xu wrote: Hi All, When File.createTempFile() is called with some special parameters, it runs into infiniteloop and hangs. It is because it does not always mean a file exists when the method, createFileExclusively(), returns false. This fix is going to solve such issues reported in JDK-8013827 and JDK-8011950.And I also added some testcases to verify it. webrev: http://cr.openjdk.java.net/~dxu/8013827/webrev.00/ bug: http://bugs.sun.com/view_bug.do?bug_id=8011950 -Dan Thanks for taking this one. The changes mean that IAE is now thrown for cases where it wasn't thrown previously and I'm wondering if this is worth doing. It might be simpler to just throw IOException on the grounds that creating the file will fail. One other thing to consider is that \ is a valid name of a file on some platforms. I suspect you'll need to delegate to the FileSystem to check the suffix/prefix. Alternatively in TempDirectory.generateFile you could create a File from prefix/n/suffix and call getName to check that its matches. If not then throw an exception. -Alan. Thanks for your good review. I have updated my changes. And it is uploaded at, http://cr.openjdk.java.net/~dxu/8013827/webrev.01/. -Dan
RFR: JDK-8015628 - Test Failure in closed/java/io/pathNames/GeneralSolaris.java
Hi All, The fix to JDK-8009258 solves the failure in GeneralWin32.java. But the changes in Line 311, 312 in checkNames() method of General.java file, lead to the new failure of GeneralSolaris.java testcase. Because it changes the implicit value of an empty ask parameter. This fix is to revert the relevant changes of the above two lines in General.java. And accordingly, it accommodates the code in GeneralWin32.java to the original General.checkNames() method. webrev: http://cr.openjdk.java.net/~dxu/8015628/webrev.00/ Thanks for your review! -Dan
RFR: JDK-8013827 and JDK-8011950, , java.io.File.createTempFile enters infinite loop when passed invalid data
Hi All, When File.createTempFile() is called with some special parameters, it runs into infiniteloop and hangs. It is because it does not always mean a file exists when the method, createFileExclusively(), returns false. This fix is going to solve such issues reported in JDK-8013827 and JDK-8011950.And I also added some testcases to verify it. webrev: http://cr.openjdk.java.net/~dxu/8013827/webrev.00/ bug: http://bugs.sun.com/view_bug.do?bug_id=8011950 -Dan
Re: [PATCH] Review Request for 8009258: TEST_BUG: java/io/pathNames/GeneralWin32.java fails intermittently
On 05/24/2013 07:07 AM, Alan Bateman wrote: On 24/05/2013 05:19, Eric Wang wrote: Hi Dan All, I have updated the test based on your comments, Can you please review the fix? Thanks! http://cr.openjdk.java.net/~ewang/8009258/webrev.02/ I think this is okay. Dan is going to sponsor this one. -Alan Hi Eric, The changes look good. I am going to push it for you today. Thanks for your effort! -Dan
hg: jdk8/tl/jdk: 8009258: TEST_BUG:java/io/pathNames/GeneralWin32.java fails intermittently
Changeset: e59d7f0f36f7 Author:ewang Date: 2013-05-28 22:22 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e59d7f0f36f7 8009258: TEST_BUG:java/io/pathNames/GeneralWin32.java fails intermittently Reviewed-by: dxu, alanb Contributed-by: yiming.w...@oracle.com ! test/java/io/pathNames/General.java ! test/java/io/pathNames/GeneralWin32.java
hg: jdk8/tl/jdk: 8011136: FileInputStream.available and skip inconsistencies
Changeset: 3b1450ee2bb9 Author:dxu Date: 2013-05-17 12:04 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3b1450ee2bb9 8011136: FileInputStream.available and skip inconsistencies Summary: Correct the behavior of available() and update related java specs for available() and skip() in InputStream and FileInputStream classes. Reviewed-by: alanb ! src/share/classes/java/io/FileInputStream.java ! src/share/classes/java/io/InputStream.java ! src/share/native/java/io/FileInputStream.c ! test/java/io/FileInputStream/LargeFileAvailable.java ! test/java/io/FileInputStream/NegativeAvailable.java
Re: RFR: JDK-8011136 - FileInputStream.available and skip inconsistencies
On 05/13/2013 06:25 AM, Alan Bateman wrote: On 10/05/2013 22:20, Dan Xu wrote: Hi, The FileInputStream.available() method can return a negative value when the file position is beyond the endof afile. This is an unspecified behaviour that has the potential to break applications that expect available to return a value of 0 or greater. The FileInputStream.skip(long) method allows a negative value as its parameter. This conflicts with the specifications of InputStream and FileInputStream classes. They are both long standing behaviours. In the fix, available() has been changed to only return 0 or positive values. And for the skip(long) method, due to the compatibility concern, its behaviour will not be changed. Instead, the related java specs are going to be changed to describe its current behaviour. bug: http://bugs.sun.com/view_bug.do?bug_id=8011136 webrev: http://cr.openjdk.java.net/~dxu/8011136/webrev.00/ Thanks for your review! -Dan Thanks for following up on this one. Overall I agree with the approach, it specifies skip to match long standing behavior and fixes available to not return negative values. Just on wording, it might be better if the new statement in InputStream.skip didn't start with But. How about Subclasses may ... as this would be consistent with exiting wording in this class. For FileInputStream then the statement on how available behaves should probably move to the available javadoc. Something like Returns 0 when the file position is beyond EOF should be fine. -Alan. Thanks for your review, I have updated wordings in the java doc. The new webrev can be reviewed at http://cr.openjdk.java.net/~dxu/8011136/webrev.01/ http://cr.openjdk.java.net/%7Edxu/8011136/webrev.01/. -Dan
RFR: JDK-8011136 - FileInputStream.available and skip inconsistencies
Hi, The FileInputStream.available() method can return a negative value when the file position is beyond the endof afile. This is an unspecified behaviour that has the potential to break applications that expect available to return a value of 0 or greater. The FileInputStream.skip(long) method allows a negative value as its parameter. This conflicts with the specifications of InputStream and FileInputStream classes. They are both long standing behaviours. In the fix, available() has been changed to only return 0 or positive values. And for the skip(long) method, due to the compatibility concern, its behaviour will not be changed. Instead, the related java specs are going to be changed to describe its current behaviour. bug: http://bugs.sun.com/view_bug.do?bug_id=8011136 webrev: http://cr.openjdk.java.net/~dxu/8011136/webrev.00/ Thanks for your review! -Dan
RFR: JDK-8014360-Optimize and Refactor the Normalize Process in java.io File Systems
Hi, The normalize() implementations in UnixFileSystem.java and WinNTFileSystem.java can be improved to make it simpler, easier, and more efficient. I have refactoredthecode when working on JDK-8003992. And Iwould like to send it out for a review in this separate bug. Thanks! webrev: http://cr.openjdk.java.net/~dxu/8014360/webrev.00/ -Dan
Re: Review Request for JDK-8003992: File and other classes in java.io do not handle embedded nulls properly
On 05/06/2013 06:59 AM, Florian Weimer wrote: On 05/03/2013 01:08 AM, Dan Xu wrote: Hi All, Thanks for all your comments. Based on the previous feedback, I have moved to the other approach, i.e., to fail file operations if the invalid NUL characher is found in a file path. As you know, due to the compatibility issue, we cannot throw an exception immediately in the File constructors. So the failure is delayed and only shown up when any file operation is triggered. As for FileInputStream, FileOutputStream, and RandomAccessFile classes, the FileNotFoundException will be thrown right away since their spec allow this exception happen in the constructors. Thanks for your review! webrev: http://cr.openjdk.java.net/~dxu/8003992/webrev.01/ I like this approach, thanks. I think the additional parenthesis around the return expression and the ternary operator are not part of the usual coding standard. Thank you for your good review. I will fix the format. As for the possible scenario, NUL appearing at the end of a path, I will regard it as the same kind of invalidity as other embedded NUL cases since there are no obvious use cases for that. I will push the fix today. Thanks! -Dan
hg: jdk8/tl/jdk: 8003992: File and other classes in java.io do not handle embedded nulls properly
Changeset: bd118033e44c Author:dxu Date: 2013-05-06 14:17 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/bd118033e44c 8003992: File and other classes in java.io do not handle embedded nulls properly Summary: Have every file operation done with File, FileInputStream, FileOutputStream, or RandomAccessFile that involves a file path containing NUL fail. Also reviewed by fwei...@redhat.com Reviewed-by: alanb, sherman, ahgross, mduigou, dholmes, aph, plevart, martin ! src/share/classes/java/io/File.java ! src/share/classes/java/io/FileInputStream.java ! src/share/classes/java/io/FileOutputStream.java ! src/share/classes/java/io/RandomAccessFile.java + test/java/io/File/NulFile.java
Re: Review Request for JDK-8003992: File and other classes in java.io do not handle embedded nulls properly
Hi All, Thanks for all your comments. Based on the previous feedback, I have moved to the other approach, i.e., to fail file operations if the invalid NUL characher is found in a file path. As you know, due to the compatibility issue, we cannot throw an exception immediately in the File constructors. So the failure is delayed and only shown up when any file operation is triggered. As for FileInputStream, FileOutputStream, and RandomAccessFile classes, the FileNotFoundException will be thrown right away since their spec allow this exception happen in the constructors. Thanks for your review! webrev: http://cr.openjdk.java.net/~dxu/8003992/webrev.01/ -Dan
Re: [PATCH] Review Request for 8009258: TEST_BUG: java/io/pathNames/GeneralWin32.java fails intermittently
Hi Eric, Thanks for fixing the test failures. I recently reviewed your changes. And I like your idea to add a base dir to restrict the test only touching files/directories that are created by itself to avoid the interferences from the OS or other test activities. And in Line 341 of General.java, I notice you make the code return if it tries to test baseDir or its ascendant directories, which reduces the test coverage. Since GeneralWin32.java knows its max tree depth, I think you can make the baseDir deep enough in the prepared directory structure so that the test can still run inside the testing directories even if it visits the baseDir's ascendant directories. One idea is to make the max depth as a parameter of initTestData(), and this method can intelligently return an appropriate baseDir basing on it. After the above change, you can move the baseDir and userDir back to GeneralWin32.java to make the two classes loosely coupled. In the changes, the usages of NIO classes are not necessary. The test is against java.io packages, and it is better to keep it clean in case it might be used to test old jdk version which does not have NIO. Before the test ends, it is better to clean the testing files and directories which are created at the beginning. Thanks! -Dan On 03/12/2013 11:28 PM, Eric Wang wrote: Hi, Please review the code change, I have updated the test to make sure test only access files and directories created by itself. http://cr.openjdk.java.net/~ewang/8009258/webrev.01/ Here is the execution result: http://cr.openjdk.java.net/~ewang/8009258/GeneralWin32.jtr Thanks, Eric On 2013/3/5 1:39, Alan Bateman wrote: On 04/03/2013 17:32, Eric Wang wrote: Hi, Please help to review fix below for bug 8009258 https://jbs.oracle.com/bugs/browse/JDK-8009258, TEST_BUG:java/io/pathNames/GeneralWin32.java fails intermittently. http://cr.openjdk.java.net/~ewang/8009258/webrev.00/ The File.canRead() method should not be used to check read permission of a directory. Thanks, Eric I wonder if it would be better to change this test so that it doesn't even attempt to poke around in these directories. I suggest this because there may be other activity going on at the same time. See also 8004096 where the test is running in agentvm mode and is straying into the directory used by another agent VM. -Alan
Re: [PATCH] Review Request for 8009258: TEST_BUG: java/io/pathNames/GeneralWin32.java fails intermittently
On 04/17/2013 02:15 AM, Alan Bateman wrote: On 17/04/2013 07:36, Dan Xu wrote: : In the changes, the usages of NIO classes are not necessary. The test is against java.io packages, and it is better to keep it clean in case it might be used to test old jdk version which does not have NIO. I briefly looked at it and I think the usage okay is because it just makes it easier to check the results and also to keep the directory walk from wandering into other parts of the file system. If it changed what was being tested then I would agree that it shouldn't be used. -Alan I see. My thought is that the relative path between baseDir and userDir can be calculated inside initTestData() when it constructs the baseDir. But I agree it is not an issue here. Thanks! -Dan
Re: What is the Build Process for Codes inside jdk/src/macosx/native/jobjc
Adding core-libs-dev On 04/17/2013 04:47 PM, Dan Xu wrote: Hi, As for the sourcecodes for mac platform, it has a special place holding native and java codes for jdk, jdk/src/macosx/native/jobjc. I wonder how those codes are builtand whether its compilation process has any special handling. Thanks! -Dan
Re: What is the Build Process for Codes inside jdk/src/macosx/native/jobjc
Hi David, Under src/macosx/native/jobjc folder, it contains not only native *.m source files, but also *.java files. If you check the build results in build/macosx-x86_64-normal-server-release/jdk folder, it contains some build results specific for jobjc, say gensrc_jobjc/, gensrc_headers_jobjc/, jobjc_classes/, jobjc_classes_headers/. So it must have some extra build steps to generate those jobjc results. And I wonder what they are and why they are special and not merged into the regular native compilation and java compilation processes. Thanks! -Dan On 04/17/2013 05:30 PM, David Holmes wrote: Hi Dan, I don't quite understand the question but all native code building is handled via jdk/makefiles/CompileNativeLibraries.gmk which in turn utilizes the set up from top/common/makefiles/NativeCompilation.gmk HTH David On 18/04/2013 9:51 AM, Dan Xu wrote: Adding core-libs-dev On 04/17/2013 04:47 PM, Dan Xu wrote: Hi, As for the sourcecodes for mac platform, it has a special place holding native and java codes for jdk, jdk/src/macosx/native/jobjc. I wonder how those codes are builtand whether its compilation process has any special handling. Thanks! -Dan
RFR: JDK8011946 - java.util.Currency javadoc has broken link to iso.org
Hi All, Here is ajava doc fix. I have changed the broken link and pointed it to the page for standard ISO 4217. Thanks! Webrev: http://cr.openjdk.java.net/~dxu/8011946/webrev/ Bug: http://bugs.sun.com/view_bug.do?bug_id=8011946 -Dan
Review Request: JDK-8011602 - jobjc build failure on Mac
Hi All, Please review the change to fix the build issue on mac platformat http://cr.openjdk.java.net/~dxu/8011602/webrev/.It removes the un-necessary @Native annotation from src/macosx/native/jobjc/src/core/java/com/apple/jobjc/Coder.java. Therefore, the objc compilation will not dependon the new java.lang.annotation.Native interfacethat is only introduced in jdk8. For the normalbuild process, even though @Native annotationis added to many other java classes to replace @GenerateNativeHeader, the langtool has the capability to handle that when building with jdk7. But on Mac platform, objc compilation is a special process, where the magic of langtool to handle new jdk8 interface in old jdk7 cannot be applied. Since Coder.java already has a native method, getNativeFFITypePtrForCode(), declared, @Native is notnecessary to be present, and it is safe to remove them. The other change in file, src/share/classes/sun/java2d/opengl/OGLContext.java, is only a format correction. I have tested this fix in jprt with boot jdk as jdk7 across all platforms. Thanks! webreve: http://cr.openjdk.java.net/~dxu/8011602/webrev/ -Dan
