Re: RFR 6642881: Improve performance of Class.getClassLoader()

2014-06-23 Thread David Holmes 

On 24/06/2014 11:44 AM, Coleen Phillimore wrote:


On 6/23/14, 9:36 PM, Mandy Chung wrote:

Coleen,

On 6/23/2014 4:45 PM, Coleen Phillimore wrote:


Please review a change to the JDK code for adding classLoader field
to the instances of java/lang/Class.  This change restricts
reflection from changing access to the classLoader field.  In the
spec, AccessibleObject.setAccessible() may throw SecurityException if
the accessibility of an object may not be changed:

http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean)


Only AccessibleObject.java has changes from the previous version of
this change.



This change looks reasonable.As a side note:  Joel mentions about
the mechanism to hide class members from reflection.   I discussed
with Joel offline before he went on parental leave and suggest that we
should revisit the two mechanisms that both effectively disallow
access to private members in the future.


Thanks, Mandy.  Yes, let me know what you come up with and we can
improve this.  Thank you for the help fixing this bug.


Note that completely hiding something from reflection is different to 
controlling its accessability via reflection. I think what is being done 
here is the right way to go.


The changes look okay to me.

Thanks,
David



Coleen



Mandy


open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
bug link https://bugs.openjdk.java.net/browse/JDK-6642881

Thanks,
Coleen

On 6/19/14, 11:01 PM, David Holmes wrote:

On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung  wrote:


On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore
 wrote:

On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:

Have you considered hiding the Class.classLoader field from
reflection? I’m not sure it is necessary but I’m not to keen on
the idea of people poking at this field with Unsafe (which
should go away in 9 but …).

I don't know how to hide the field from reflection. It's a
private final field so you need to get priviledges to make it
setAccessible.  If you mean injecting it on the JVM side, the
reason for this change is so that the JIT compilers can inline
this call and not have to call into the JVM to get the class
loader.


There is sun.reflect.Reflection.registerFieldsToFilter() that
hides a field from being found using reflection. It might very
well be the case that private and final is enough, I haven’t
thought this through 100%. On the other hand, is there a reason
to give users access through the field instead of having to use
Class.getClassLoader()?


There are many getter methods that returns a private final field.
I'm not sure if hiding the field is necessary nor a good precedence
to set. Accessing a private field requires "accessDeclaredMember"
permission although it's a different security check (vs
"getClassLoader"
permission).  Arguably before this new classLoader field, one could
call Class.getClassLoader0() via reflection to get a hold of class
loader.

Perhaps you are concerned that the "accessDeclaredMember" permission
is too coarse-grained?  I think the security team is looking into
the improvement in that regards.


I think I’m a bit worried about two things, first as you wrote,
“accessDeclaredMember” isn’t the same as “getClassLoader”, but
since you could get the class loader through getClassLoader0() that
shouldn’t be a new issue.

The second thing is that IIRC there are ways to set a final field
after initialization. I’m not sure we need to care about that
either if you need Unsafe to do it.


Normal reflection can set a final field if you can successfully call
setAccessible(true) on the Field object.

David
-



cheers
/Joel

Re: RFR 6642881: Improve performance of Class.getClassLoader()

2014-06-23 Thread Coleen Phillimore 


On 6/23/14, 9:36 PM, Mandy Chung wrote:

Coleen,

On 6/23/2014 4:45 PM, Coleen Phillimore wrote:


Please review a change to the JDK code for adding classLoader field 
to the instances of java/lang/Class.  This change restricts 
reflection from changing access to the classLoader field.  In the 
spec, AccessibleObject.setAccessible() may throw SecurityException if 
the accessibility of an object may not be changed:


http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) 



Only AccessibleObject.java has changes from the previous version of 
this change.




This change looks reasonable.As a side note:  Joel mentions about 
the mechanism to hide class members from reflection.   I discussed 
with Joel offline before he went on parental leave and suggest that we 
should revisit the two mechanisms that both effectively disallow 
access to private members in the future.


Thanks, Mandy.  Yes, let me know what you come up with and we can 
improve this.  Thank you for the help fixing this bug.


Coleen



Mandy


open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
bug link https://bugs.openjdk.java.net/browse/JDK-6642881

Thanks,
Coleen

On 6/19/14, 11:01 PM, David Holmes wrote:

On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung  wrote:


On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore 
 wrote:

On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:
Have you considered hiding the Class.classLoader field from 
reflection? I’m not sure it is necessary but I’m not to keen on 
the idea of people poking at this field with Unsafe (which 
should go away in 9 but …).
I don't know how to hide the field from reflection. It's a 
private final field so you need to get priviledges to make it 
setAccessible.  If you mean injecting it on the JVM side, the 
reason for this change is so that the JIT compilers can inline 
this call and not have to call into the JVM to get the class 
loader.


There is sun.reflect.Reflection.registerFieldsToFilter() that 
hides a field from being found using reflection. It might very 
well be the case that private and final is enough, I haven’t 
thought this through 100%. On the other hand, is there a reason 
to give users access through the field instead of having to use 
Class.getClassLoader()?



There are many getter methods that returns a private final field.
I'm not sure if hiding the field is necessary nor a good precedence
to set. Accessing a private field requires "accessDeclaredMember"
permission although it's a different security check (vs 
"getClassLoader"

permission).  Arguably before this new classLoader field, one could
call Class.getClassLoader0() via reflection to get a hold of class
loader.

Perhaps you are concerned that the "accessDeclaredMember" permission
is too coarse-grained?  I think the security team is looking into
the improvement in that regards.


I think I’m a bit worried about two things, first as you wrote, 
“accessDeclaredMember” isn’t the same as “getClassLoader”, but 
since you could get the class loader through getClassLoader0() that 
shouldn’t be a new issue.


The second thing is that IIRC there are ways to set a final field 
after initialization. I’m not sure we need to care about that 
either if you need Unsafe to do it.


Normal reflection can set a final field if you can successfully call 
setAccessible(true) on the Field object.


David
-



cheers
/Joel

Re: RFR 6642881: Improve performance of Class.getClassLoader()

2014-06-23 Thread Mandy Chung 

Coleen,

On 6/23/2014 4:45 PM, Coleen Phillimore wrote:


Please review a change to the JDK code for adding classLoader field to 
the instances of java/lang/Class.  This change restricts reflection 
from changing access to the classLoader field.  In the spec, 
AccessibleObject.setAccessible() may throw SecurityException if the 
accessibility of an object may not be changed:


http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) 



Only AccessibleObject.java has changes from the previous version of 
this change.




This change looks reasonable.As a side note:  Joel mentions about 
the mechanism to hide class members from reflection.   I discussed with 
Joel offline before he went on parental leave and suggest that we should 
revisit the two mechanisms that both effectively disallow access to 
private members in the future.


Mandy


open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
bug link https://bugs.openjdk.java.net/browse/JDK-6642881

Thanks,
Coleen

On 6/19/14, 11:01 PM, David Holmes wrote:

On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung  wrote:


On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore 
 wrote:

On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:
Have you considered hiding the Class.classLoader field from 
reflection? I’m not sure it is necessary but I’m not to keen on 
the idea of people poking at this field with Unsafe (which 
should go away in 9 but …).
I don't know how to hide the field from reflection. It's a 
private final field so you need to get priviledges to make it 
setAccessible.  If you mean injecting it on the JVM side, the 
reason for this change is so that the JIT compilers can inline 
this call and not have to call into the JVM to get the class loader.


There is sun.reflect.Reflection.registerFieldsToFilter() that 
hides a field from being found using reflection. It might very 
well be the case that private and final is enough, I haven’t 
thought this through 100%. On the other hand, is there a reason to 
give users access through the field instead of having to use 
Class.getClassLoader()?



There are many getter methods that returns a private final field.
I'm not sure if hiding the field is necessary nor a good precedence
to set. Accessing a private field requires "accessDeclaredMember"
permission although it's a different security check (vs 
"getClassLoader"

permission).  Arguably before this new classLoader field, one could
call Class.getClassLoader0() via reflection to get a hold of class
loader.

Perhaps you are concerned that the "accessDeclaredMember" permission
is too coarse-grained?  I think the security team is looking into
the improvement in that regards.


I think I’m a bit worried about two things, first as you wrote, 
“accessDeclaredMember” isn’t the same as “getClassLoader”, but since 
you could get the class loader through getClassLoader0() that 
shouldn’t be a new issue.


The second thing is that IIRC there are ways to set a final field 
after initialization. I’m not sure we need to care about that either 
if you need Unsafe to do it.


Normal reflection can set a final field if you can successfully call 
setAccessible(true) on the Field object.


David
-



cheers
/Joel

Re: RFR: 8042872: Fix raw and unchecked warnings in sun.applet

2014-06-23 Thread Joe Darcy 

Hi Henry,

The changes look good to me; thanks,

-Joe

On 06/23/2014 05:31 PM, Henry Jen wrote:

Hi,

Please review another webrev to clean up rawtypes and unchecked 
warning, this time for sun.applet.


The webrev applied to both jdk9/dev and jdk9/client cleanly, I am not 
sure which repo should the webrev go once pass review.


Webrev can be found at
http://cr.openjdk.java.net/~henryjen/jdk9/8042872/0/webrev/

Please also let me know any particular test I should do other than 
regular jprt.


Cheers,
Henry

RFR: 8047795: Collections.checkedList checking bypassed by List.replaceAll

2014-06-23 Thread Mike Duigou 
Hello all;

This changeset corrects a reported problem with the lists returned by 
Collections.checkedList(). Since Java 8 the replaceAll() method on checked 
lists has erroneously allowed the operator providing replacements to provide 
illegal replacement values which are then stored, unchecked into the wrapped 
list.

This changeset adds a check on the proposed replacement value and throws a 
ClassCastException if the replacement value is incompatible with the list. 
Additionally the javadoc is updated to inform users that a ClassCastException 
may result if the proposed replacement is unacceptable.

Note that this changeset takes the strategy of failing when the illegal value 
is encountered. Replacements of earlier items in the list are retained.

jbsbug: https://bugs.openjdk.java.net/browse/JDK-8047795
webrev: http://cr.openjdk.java.net/~mduigou/JDK-8047795/0/webrev/

This change will be backported to Java 8.

Mike

RFR: 8042872: Fix raw and unchecked warnings in sun.applet

2014-06-23 Thread Henry Jen 

Hi,

Please review another webrev to clean up rawtypes and unchecked warning, 
this time for sun.applet.


The webrev applied to both jdk9/dev and jdk9/client cleanly, I am not 
sure which repo should the webrev go once pass review.


Webrev can be found at
http://cr.openjdk.java.net/~henryjen/jdk9/8042872/0/webrev/

Please also let me know any particular test I should do other than 
regular jprt.


Cheers,
Henry

Re: RFR 8035490: move xml jaxp unit tests in bugxxx dir into jaxp repo

2014-06-23 Thread huizhe wang 


On 6/20/2014 2:26 AM, Alan Bateman wrote:

On 20/06/2014 09:42, Patrick Zhang wrote:

Hi Team,

8035490 is one task used to track xml code colocation from sqe repo 
into jaxp repo.

As one initial change set, it includes below materials:
1. TEST.ROOT to declare test code location.
2. ProblemList.txt to exclude 6 tests which have been discussed on past.
3. One Makefile to support future jprt/aurora execution.
4. test code located in bugxxx dir. All are based on testNG.


webrev:
http://cr.openjdk.java.net/~pzhang/xml_colocation/8035490/webrev/

:


(Moving this thread this to core-libs-dev).


+1!  Patrick, please send future review request to core-libs-dev.

It's a great effort from the SQE team and Patrick migrating all of the 
jaxp tests into the OpenJDK repo. Thanks again!




At a high-level then it's good to have the tests for JAXP in the same 
repository as the code. I've only skimmed over these webrev (not a 
review) and they look like they are mostly regression tests for 
specific bugs - is that right?


Yes.  This is the first set of jaxp tests to be migrated. To help 
understand jaxp test, here is what it looks like:

JAXP tests -
   - 1. JAXP Unit tests
   - a. those named after bug id
   - b. javax.xml.datatype
   - c. javax.xml.parsers
   - d. javax.xml.stream
   - e. javax.xml.transform
   - f. javax.xml.validation
   - g. javax.xml.xpath
   - h. org.w3c.dom
   - i. org.w3c.saxtest

   - 2. JAXP SQE (function) tests
   - a. product tests
   - b. ASTRO xsltc tests
   - c. Auction Portal tests
   - d. GAP tests
   - e. XSLT tests


1a were some of the oldest unit tests that were put in packages by their 
bug ids. Newer tests were named after their corresponding API package names.


Patrick's plan is to migrate 1 (unit tests) in 4 batches. The 1st (this 
one) covers 1a.




One thing that concerns me a bit is that none of the .java files that 
I looked at have any comments to say what the test does. Is there 
anything that could be brought over from the original issue in JIRA to 
explain what each of these tests is about?


It would have been nice if they had comments. But adding comments would 
be too much work, unnecessary I would think. These tests serve their 
purpose, that is, preventing regressions. In case any fails, its bugid 
would allow us to easily find out what it was testing.




Is Hello.wsdl.data a binary file? Have any alternatives been looked at?


It's UTF-16 encoded. Patrick and I looked at it earlier. Mercurial has 
no problem handling binary. Would you think it'd cause any problem?  If 
it's really unwanted, we can look into modifying the test.




As you mention ProblemList.txt then I see that it has headings that 
don't make sense in the jaxp repository, maybe they should be removed.


The webrev only has the changes for jaxp repository, will there be 
follow-on changes to make it runnable at the top level, JPRT 
configuration, etc.


I think that'd be jdk_jaxp. Patrick, you've run JPRT, correct?

-Joe



-Alan

Re: RFR 6642881: Improve performance of Class.getClassLoader()

2014-06-23 Thread Coleen Phillimore 


Please review a change to the JDK code for adding classLoader field to 
the instances of java/lang/Class.  This change restricts reflection from 
changing access to the classLoader field.  In the spec, 
AccessibleObject.setAccessible() may throw SecurityException if the 
accessibility of an object may not be changed:


http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean)

Only AccessibleObject.java has changes from the previous version of this 
change.


open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
bug link https://bugs.openjdk.java.net/browse/JDK-6642881

Thanks,
Coleen

On 6/19/14, 11:01 PM, David Holmes wrote:

On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung  wrote:


On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore 
 wrote:

On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:
Have you considered hiding the Class.classLoader field from 
reflection? I’m not sure it is necessary but I’m not to keen on 
the idea of people poking at this field with Unsafe (which should 
go away in 9 but …).
I don't know how to hide the field from reflection.  It's a 
private final field so you need to get priviledges to make it 
setAccessible.  If you mean injecting it on the JVM side, the 
reason for this change is so that the JIT compilers can inline 
this call and not have to call into the JVM to get the class loader.


There is sun.reflect.Reflection.registerFieldsToFilter() that hides 
a field from being found using reflection. It might very well be 
the case that private and final is enough, I haven’t thought this 
through 100%. On the other hand, is there a reason to give users 
access through the field instead of having to use 
Class.getClassLoader()?



There are many getter methods that returns a private final field.
I'm not sure if hiding the field is necessary nor a good precedence
to set. Accessing a private field requires "accessDeclaredMember"
permission although it's a different security check (vs 
"getClassLoader"

permission).  Arguably before this new classLoader field, one could
call Class.getClassLoader0() via reflection to get a hold of class
loader.

Perhaps you are concerned that the "accessDeclaredMember" permission
is too coarse-grained?  I think the security team is looking into
the improvement in that regards.


I think I’m a bit worried about two things, first as you wrote, 
“accessDeclaredMember” isn’t the same as “getClassLoader”, but since 
you could get the class loader through getClassLoader0() that 
shouldn’t be a new issue.


The second thing is that IIRC there are ways to set a final field 
after initialization. I’m not sure we need to care about that either 
if you need Unsafe to do it.


Normal reflection can set a final field if you can successfully call 
setAccessible(true) on the Field object.


David
-



cheers
/Joel

Re: RFR: 8047721: @since should have JDK version

2014-06-23 Thread Bradford Wetmore 
Except for these two classes, none of the JCE APIs ever contained @since 
until the JCE was put into JDK 1.4 back in 2002.  The unbundled JCE 
hasn't been shipped in probably almost a decade.  None of the unbundled 
JSSE/JGSS should have them either.


Carrying around this old information is just cruft, IMO.

Brad





On 6/23/2014 2:28 PM, Paul Benedict wrote:

What's the rationale for removing the secondary version? Or I guess the
question should really be: when are secondary versions useful? At least
in the EE specs, the EE version plus the spec version are listed in many
places like this.


Cheers,
Paul


On Mon, Jun 23, 2014 at 3:50 PM, Henry Jen mailto:henry@oracle.com>> wrote:

OK, I'll remove all @since JCE line, as the class already has @since
1.4 as Joe pointed out earlier.

Uodated webrev at

http://cr.openjdk.java.net/~__henryjen/jdk9/8047721/2/__webrev/


Cheers,
Henry



On 06/23/2014 10:04 AM, Bradford Wetmore wrote:

I would prefer that JCE1.2 be pulled out completely in the Cipher*
classes.  I will be sending you a separate note about JCE logistics.

Thanks for doing this cleanup.

Brad


On 6/20/2014 11:46 AM, Henry Jen wrote:

Hi,

Please review a trivial webrev to add JDK version to @since
in a format
as Mark suggested[1].

http://cr.openjdk.java.net/~__henryjen/jdk9/8047721/0/__webrev/


[1]

http://mail.openjdk.java.net/__pipermail/jdk9-dev/2014-June/__000806.html



Appened is the diff as in the webrev.

Cheers,
Henry


diff --git a/src/share/classes/java/lang/__Package.java
b/src/share/classes/java/lang/__Package.java
--- a/src/share/classes/java/lang/__Package.java
+++ b/src/share/classes/java/lang/__Package.java
@@ -107,6 +107,7 @@
* loader to be found.
*
* @see ClassLoader#definePackage
+ * @since 1.2
*/
   public class Package implements
java.lang.reflect.__AnnotatedElement {
   /**
diff --git
a/src/share/classes/javax/__crypto/CipherInputStream.java
b/src/share/classes/javax/__crypto/CipherInputStream.java
--- a/src/share/classes/javax/__crypto/CipherInputStream.java
+++ b/src/share/classes/javax/__crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
* @return  the next byte of data, or -1
if the end
of the
*  stream is reached.
* @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
   public int read() throws IOException {
   if (ostart >= ofinish) {
@@ -196,7 +196,7 @@
* the stream has been reached.
* @exception  IOException  if an I/O error occurs.
* @seejava.io.InputStream#read(byte[__],
int, int)
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
*/
   public int read(byte b[]) throws IOException {
   return read(b, 0, b.length);
@@ -217,7 +217,7 @@
* the stream has been reached.
* @exception  IOException  if an I/O error occurs.
* @seejava.io.InputStream#read()
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
*/
   public int read(byte b[], int off, int len) throws
IOException {
   if (ostart >= ofinish) {
@@ -254,7 +254,7 @@
* @param  n the number of bytes to be skipped.
* @return the actual number of bytes skipped.
* @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
   public long skip(long n) throws IOException {
   int available = ofinish - ostart;
@@ -277,7 +277,7 @@
* @return the number of bytes that can be read
from this
input stream
* without blocking.
* @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
*/

Re: RFR: 8047721: @since should have JDK version

2014-06-23 Thread Paul Benedict 
What's the rationale for removing the secondary version? Or I guess the
question should really be: when are secondary versions useful? At least in
the EE specs, the EE version plus the spec version are listed in many
places like this.


Cheers,
Paul


On Mon, Jun 23, 2014 at 3:50 PM, Henry Jen  wrote:

> OK, I'll remove all @since JCE line, as the class already has @since 1.4
> as Joe pointed out earlier.
>
> Uodated webrev at
>
> http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/
>
> Cheers,
> Henry
>
>
>
> On 06/23/2014 10:04 AM, Bradford Wetmore wrote:
>
>> I would prefer that JCE1.2 be pulled out completely in the Cipher*
>> classes.  I will be sending you a separate note about JCE logistics.
>>
>> Thanks for doing this cleanup.
>>
>> Brad
>>
>>
>> On 6/20/2014 11:46 AM, Henry Jen wrote:
>>
>>> Hi,
>>>
>>> Please review a trivial webrev to add JDK version to @since in a format
>>> as Mark suggested[1].
>>>
>>> http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/
>>>
>>> [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/
>>> 000806.html
>>>
>>> Appened is the diff as in the webrev.
>>>
>>> Cheers,
>>> Henry
>>>
>>>
>>> diff --git a/src/share/classes/java/lang/Package.java
>>> b/src/share/classes/java/lang/Package.java
>>> --- a/src/share/classes/java/lang/Package.java
>>> +++ b/src/share/classes/java/lang/Package.java
>>> @@ -107,6 +107,7 @@
>>>* loader to be found.
>>>*
>>>* @see ClassLoader#definePackage
>>> + * @since 1.2
>>>*/
>>>   public class Package implements java.lang.reflect.AnnotatedElement {
>>>   /**
>>> diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
>>> b/src/share/classes/javax/crypto/CipherInputStream.java
>>> --- a/src/share/classes/javax/crypto/CipherInputStream.java
>>> +++ b/src/share/classes/javax/crypto/CipherInputStream.java
>>> @@ -170,7 +170,7 @@
>>>* @return  the next byte of data, or -1 if the end
>>> of the
>>>*  stream is reached.
>>>* @exception  IOException  if an I/O error occurs.
>>> - * @since JCE1.2
>>> + * @since 1.4, JCE1.2
>>>*/
>>>   public int read() throws IOException {
>>>   if (ostart >= ofinish) {
>>> @@ -196,7 +196,7 @@
>>>* the stream has been reached.
>>>* @exception  IOException  if an I/O error occurs.
>>>* @seejava.io.InputStream#read(byte[], int, int)
>>> - * @since  JCE1.2
>>> + * @since  1.4, JCE1.2
>>>*/
>>>   public int read(byte b[]) throws IOException {
>>>   return read(b, 0, b.length);
>>> @@ -217,7 +217,7 @@
>>>* the stream has been reached.
>>>* @exception  IOException  if an I/O error occurs.
>>>* @seejava.io.InputStream#read()
>>> - * @since  JCE1.2
>>> + * @since  1.4, JCE1.2
>>>*/
>>>   public int read(byte b[], int off, int len) throws IOException {
>>>   if (ostart >= ofinish) {
>>> @@ -254,7 +254,7 @@
>>>* @param  n the number of bytes to be skipped.
>>>* @return the actual number of bytes skipped.
>>>* @exception  IOException  if an I/O error occurs.
>>> - * @since JCE1.2
>>> + * @since 1.4, JCE1.2
>>>*/
>>>   public long skip(long n) throws IOException {
>>>   int available = ofinish - ostart;
>>> @@ -277,7 +277,7 @@
>>>* @return the number of bytes that can be read from this
>>> input stream
>>>* without blocking.
>>>* @exception  IOException  if an I/O error occurs.
>>> - * @since  JCE1.2
>>> + * @since  1.4, JCE1.2
>>>*/
>>>   public int available() throws IOException {
>>>   return (ofinish - ostart);
>>> @@ -292,7 +292,7 @@
>>>* stream.
>>>*
>>>* @exception  IOException  if an I/O error occurs.
>>> - * @since JCE1.2
>>> + * @since 1.4, JCE1.2
>>>*/
>>>   public void close() throws IOException {
>>>   if (closed) {
>>> @@ -321,7 +321,7 @@
>>>*  mark and reset methods.
>>>* @see java.io.InputStream#mark(int)
>>>* @see java.io.InputStream#reset()
>>> - * @since   JCE1.2
>>> + * @since   1.4, JCE1.2
>>>*/
>>>   public boolean markSupported() {
>>>   return false;
>>> diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
>>> b/src/share/classes/javax/crypto/CipherOutputStream.java
>>> --- a/src/share/classes/javax/crypto/CipherOutputStream.java
>>> +++ b/src/share/classes/javax/crypto/CipherOutputStream.java
>>> @@ -114,7 +114,7 @@
>>>*
>>>* @param  b   the byte.
>>>* @exception  IOException  if an I/O error occurs.
>>> - * @since  JCE1.2
>>> + * @since  1.4, JCE1.2
>>>*/
>>>   public void write(int b) throws IOException {
>>>   ibuffer[0] = (byte) b;
>>> @@ -138,7 +138,7 @@
>>>* @exception  NullPointerException if

Re: RFR: 8047721: @since should have JDK version

2014-06-23 Thread Bradford Wetmore 

JCE (Cipher) changes look good to me.

Let me know if there's any problem with the point I mentioned in the 
other email.


Brad



On 6/23/2014 1:50 PM, Henry Jen wrote:

OK, I'll remove all @since JCE line, as the class already has @since 1.4
as Joe pointed out earlier.

Uodated webrev at

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/

Cheers,
Henry


On 06/23/2014 10:04 AM, Bradford Wetmore wrote:

I would prefer that JCE1.2 be pulled out completely in the Cipher*
classes.  I will be sending you a separate note about JCE logistics.

Thanks for doing this cleanup.

Brad


On 6/20/2014 11:46 AM, Henry Jen wrote:

Hi,

Please review a trivial webrev to add JDK version to @since in a format
as Mark suggested[1].

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/

[1]
http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html

Appened is the diff as in the webrev.

Cheers,
Henry


diff --git a/src/share/classes/java/lang/Package.java
b/src/share/classes/java/lang/Package.java
--- a/src/share/classes/java/lang/Package.java
+++ b/src/share/classes/java/lang/Package.java
@@ -107,6 +107,7 @@
   * loader to be found.
   *
   * @see ClassLoader#definePackage
+ * @since 1.2
   */
  public class Package implements java.lang.reflect.AnnotatedElement {
  /**
diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
b/src/share/classes/javax/crypto/CipherInputStream.java
--- a/src/share/classes/javax/crypto/CipherInputStream.java
+++ b/src/share/classes/javax/crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
   * @return  the next byte of data, or -1 if the end
of the
   *  stream is reached.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public int read() throws IOException {
  if (ostart >= ofinish) {
@@ -196,7 +196,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read(byte[], int, int)
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[]) throws IOException {
  return read(b, 0, b.length);
@@ -217,7 +217,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read()
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[], int off, int len) throws IOException {
  if (ostart >= ofinish) {
@@ -254,7 +254,7 @@
   * @param  n the number of bytes to be skipped.
   * @return the actual number of bytes skipped.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public long skip(long n) throws IOException {
  int available = ofinish - ostart;
@@ -277,7 +277,7 @@
   * @return the number of bytes that can be read from this
input stream
   * without blocking.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int available() throws IOException {
  return (ofinish - ostart);
@@ -292,7 +292,7 @@
   * stream.
   *
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void close() throws IOException {
  if (closed) {
@@ -321,7 +321,7 @@
   *  mark and reset methods.
   * @see java.io.InputStream#mark(int)
   * @see java.io.InputStream#reset()
- * @since   JCE1.2
+ * @since   1.4, JCE1.2
   */
  public boolean markSupported() {
  return false;
diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
b/src/share/classes/javax/crypto/CipherOutputStream.java
--- a/src/share/classes/javax/crypto/CipherOutputStream.java
+++ b/src/share/classes/javax/crypto/CipherOutputStream.java
@@ -114,7 +114,7 @@
   *
   * @param  b   the byte.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(int b) throws IOException {
  ibuffer[0] = (byte) b;
@@ -138,7 +138,7 @@
   * @exception  NullPointerException if b is null.
   * @exception  IOException  if an I/O error occurs.
   * @seejavax.crypto.CipherOutputStream#write(byte[], int,
int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void write(byte b[]) throws IOException {
  write(b, 0, b.length);
@@ -152,7 +152,7 @@
   * @param  off   the start offset in the data.
   * @param  len   the number of bytes to write.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(byte b[], int off, int len) throws IOExcep

Re: RFR: 8047721: @since should have JDK version

2014-06-23 Thread Joe Darcy 

Looks good to me; thanks,

-Joe

On 06/23/2014 01:50 PM, Henry Jen wrote:
OK, I'll remove all @since JCE line, as the class already has @since 
1.4 as Joe pointed out earlier.


Uodated webrev at

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/

Cheers,
Henry


On 06/23/2014 10:04 AM, Bradford Wetmore wrote:

I would prefer that JCE1.2 be pulled out completely in the Cipher*
classes.  I will be sending you a separate note about JCE logistics.

Thanks for doing this cleanup.

Brad


On 6/20/2014 11:46 AM, Henry Jen wrote:

Hi,

Please review a trivial webrev to add JDK version to @since in a format
as Mark suggested[1].

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/

[1] 
http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html


Appened is the diff as in the webrev.

Cheers,
Henry


diff --git a/src/share/classes/java/lang/Package.java
b/src/share/classes/java/lang/Package.java
--- a/src/share/classes/java/lang/Package.java
+++ b/src/share/classes/java/lang/Package.java
@@ -107,6 +107,7 @@
   * loader to be found.
   *
   * @see ClassLoader#definePackage
+ * @since 1.2
   */
  public class Package implements java.lang.reflect.AnnotatedElement {
  /**
diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
b/src/share/classes/javax/crypto/CipherInputStream.java
--- a/src/share/classes/javax/crypto/CipherInputStream.java
+++ b/src/share/classes/javax/crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
   * @return  the next byte of data, or -1 if the end
of the
   *  stream is reached.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public int read() throws IOException {
  if (ostart >= ofinish) {
@@ -196,7 +196,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read(byte[], int, int)
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[]) throws IOException {
  return read(b, 0, b.length);
@@ -217,7 +217,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read()
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[], int off, int len) throws IOException {
  if (ostart >= ofinish) {
@@ -254,7 +254,7 @@
   * @param  n the number of bytes to be skipped.
   * @return the actual number of bytes skipped.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public long skip(long n) throws IOException {
  int available = ofinish - ostart;
@@ -277,7 +277,7 @@
   * @return the number of bytes that can be read from this
input stream
   * without blocking.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int available() throws IOException {
  return (ofinish - ostart);
@@ -292,7 +292,7 @@
   * stream.
   *
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void close() throws IOException {
  if (closed) {
@@ -321,7 +321,7 @@
   *  mark and reset methods.
   * @see java.io.InputStream#mark(int)
   * @see java.io.InputStream#reset()
- * @since   JCE1.2
+ * @since   1.4, JCE1.2
   */
  public boolean markSupported() {
  return false;
diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
b/src/share/classes/javax/crypto/CipherOutputStream.java
--- a/src/share/classes/javax/crypto/CipherOutputStream.java
+++ b/src/share/classes/javax/crypto/CipherOutputStream.java
@@ -114,7 +114,7 @@
   *
   * @param  b   the byte.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(int b) throws IOException {
  ibuffer[0] = (byte) b;
@@ -138,7 +138,7 @@
   * @exception  NullPointerException if b is null.
   * @exception  IOException  if an I/O error occurs.
   * @see javax.crypto.CipherOutputStream#write(byte[], int,
int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void write(byte b[]) throws IOException {
  write(b, 0, b.length);
@@ -152,7 +152,7 @@
   * @param  off   the start offset in the data.
   * @param  len   the number of bytes to write.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(byte b[], int off, int len) throws 
IOException {

  obuffer = cipher.update(b, off, len);
@@ -174,7 +174,7 @@
   * the cipher's bl

Re: RFR: 8047721: @since should have JDK version

2014-06-23 Thread Henry Jen 
OK, I'll remove all @since JCE line, as the class already has @since 1.4 
as Joe pointed out earlier.


Uodated webrev at

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/

Cheers,
Henry


On 06/23/2014 10:04 AM, Bradford Wetmore wrote:

I would prefer that JCE1.2 be pulled out completely in the Cipher*
classes.  I will be sending you a separate note about JCE logistics.

Thanks for doing this cleanup.

Brad


On 6/20/2014 11:46 AM, Henry Jen wrote:

Hi,

Please review a trivial webrev to add JDK version to @since in a format
as Mark suggested[1].

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/

[1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html

Appened is the diff as in the webrev.

Cheers,
Henry


diff --git a/src/share/classes/java/lang/Package.java
b/src/share/classes/java/lang/Package.java
--- a/src/share/classes/java/lang/Package.java
+++ b/src/share/classes/java/lang/Package.java
@@ -107,6 +107,7 @@
   * loader to be found.
   *
   * @see ClassLoader#definePackage
+ * @since 1.2
   */
  public class Package implements java.lang.reflect.AnnotatedElement {
  /**
diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
b/src/share/classes/javax/crypto/CipherInputStream.java
--- a/src/share/classes/javax/crypto/CipherInputStream.java
+++ b/src/share/classes/javax/crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
   * @return  the next byte of data, or -1 if the end
of the
   *  stream is reached.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public int read() throws IOException {
  if (ostart >= ofinish) {
@@ -196,7 +196,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read(byte[], int, int)
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[]) throws IOException {
  return read(b, 0, b.length);
@@ -217,7 +217,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read()
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[], int off, int len) throws IOException {
  if (ostart >= ofinish) {
@@ -254,7 +254,7 @@
   * @param  n the number of bytes to be skipped.
   * @return the actual number of bytes skipped.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public long skip(long n) throws IOException {
  int available = ofinish - ostart;
@@ -277,7 +277,7 @@
   * @return the number of bytes that can be read from this
input stream
   * without blocking.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int available() throws IOException {
  return (ofinish - ostart);
@@ -292,7 +292,7 @@
   * stream.
   *
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void close() throws IOException {
  if (closed) {
@@ -321,7 +321,7 @@
   *  mark and reset methods.
   * @see java.io.InputStream#mark(int)
   * @see java.io.InputStream#reset()
- * @since   JCE1.2
+ * @since   1.4, JCE1.2
   */
  public boolean markSupported() {
  return false;
diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
b/src/share/classes/javax/crypto/CipherOutputStream.java
--- a/src/share/classes/javax/crypto/CipherOutputStream.java
+++ b/src/share/classes/javax/crypto/CipherOutputStream.java
@@ -114,7 +114,7 @@
   *
   * @param  b   the byte.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(int b) throws IOException {
  ibuffer[0] = (byte) b;
@@ -138,7 +138,7 @@
   * @exception  NullPointerException if b is null.
   * @exception  IOException  if an I/O error occurs.
   * @seejavax.crypto.CipherOutputStream#write(byte[], int,
int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void write(byte b[]) throws IOException {
  write(b, 0, b.length);
@@ -152,7 +152,7 @@
   * @param  off   the start offset in the data.
   * @param  len   the number of bytes to write.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(byte b[], int off, int len) throws IOException {
  obuffer = cipher.update(b, off, len);
@@ -174,7 +174,7 @@
   * the cipher's block size, no bytes will be written out.
   *
   * @exception  IO

Review request for 8047904: Runtime.loadLibrary throws SecurityException when security manager is installed

2014-06-23 Thread Mandy Chung 

The loadLibrary implementation is missing to wrap the call
to File.getCanonicalPath  method with doPrivileged block.

Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8047904/webrev.00/

thanks
Mandy

JDK9 project: XML/JAXP Approachability / Ease of Use

2014-06-23 Thread huizhe wang 

Hi,

We're planning on a jaxp project to address usability issues in the JAXP 
API. One of the complaints about the JAXP API is the number of lines of 
code that are needed to implement a simple task. Tasks that should take 
one or two lines often take ten or twelve lines instead. Consider the 
following example:


File file = new File(FILEPATH + "results.xml");
DocumentBuilderFactory factory = 
DocumentBuilderFactory.newInstance();

factory.setNamespaceAware(true);

XPathFactory xf = XPathFactory.newInstance();
XPath xp = xf.newXPath();
xp.setNamespaceContext(new NamespaceContext() {
@Override
public String getNamespaceURI(String prefix) {
return "http://example.com/users/";;
}

@Override
public String getPrefix(String namespaceURI) {
return "ns1";
}

@Override
public Iterator getPrefixes(String namespaceURI) {
ArrayList list = new ArrayList();
list.add("ns1");
return list.iterator();
}

});
try {
DocumentBuilder builder = factory.newDocumentBuilder();
Document document = builder.parse(file);
String s = (String) xp.evaluate(
"/ns1:Results/ns1:Row[ns1:USERID=2]/ns1:NAME[text()]",
document, XPathConstants.STRING);
System.out.println("Company: " + s);
} catch (ParserConfigurationException e) {
//creating DocumentBuilder
} catch (SAXException ex) {
//parsing
} catch (IOException ex) {
//parsing
} catch (XPathExpressionException ex) {
//xpath evaluation
}

The issues reflected in the above sample include:

*1. Too many abstractions*

As shown in the above sample, there are multiple layers of abstraction 
in the DOM and Xpath API: factory, builder, and document, XPathFactory 
and XPath.


*2. Unnecessary Pluggability*

The pluggability layer allows easily plugging in third party 
implementations. However, in many use cases where pluggability is not 
needed, it becomes the performance bottleneck for the applications.


*3. Too many unrelated checked exceptions*

There are four unrelated checked exceptions in the above example. None 
of them is recoverable. ParserConfigurationException actually reflects a 
design flaw in the DocumentBuilderFactory that allowed setting both a 
Schema and Schema Language. In practice, Exception is often used to 
avoid having to catch each of the checked exceptions.


*4. Lack of integration*

JAXP is an umbrella of several libraries. The sample code above 
demonstrated the lack of integration among them. First of all, A DOM 
Document and XPath have to be created separately. Secondly, as in the 
above case, the Document may be either namespace-aware or unaware, 
depending on the setting on the DOM Factory, which is unknown to XPath 
created later.




This project may have two aspects: one to provide APIs to get straight 
to the objects such as DOM Document, and another to provide convenient 
methods for some common use cases.  (*Note that, there is no intention 
to replace the existing API nor duplicate all of the features.)


For the example above, it could potentially be done in a couple of lines 
(this is just an illustration on how existing APIs could be simplified 
and may not reflect what the API would look like):


String company =
XMLDocument.fromFile(FILEPATH + "results.xml")
.evalXPath("/Results/Row[USERID=2]/NAME[text()]");
System.out.printf("Company: %s%n", company);


We would love to hear from you. Any thoughts, concerns, 
experiences/complaints would be very welcome.


Thanks,
Joe

Re: ThreadLocalRandom clinit troubles

2014-06-23 Thread Bradford Wetmore 

Martin,

Thanks for filing.  I was positive there was already a bug for this, but 
for the life of me I can't find it now.  There's some other more minor 
cleanup that needs to take place, but seems like I've been in 
escalation/firefighting mode for more than a year now and it hasn't 
bubbled to the top.


Brad


On 6/21/2014 9:05 PM, Martin Buchholz wrote:

While looking at NativePRNG, I filed

https://bugs.openjdk.java.net/browse/JDK-8047769

SecureRandom should be more frugal with file descriptors

If I run this java program on Linux

public class SecureRandoms {
 public static void main(String[] args) throws Throwable {
 new java.security.SecureRandom();
 }
}

it creates 6 file descriptors for /dev/random and /dev/urandom, as shown
by:

strace -q -ff -e open java SecureRandoms |& grep /dev/
[pid 20769] open("/dev/random", O_RDONLY) = 5
[pid 20769] open("/dev/urandom", O_RDONLY) = 6
[pid 20769] open("/dev/random", O_RDONLY) = 7
[pid 20769] open("/dev/random", O_RDONLY) = 8
[pid 20769] open("/dev/urandom", O_RDONLY) = 9
[pid 20769] open("/dev/urandom", O_RDONLY) = 10

Looking at jdk/src/solaris/classes/sun/security/provider/NativePRNG.java
it looks like 2 file descriptors are created for every variant of
NativePRNG, whether or not they are ever used. Which is wasteful. In
fact, you only ever need at most two file descriptors, one for
/dev/random and one for /dev/urandom.

Further, it would be nice if the file descriptors were closed when idle
and lazily re-created. Especially /dev/random should typically be used
at startup and never thereafter.


On Fri, Jun 20, 2014 at 7:59 AM, Alan Bateman mailto:alan.bate...@oracle.com>> wrote:

On 20/06/2014 15:02, Peter Levart wrote:


And, as Martin pointed out, it seems to be used for tests that
exercise particular responses from NameService API to test the
behaviour of JDK classes. It would be a shame for those tests to
go away.

We've been talking about removing it for many years because it has
been so troublesome. If we really need to having something for
testing then I don't think it needs to be general purpose, we can
get right of the lookup at least.

-Alan.

Re: Ready for Review : 8042469 : Launcher changes for native memory tracking scalability enhancement

2014-06-23 Thread Neil Toda 


I've spun a new webrev based on the comments for webrev-03:

http://cr.openjdk.java.net/~ntoda/8042469/webrev-04/

Changes are:

  1) java.c
a) add free(envName) line 1063
b) change from malloc() to JLI_MemAlloc() @ lines 1048 and 1056

Thanks

-neil

On 6/20/2014 4:45 PM, Kumar Srinivasan wrote:

Neil,

Generally looks good, yes JLI_* functions must be used, I missed that 
one.

Are you going to post another iteration ?

Kumar

On 6/20/2014 4:27 PM, Neil Toda wrote:


Thanks Joe.  It would have checked for NULL for me.
I'll use the JLI wrapper.

-neil

On 6/20/2014 4:04 PM, Joe Darcy wrote:
Memory allocation in the launcher should use one of the JLI_MemAlloc 
wrappers, if possible.


-Joe


On 06/20/2014 09:50 AM, Neil Toda wrote:


They should complain.  Thanks Zhengyu.  I'll make sure these are 
non-null.


-neil

On 6/20/2014 5:01 AM, Zhengyu Gu wrote:

Neil,

Thanks for quick implementation.

java.c:
  Did not check return values of malloc(), I wonder if source code 
analyzers will complain.


-Zhengyu

On 6/19/2014 8:29 PM, Neil Toda wrote:


Launcher support for modified Native Memory Tracking mechanism in 
JVM in JDK9.


Webrev  : http://cr.openjdk.java.net/~ntoda/8042469/webrev-03/
bug : https://bugs.openjdk.java.net/browse/JDK-8042469
CCC : http://ccc.us.oracle.com/8042469

Thanks.

-neil

Re: RFR: 8047721: @since should have JDK version

2014-06-23 Thread Bradford Wetmore 
I would prefer that JCE1.2 be pulled out completely in the Cipher* 
classes.  I will be sending you a separate note about JCE logistics.


Thanks for doing this cleanup.

Brad


On 6/20/2014 11:46 AM, Henry Jen wrote:

Hi,

Please review a trivial webrev to add JDK version to @since in a format
as Mark suggested[1].

http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/

[1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html

Appened is the diff as in the webrev.

Cheers,
Henry


diff --git a/src/share/classes/java/lang/Package.java
b/src/share/classes/java/lang/Package.java
--- a/src/share/classes/java/lang/Package.java
+++ b/src/share/classes/java/lang/Package.java
@@ -107,6 +107,7 @@
   * loader to be found.
   *
   * @see ClassLoader#definePackage
+ * @since 1.2
   */
  public class Package implements java.lang.reflect.AnnotatedElement {
  /**
diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
b/src/share/classes/javax/crypto/CipherInputStream.java
--- a/src/share/classes/javax/crypto/CipherInputStream.java
+++ b/src/share/classes/javax/crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
   * @return  the next byte of data, or -1 if the end
of the
   *  stream is reached.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public int read() throws IOException {
  if (ostart >= ofinish) {
@@ -196,7 +196,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read(byte[], int, int)
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[]) throws IOException {
  return read(b, 0, b.length);
@@ -217,7 +217,7 @@
   * the stream has been reached.
   * @exception  IOException  if an I/O error occurs.
   * @seejava.io.InputStream#read()
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int read(byte b[], int off, int len) throws IOException {
  if (ostart >= ofinish) {
@@ -254,7 +254,7 @@
   * @param  n the number of bytes to be skipped.
   * @return the actual number of bytes skipped.
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public long skip(long n) throws IOException {
  int available = ofinish - ostart;
@@ -277,7 +277,7 @@
   * @return the number of bytes that can be read from this
input stream
   * without blocking.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public int available() throws IOException {
  return (ofinish - ostart);
@@ -292,7 +292,7 @@
   * stream.
   *
   * @exception  IOException  if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void close() throws IOException {
  if (closed) {
@@ -321,7 +321,7 @@
   *  mark and reset methods.
   * @see java.io.InputStream#mark(int)
   * @see java.io.InputStream#reset()
- * @since   JCE1.2
+ * @since   1.4, JCE1.2
   */
  public boolean markSupported() {
  return false;
diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
b/src/share/classes/javax/crypto/CipherOutputStream.java
--- a/src/share/classes/javax/crypto/CipherOutputStream.java
+++ b/src/share/classes/javax/crypto/CipherOutputStream.java
@@ -114,7 +114,7 @@
   *
   * @param  b   the byte.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(int b) throws IOException {
  ibuffer[0] = (byte) b;
@@ -138,7 +138,7 @@
   * @exception  NullPointerException if b is null.
   * @exception  IOException  if an I/O error occurs.
   * @seejavax.crypto.CipherOutputStream#write(byte[], int,
int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
   */
  public void write(byte b[]) throws IOException {
  write(b, 0, b.length);
@@ -152,7 +152,7 @@
   * @param  off   the start offset in the data.
   * @param  len   the number of bytes to write.
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void write(byte b[], int off, int len) throws IOException {
  obuffer = cipher.update(b, off, len);
@@ -174,7 +174,7 @@
   * the cipher's block size, no bytes will be written out.
   *
   * @exception  IOException  if an I/O error occurs.
- * @since  JCE1.2
+ * @since  1.4, JCE1.2
   */
  public void flush() throws IOException {
  if (obuffer != null) {
@@ -198,7 +198,7 @@
   * stream.
   *
   * @excepti

Re: RFR JDK-5077522 : Duration.compare incorrect for some values

2014-06-23 Thread huizhe wang 

Thanks again Daniel and Lance!

Joe

On 6/21/2014 3:32 AM, Lance @ Oracle wrote:

Agree this is better and cleaner!


Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037 


Oracle Java Engineering
1 Network Drive 
Burlington, MA 01803 
lance.ander...@oracle.com 
Sent from my iPad

On Jun 21, 2014, at 4:27 AM, Daniel Fuchs > wrote:



Thanks Joe!

This is much cleaner indeed :-)

-- daniel

On 6/21/14 4:36 AM, huizhe wang wrote:

Thanks Daniel, Lance.

On 6/20/2014 3:02 AM, Daniel Fuchs wrote:

Hi Joe,

Thanks for the detailed explanation.
It really helps reviewing the fix!


Glad to know it helps. I thought this part of spec could be 
unfamiliar to people.




This looks reasonable to me. One minor nit is that you
could turn:

769 BigInteger maxintAsBigInteger = 
BigInteger.valueOf((long) Integer.MAX_VALUE);


Good catch!  I was going to do so but then forgot.

I also refactored the check method so that the checks can be done in 
one loop: 24 lines of code instead of the original 170. I feel good :-)


The other changes are purely clean-up and in one case, JavaDoc.

http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ 



Best regards,
Joe




into a static final constant in the class.

best regards,

-- daniel

On 6/17/14 9:19 PM, huizhe wang wrote:

Hi,

This is a long time compatibility issue: Duration.compare returns 
equal

for INDETERMINATE relations defined in XML Schema standard
(http://www.w3.org/TR/xmlschema-2/#duration-order) as listed in the
following table:

Relation
P*1Y* > P*364D* <> P*365D* <> P*366D* < P*367D*
P*1M* > P*27D* <> P*28D* <> P*29D* <> P*30D* <>
P*31D* < P*32D*
P*5M* > P*149D* <> P*150D* <> P*151D* <> P*152D* 
<>

P*153D* < P*154D*



The order-relation of two Duratoin values x and y is x < y iff s+x 
< s+y

for each qualified datetime s listed below:

 * 1696-09-01T00:00:00Z
 * 1697-02-01T00:00:00Z
 * 1903-03-01T00:00:00Z
 * 1903-07-01T00:00:00Z


The original implementation used Unix epoch, that is, 00:00:00 UTC 
on 1

January 1970, as s in the above calculation which violated the above
specification. A patch during JDK 6 development added correct
implementation of the spec, but it was unfortunately added after the
original calculation using Epoch time.

*The fix to the issue therefore is simply removing the calculation 
using
Epoch time.* I also consolidated the tedious max field value 
checks into

a method called checkMaxValue.

*Patch:*
http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ 



Test:
testCompareWithInderterminateRelation: this is a copy of the JCK test
that tests INDETERMINATE relations.
testVerifyOtherRelations: this is added to verify edge cases, e.g. 
+- 1

second to the original test cases. For example, to the original test:
PT525600M is P365D <> P1Y, I added "PT525599M59S", "<", "P1Y", and
PT527040M -> P366D <> P1Y, "PT527040M1S", ">", "P1Y"

Below is the test result:
Comparing P1Y and P365D: INDETERMINATE
Comparing P1Y and P366D: INDETERMINATE
Comparing P1M and P28D: INDETERMINATE
Comparing P1M and P29D: INDETERMINATE
Comparing P1M and P30D: INDETERMINATE
Comparing P1M and P31D: INDETERMINATE
Comparing P5M and P150D: INDETERMINATE
Comparing P5M and P151D: INDETERMINATE
Comparing P5M and P152D: INDETERMINATE
Comparing P5M and P153D: INDETERMINATE
Comparing PT2419200S and P1M: INDETERMINATE
Comparing PT2678400S and P1M: INDETERMINATE
Comparing PT31536000S and P1Y: INDETERMINATE
Comparing PT31622400S and P1Y: INDETERMINATE
Comparing PT525600M and P1Y: INDETERMINATE
Comparing PT527040M and P1Y: INDETERMINATE
Comparing PT8760H and P1Y: INDETERMINATE
Comparing PT8784H and P1Y: INDETERMINATE
Comparing P365D and P1Y: INDETERMINATE
Comparing P1Y and P364D: expected: GREATER actual: GREATER
Comparing P1Y and P367D: expected: LESSER actual: LESSER
Comparing P1Y2D and P366D: expected: GREATER actual: GREATER
Comparing P1M and P27D: expected: GREATER actual: GREATER
Comparing P1M and P32D: expected: LESSER actual: LESSER
Comparing P1M and P31DT1H: expected: LESSER actual: LESSER
Comparing P5M and P149D: expected: GREATER actual: GREATER
Comparing P5M and P154D: expected: LESSER actual: LESSER
Comparing P5M and P153DT1H: expected: LESSER actual: LESSER
Comparing PT2419199S and P1M: expected: LESSER actual: LESSER
Comparing PT2678401S and P1M: expected: GREATER actual: GREATER
Comparing PT31535999S and P1Y: expected: LESSER actual: LESSER
Comparing PT31622401S and P1Y: expected: GREATER actual: GREATER
Comparing PT525599M59S and P1Y: expected: LESSER actual: LESSER
Comparing PT527040M1S and P1Y: expected: GREATER actual: GREATER
Comparing PT8759H59M59S and P1Y: expected: LESSER actual: LESSER
Comparing PT8784H1S and P1Y: expected

Re: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Daniel Fuchs 

On 6/23/14 4:53 PM, Alan Bateman wrote:

On 23/06/2014 10:48, Daniel Fuchs wrote:

:

All in all - I feel our best options would be to revert to using
CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything
and live with the issue.
Hopefully some nio experts will chime in ;-)


The logging use of FileLock is a bit unusual but looking at it now then
I don't see the reason why it needs to use CREATE_NEW.


OK - thanks - in the discussion that ended up in the patch where
CREATE_NEW was introduced Jason (I think it was Jason) pointed at
https://bugs.openjdk.java.net/browse/JDK-4420020

I'm guessing here that maybe it would be wrong to use an already
existing lock file if you don't have the rights to write to
its directory - and that using CREATE_NEW ensures that you have
all necessary access rights all along the path.


I don't think
DELETE_ON_CLOSE is suitable here as that work is for transient work
files which isn't the case here. Instead it could use deleteOnExit to
have some chance of removing it on a graceful exit.


Right - I suggested a patch in another mail where I just use that.



BTW: Do you know why locks is Map? Would a Set do?


It certainly looks as if we could use a simple HashSet here.

Thanks Alan!

-- daniel



-Alan.

Re: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Alan Bateman 

On 23/06/2014 10:48, Daniel Fuchs wrote:

:

All in all - I feel our best options would be to revert to using
CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything
and live with the issue.
Hopefully some nio experts will chime in ;-)

The logging use of FileLock is a bit unusual but looking at it now then 
I don't see the reason why it needs to use CREATE_NEW. I don't think 
DELETE_ON_CLOSE is suitable here as that work is for transient work 
files which isn't the case here. Instead it could use deleteOnExit to 
have some chance of removing it on a graceful exit.


BTW: Do you know why locks is Map? Would a Set do?

-Alan.

Re: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Daniel Fuchs 

Hi,

I wonder whether the following patch would solve the
issue. In the case where the lock file already exists,
we check whether it's writable, and whether its directory
is writable, and if so, we open it for 'write+append',
otherwise, we continue with the next lock name.

I we have manage to open the lock file, then we proceed
with trying to lock it, as before.
If locking succeeds - we use that file.
If locking throws exception - we only fall through
if we actually created the file, otherwise we proceed
with the next lock name.

Would that make sense? It passes the CheckLockLocationTest
that was added for 6244047.
Note: I also added a deleteOnExit() hook for good measure.

-- daniel

--- a/src/share/classes/java/util/logging/FileHandler.java
+++ b/src/share/classes/java/util/logging/FileHandler.java
@@ -25,6 +25,7 @@

 package java.util.logging;

+import static java.nio.file.StandardOpenOption.APPEND;
 import static java.nio.file.StandardOpenOption.CREATE_NEW;
 import static java.nio.file.StandardOpenOption.WRITE;

@@ -35,6 +36,8 @@
 import java.io.OutputStream;
 import java.nio.channels.FileChannel;
 import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -434,24 +437,44 @@
 continue;
 }

+final Path lockFilePath = Paths.get(lockFileName);
+boolean fileCreated;
 try {
-lockFileChannel = 
FileChannel.open(Paths.get(lockFileName),

+lockFileChannel = FileChannel.open(lockFilePath,
 CREATE_NEW, WRITE);
+fileCreated = true;
 } catch (FileAlreadyExistsException ix) {
+// This may be a zombie file left over by a previous
+// execution. Reuse it - but since we didn't create
+// it we won't delete it either.
+if (Files.isRegularFile(lockFilePath)
+&& Files.isWritable(lockFilePath)
+&& 
Files.isWritable(lockFilePath.getParent())) {

+lockFileChannel = FileChannel.open(lockFilePath,
+WRITE, APPEND);
+fileCreated = false;
+} else {
 // try the next lock file name in the sequence
 continue;
 }
+}

 boolean available;
 try {
 available = lockFileChannel.tryLock() != null;
+if (available) {
+// we got the lock - ensure that the lock file
+// will be deleted on exit.
+lockFilePath.toFile().deleteOnExit();
+}
 // We got the lock OK.
 } catch (IOException ix) {
 // We got an IOException while trying to get the lock.
 // This normally indicates that locking is not 
supported
 // on the target directory.  We have to proceed 
without

-// getting a lock.   Drop through.
-available = true;
+// getting a lock.   Drop through, but only if we did
+// create the file...
+available = fileCreated;
 }
 if (available) {
 // We got the lock.  Remember it.



On 6/23/14 3:18 PM, Daniel Fuchs wrote:

Hi Jason,

On 6/23/14 2:51 PM, Jason Mehrens wrote:

Daniel,


My understanding is that changing CREATE_NEW to use CREATE would make
it work like does in JDK7.  Closing the lock files when the
FileHandler is unreferenced I is probably the fix for JDK-6774110:
lock file is not deleted when child logger is used.


I'll see if I can dig more info about the reason for the change.
Thanks for pointing at JDK-6774110 - I'll have a look at that as well.

-- daniel




If we could prove that system FileLock is mandatory we could use the
JDK7 behavior otherwise if they are advisory then use the JDK8
behavior.  If we knew the boot time of the JVM host O/S we could
delete all lock files older than the boot time + some constant.


Jason






Date: Mon, 23 Jun 2014 11:48:18 +0200
From: daniel.fu...@oracle.com
To: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net
Subject: Re: Zombie FileHandler locks can exhaust all available log
file locks.

Hi Jason,

Looking at the diff for 6244047 - I see that, as you pointed
out, the unwanted behavior described comes from the fact that
the new code is using CREATE_NEW - which prevents the 'zombie
lock files' from being reused.

I am not an expert in file locks - but I wonder whether we
could revert to using CREATE in

Re: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Daniel Fuchs 

Hi Jason,

On 6/23/14 2:51 PM, Jason Mehrens wrote:

Daniel,


My understanding is that changing CREATE_NEW to use CREATE would make it work 
like does in JDK7.  Closing the lock files when the FileHandler is unreferenced 
I is probably the fix for JDK-6774110: lock file is not deleted when child 
logger is used.


I'll see if I can dig more info about the reason for the change.
Thanks for pointing at JDK-6774110 - I'll have a look at that as well.

-- daniel




If we could prove that system FileLock is mandatory we could use the JDK7 
behavior otherwise if they are advisory then use the JDK8 behavior.  If we knew 
the boot time of the JVM host O/S we could delete all lock files older than the 
boot time + some constant.


Jason






Date: Mon, 23 Jun 2014 11:48:18 +0200
From: daniel.fu...@oracle.com
To: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net
Subject: Re: Zombie FileHandler locks can exhaust all available log file locks.

Hi Jason,

Looking at the diff for 6244047 - I see that, as you pointed
out, the unwanted behavior described comes from the fact that
the new code is using CREATE_NEW - which prevents the 'zombie
lock files' from being reused.

I am not an expert in file locks - but I wonder whether we
could revert to using CREATE instead: wouldn't tryLock() later
tell us if the file is used by another entity?

Another possibility would be to combine CREATE_NEW and
DELETE_ON_CLOSE, which according to StandardOpenOptions will
attempt to delete the file on JVM termination if close()
hasn't been called.
This probably wouldn't help in case on VM crash,
but it would help in the case demonstrated by your test below.
I have however some reluctance because I see that we call
FileChannel.close() in the case were the lock can't be obtained,
and I'm not sure what that would do...
Also StandardOpenOptions has some warning about using
DELETE_ON_CLOSE for files that can be opened concurrently
by other entities - so I'm not sure it would be appropriate.

The last possibility I see would be to change the lock HashMap to
take instances of a subclass of WeakReference as
values (instead of String), and add some code that attempts to
close & remove the lock file when the FileHandler is no longer referenced.
Again - this will probably not help in the case of crash, and also
adds the question on when the weak reference queue should be polled to
ensure that the no longer referenced FileChannel are closed in a
timely fashion.

All in all - I feel our best options would be to revert to using
CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything
and live with the issue.
Hopefully some nio experts will chime in ;-)

On another track - we could also make MAX configurable - but that
would just be shifting the issue around - wouldn't it?

best regards,

-- daniel

On 6/20/14 10:54 PM, Jason Mehrens wrote:

Daniel, Jim,


In JDK8 the FileHandler file locking was changed to use FileChannel.open with 
CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due 
to safety concerns about writing to the same log file. The FileHandler has an 
upper bound of 100 as the number of file locks that can be attempted to be 
acquired. Given the right pattern, enough time, and enough failures it seems at 
it is possible for a program to end up in a state where the FileHandler is 
surrounded by zombie lock files refuses log or perform a clean up action. 
(A.K.A Farmer Rick Grimes). This means that the lck files have to manually be 
deleted or the FileHandler will just fail with an IOException every time it is 
created.


A simple test to emulate crashing is to run (JDK7 vs. JDK8) the following code 
twice without deleting the log and lck files:


public static void main(String[] args) throws Exception {
System.out.println(System.getProperty("java.runtime.version"));
ReferenceQueue q = new ReferenceQueue<>();

for (int i=0; i<100; i++) {
WeakReference h = new WeakReference<>(
new FileHandler("./test_%u.log", 1, 2, true), q);
while (q.poll() != h) {
System.runFinalization();
System.gc();
System.runFinalization();
Thread.yield();
}
}

}


I understand that if you can't trust that the file locking always works then 
there isn't much that can be done. Leaving the number of locks as unbounded 
isn't really an option either. Seems like there should be a way to identify 
zombie lock files (ATOMIC_MOVE) and delete them. Any thoughts on this?

The source discussion on this can be found at 
http://stackoverflow.com/questions/24321098/is-java-util-logging-filehandler-in-java-8-broken


Regards,

Jason

RE: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Jason Mehrens 
Daniel,


My understanding is that changing CREATE_NEW to use CREATE would make it work 
like does in JDK7.  Closing the lock files when the FileHandler is unreferenced 
I is probably the fix for JDK-6774110: lock file is not deleted when child 
logger is used.


If we could prove that system FileLock is mandatory we could use the JDK7 
behavior otherwise if they are advisory then use the JDK8 behavior.  If we knew 
the boot time of the JVM host O/S we could delete all lock files older than the 
boot time + some constant.


Jason





> Date: Mon, 23 Jun 2014 11:48:18 +0200
> From: daniel.fu...@oracle.com
> To: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net
> Subject: Re: Zombie FileHandler locks can exhaust all available log file 
> locks.
>
> Hi Jason,
>
> Looking at the diff for 6244047 - I see that, as you pointed
> out, the unwanted behavior described comes from the fact that
> the new code is using CREATE_NEW - which prevents the 'zombie
> lock files' from being reused.
>
> I am not an expert in file locks - but I wonder whether we
> could revert to using CREATE instead: wouldn't tryLock() later
> tell us if the file is used by another entity?
>
> Another possibility would be to combine CREATE_NEW and
> DELETE_ON_CLOSE, which according to StandardOpenOptions will
> attempt to delete the file on JVM termination if close()
> hasn't been called.
> This probably wouldn't help in case on VM crash,
> but it would help in the case demonstrated by your test below.
> I have however some reluctance because I see that we call
> FileChannel.close() in the case were the lock can't be obtained,
> and I'm not sure what that would do...
> Also StandardOpenOptions has some warning about using
> DELETE_ON_CLOSE for files that can be opened concurrently
> by other entities - so I'm not sure it would be appropriate.
>
> The last possibility I see would be to change the lock HashMap to
> take instances of a subclass of WeakReference as
> values (instead of String), and add some code that attempts to
> close & remove the lock file when the FileHandler is no longer referenced.
> Again - this will probably not help in the case of crash, and also
> adds the question on when the weak reference queue should be polled to
> ensure that the no longer referenced FileChannel are closed in a
> timely fashion.
>
> All in all - I feel our best options would be to revert to using
> CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything
> and live with the issue.
> Hopefully some nio experts will chime in ;-)
>
> On another track - we could also make MAX configurable - but that
> would just be shifting the issue around - wouldn't it?
>
> best regards,
>
> -- daniel
>
> On 6/20/14 10:54 PM, Jason Mehrens wrote:
>> Daniel, Jim,
>>
>>
>> In JDK8 the FileHandler file locking was changed to use FileChannel.open 
>> with CREATE_NEW. If the file exists (locked or not) the FileHandler will 
>> rotate due to safety concerns about writing to the same log file. The 
>> FileHandler has an upper bound of 100 as the number of file locks that can 
>> be attempted to be acquired. Given the right pattern, enough time, and 
>> enough failures it seems at it is possible for a program to end up in a 
>> state where the FileHandler is surrounded by zombie lock files refuses log 
>> or perform a clean up action. (A.K.A Farmer Rick Grimes). This means that 
>> the lck files have to manually be deleted or the FileHandler will just fail 
>> with an IOException every time it is created.
>>
>>
>> A simple test to emulate crashing is to run (JDK7 vs. JDK8) the following 
>> code twice without deleting the log and lck files:
>>
>>
>> public static void main(String[] args) throws Exception {
>> System.out.println(System.getProperty("java.runtime.version"));
>> ReferenceQueue q = new ReferenceQueue<>();
>>
>> for (int i=0; i<100; i++) {
>> WeakReference h = new WeakReference<>(
>> new FileHandler("./test_%u.log", 1, 2, true), q);
>> while (q.poll() != h) {
>> System.runFinalization();
>> System.gc();
>> System.runFinalization();
>> Thread.yield();
>> }
>> }
>>
>> }
>>
>>
>> I understand that if you can't trust that the file locking always works then 
>> there isn't much that can be done. Leaving the number of locks as unbounded 
>> isn't really an option either. Seems like there should be a way to identify 
>> zombie lock files (ATOMIC_MOVE) and delete them. Any thoughts on this?
>>
>> The source discussion on this can be found at 
>> http://stackoverflow.com/questions/24321098/is-java-util-logging-filehandler-in-java-8-broken
>>
>>
>> Regards,
>>
>> Jason
>>
>

Re: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Daniel Fuchs 

Hi Jason,

Looking at the diff for 6244047 - I see that, as you pointed
out, the unwanted behavior described comes from the fact that
the new code is using CREATE_NEW - which prevents the 'zombie
lock files' from being reused.

I am not an expert in file locks - but I wonder whether we
could revert to using CREATE instead: wouldn't tryLock() later
tell us if the file is used by another entity?

Another possibility would be to combine CREATE_NEW and
DELETE_ON_CLOSE, which according to StandardOpenOptions will
attempt to delete the file on JVM termination if close()
hasn't been called.
This probably wouldn't help in case on VM crash,
but it would help in the case demonstrated by your test below.
I have however some reluctance because I see that we call
FileChannel.close() in the case were the lock can't be obtained,
and I'm not sure what that would do...
Also StandardOpenOptions has some warning about using
DELETE_ON_CLOSE for files that can be opened concurrently
by other entities - so I'm not sure it would be appropriate.

The last possibility I see would be to change the lock HashMap to
take instances of a subclass of WeakReference as
values (instead of String), and add some code that attempts to
close & remove the lock file when the FileHandler is no longer referenced.
Again - this will probably not help in the case of crash, and also
adds the question on when the weak reference queue should be polled to
ensure that the no longer referenced FileChannel are closed in a
timely fashion.

All in all - I feel our best options would be to revert to using
CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything
and live with the issue.
Hopefully some nio experts will chime in ;-)

On another track - we could also make MAX configurable - but that
would just be shifting the issue around - wouldn't it?

best regards,

-- daniel

On 6/20/14 10:54 PM, Jason Mehrens wrote:

Daniel, Jim,


In JDK8 the FileHandler file locking was changed to use FileChannel.open with 
CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due 
to safety concerns about writing to the same log file. The FileHandler has an 
upper bound of 100 as the number of file locks that can be attempted to be 
acquired. Given the right pattern, enough time, and enough failures it seems at 
it is possible for a program to end up in a state where the FileHandler is 
surrounded by zombie lock files refuses log or perform a clean up action. 
(A.K.A Farmer Rick Grimes). This means that the lck files have to manually be 
deleted or the FileHandler will just fail with an IOException every time it is 
created.


A simple test to emulate crashing is to run (JDK7 vs. JDK8) the following code 
twice without deleting the log and lck files:


public static void main(String[] args) throws Exception {
System.out.println(System.getProperty("java.runtime.version"));
ReferenceQueue q = new ReferenceQueue<>();

for (int i=0; i<100; i++) {
  WeakReference h = new WeakReference<>(
  new FileHandler("./test_%u.log", 1, 2, true), q);
  while (q.poll() != h) {
  System.runFinalization();
  System.gc();
  System.runFinalization();
  Thread.yield();
  }
   }

}


I understand that if you can't trust that the file locking always works then 
there isn't much that can be done. Leaving the number of locks as unbounded 
isn't really an option either. Seems like there should be a way to identify 
zombie lock files (ATOMIC_MOVE) and delete them.  Any thoughts on this?

The source discussion on this can be found at 
http://stackoverflow.com/questions/24321098/is-java-util-logging-filehandler-in-java-8-broken


Regards,

Jason