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

2014-06-24 Thread Alan Bateman

On 24/06/2014 00:45, 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.


open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
bug link https://bugs.openjdk.java.net/browse/JDK-6642881
Having setAccessible throw SecurityException when there isn't a security 
manager set is a bit odd but as you point out, setAccessible is 
specified to allow that. The changes looks okay, although I don't think 
strictly necessary.


-Alan.


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

2014-06-24 Thread Alan Bateman

On 24/06/2014 01:14, huizhe wang wrote:

:



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.
If there is a test failure then whoever runs into it will need to 
decypher what the test is about. I would think that it would at least be 
helpful to have a short summary at the top to give some indication as to 
what it is doing. Are we even sure that all the issues are accessible to 
all in JIRA?


-Alan


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

2014-06-24 Thread Paul Sandoz

On Jun 24, 2014, at 2:42 AM, Mike Duigou mike.dui...@oracle.com wrote:

 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.

That seems like a reasonable approach and it's in sync with the behaviour of 
Map.replaceAll.


 Additionally the javadoc is updated to inform users that a ClassCastException 
 may result if the proposed replacement is unacceptable.
 

No users will see the JavaDoc on Collections.CheckedList since it is package 
private, plus i think it redundant. Any such associated documentation would 
need to be on the public static method, and i am not sure we really need to say 
anything more than what is already said:

3388  * Any attempt to insert an element of the wrong type will result in
3389  * an immediate {@link ClassCastException}.  Assuming a list contains


 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/
 

Are there existing tests for the checked Map.replaceAll (for keys and values)?

Paul.
 
 This change will be backported to Java 8.
 
 Mike



A question about Depth of container annotations

2014-06-24 Thread deven you
Hi All,

I have a question about repeated annotation depth. If this is not the
proper mailing list group, please tell me where I should send it to.

My question will be about the depth of container annotations. For instance,
assume there are 3 annotations.
- RepeatedAnn
- ContainerAnn
- ContainerContainerAnn

So, ContainerContainerAnn can have ContainerAnn and that can also have
RepeatbleAnn in it.

In this case, get*AnnotationsByType(RepeatableAnn) APIs wont return
repeteableAnns in depth 2.

Java docs don't talk about the depth. I wonder if the get*AnnotationsByType
api should return the annotations of all depth?

If we have below annotations:
@Retention(RetentionPolicy.RUNTIME)
@Repeatable(ContainerAnn.class)
@interface RepeatableAnn { String value(); }

@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Repeatable(ContainerContainerAnn.class)
@interface ContainerAnn { RepeatableAnn[] value(); }

@Inherited
@Retention(RetentionPolicy.RUNTIME)
@interface ContainerContainerAnn { ContainerAnn[] value(); }

And the main class is annotated by :

@ContainerAnn({@RepeatableAnn(Parent-3)})
@ContainerAnn({@RepeatableAnn(Parent-4)})
public class Test { ...

when we call getAnnotationsByType(RepeatableAnn.class) on this class, we
get nothing because RepeatableAnn is contained by ContainerAnn which is
also contained by ContainerContainerAnn. In other words, RepeatableAnn is
in depth 2 and get*AnnotationsByType APIs only searches depth 1. The test
results are:

/home/deven/jdk8_20/jdk1.8.0_20/bin$ java repeated.Test
CLASS = repeated.Test
getDeclaredAnnotationsByType(RepeatableAnn.class)
getDeclaredAnnotationsByType(ContainerAnn.class)

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)])

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])
getDeclaredAnnotationsByType(ContainerContainerAnn.class)

@repeated.ContainerContainerAnn(value=[@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)]),
@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])])
getAnnotationsByType(RepeatableAnn.class)
getAnnotationsByType(ContainerAnn.class)

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)])

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])
getAnnotationsByType(ContainerContainerAnn.class)

@repeated.ContainerContainerAnn(value=[@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)]),
@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])])


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

2014-06-24 Thread Peter Levart

On 06/24/2014 01:45 AM, 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.


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

Thanks,
Coleen


Hi Coleen,

So hackers are prevented from hacking...

Out of curiosity, what would happen if someone changed the classLoader 
field of some Class object? I guess VM still has it's own notion of the 
class' class loader, right? Only the code that directly uses the 
Class.getClassLoader() (and Unsafe.defineClass0) methods would be 
affected...


While we're at it, does this change in any way affect the GC logic? Will 
GC now navigate the classLoader field during marking but previously 
didn't ? Will this have any GC performance penalty ?


Regards, Peter



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

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

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote:


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

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore 
coleen.phillim...@oracle.com wrote:

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


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



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

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

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


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


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


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


David
-



cheers
/Joel







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

2014-06-24 Thread Frederic Parain

Hi Coleen,

It seems that there's still a reference to JVM_GetClassLoader
in file jdk/src/share/native/common/check_code.c. The code looks
like dead code, but it would be nice to clean it up.

Thanks,

Fred

On 06/24/2014 01:45 AM, 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.

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

Thanks,
Coleen

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

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

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote:


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

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore
coleen.phillim...@oracle.com wrote:

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

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

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


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


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

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


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

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


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

David
-



cheers
/Joel





--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com


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

2014-06-24 Thread Chris Hegarty
Looks good to me Mike.

I agree with Paul’s comment that the javadoc change will never be seen in the 
public docs, but I still think it is a reasonable addition for future 
maintainers.

Trivially, you should probably add @SuppressWarnings(unchecked”) to 
typeCheck(Object).

-Chris.

On 24 Jun 2014, at 01:42, Mike Duigou mike.dui...@oracle.com wrote:

 Hello all;
 
 This changeset corrects a reported problem with the lists returned by 
 Collections.checkedList(). Since Java 8 the replaceAll() method on checked 
 lists has erroneously allowed the operator providing replacements to provide 
 illegal replacement values which are then stored, unchecked into the wrapped 
 list.
 
 This changeset adds a check on the proposed replacement value and throws a 
 ClassCastException if the replacement value is incompatible with the list. 
 Additionally the javadoc is updated to inform users that a ClassCastException 
 may result if the proposed replacement is unacceptable.
 
 Note that this changeset takes the strategy of failing when the illegal value 
 is encountered. Replacements of earlier items in the list are retained.
 
 jbsbug: https://bugs.openjdk.java.net/browse/JDK-8047795
 webrev: http://cr.openjdk.java.net/~mduigou/JDK-8047795/0/webrev/
 
 This change will be backported to Java 8.
 
 Mike



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

2014-06-24 Thread Kumar Srinivasan

Neil,

Some nits:

TestSpecialArgs.java:

extra space
105 for ( String line : tr.testOutput) {



This is very C'ish, I suggest.
 -106 int index;
 -107 if ((index = line.indexOf(envVarPidString)) = 0) {

 +106 int index =  line.indexOf(envVarPidString);
 +107 if (index = 0) {


Needs space
-146 launcherPid = line.substring(sindex,tindex);
+146 launcherPid = line.substring(sindex, tindex);

Needs space

-168 if (!tr.contains(NativeMemoryTracking: got value 
+NMT_Option_Value)) {
+168 if (!tr.contains(NativeMemoryTracking: got value  + 
NMT_Option_Value)) {


Thanks
Kumar


On 6/23/2014 10:26 AM, Neil Toda wrote:


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 6642881: Improve performance of Class.getClassLoader()

2014-06-24 Thread Coleen Phillimore


Hi Peter,

On 6/24/14, 4:23 AM, Peter Levart wrote:

On 06/24/2014 01:45 AM, 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.


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

Thanks,
Coleen


Hi Coleen,

So hackers are prevented from hacking...

Out of curiosity, what would happen if someone changed the classLoader 
field of some Class object? I guess VM still has it's own notion of 
the class' class loader, right? Only the code that directly uses the 
Class.getClassLoader() (and Unsafe.defineClass0) methods would be 
affected...


True, Class.getClassLoader() calls would be affected but we may use this 
call for security checks.  I'm not really an expert on this, but we 
thought it wouldn't be safe to allow user access to classLoader.




While we're at it, does this change in any way affect the GC logic? 
Will GC now navigate the classLoader field during marking but 
previously didn't ? Will this have any GC performance penalty ?


I actually ran this through our performance testing and got a good 
improvement in one of the scimark benchmarks for no reason I could 
explain (and lost the results), but none of the other tests were 
affected.  GC would have to mark this new field for full collections but 
not young collections because it's only set during initialization.   I 
wouldn't expect this field to have any negative performance for GC.


Thanks,
Coleen



Regards, Peter



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

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

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote:


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

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore 
coleen.phillim...@oracle.com wrote:

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


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



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

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

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


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


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


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


David
-



cheers
/Joel









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

2014-06-24 Thread Coleen Phillimore


Fred,

Thank you for finding this.  Yes, I meant to clean this up with the bug 
to remove JVM_GetClassLoader but I should remove this with this change 
instead, since the other change will be in hotspot only.

Yes, it's dead code.

Thanks!
Coleen

On 6/24/14, 4:41 AM, Frederic Parain wrote:

Hi Coleen,

It seems that there's still a reference to JVM_GetClassLoader
in file jdk/src/share/native/common/check_code.c. The code looks
like dead code, but it would be nice to clean it up.

Thanks,

Fred

On 06/24/2014 01:45 AM, 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.

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

Thanks,
Coleen

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

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

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote:


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

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore
coleen.phillim...@oracle.com wrote:

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

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

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



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


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

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


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

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


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

David
-



cheers
/Joel









Re: RFR: JDK-8044629: (reflect) Constructor.getAnnotatedReceiverType() returns wrong value

2014-06-24 Thread Paul Sandoz

On Jun 17, 2014, at 6:52 PM, Joel Borggrén-Franck joel.fra...@oracle.com 
wrote:

 Hi,
 
 Can I get a review for this fix and javadoc clarification for 
 https://bugs.openjdk.java.net/browse/JDK-8044629
 
 The problem is with potentially annotated receiver parameters, they only 
 exist for inner class constructors, so this fix makes sure that a ctor is for 
 an inner class or returns null (as for a static method) otherwise.
 
 CCC has been approved.
 
 Webrev: http://cr.openjdk.java.net/~jfranck/8044629/webrev.00/
 

+1

I never quite realised just how convoluted it was to determine that a class is 
an inner class.

Paul.



Re: ThreadLocalRandom clinit troubles

2014-06-24 Thread Peter Levart

Hi Martin,

On 06/22/2014 07:12 PM, Martin Buchholz wrote:

We know that loading the networking machinery is problematic.  On Linux we
would be content to hard-code a read from /dev/urandom, which is safer and
strictly more random than the existing network hardware determination, but
y'all will reject that as too system-dependent (insufficient machinery!).
H  maybe not  as long as we code up a good fallback ...

I learned that SecureRandom by default on Unix uses /dev/random for seed
bytes and /dev/urandom for nextBytes.

Here's my proposal, that in the default case on Unix doesn't load any
machinery, and as a fallback loads the SecureRandom machinery instead of
the network machinery, while maintaining the ultra-secure behavior of the
java.util.secureRandomSeed system property:


 private static long initialSeed() {
 byte[] seedBytes = initialSeedBytes();
 long s = (long)(seedBytes[0])  0xffL;
 for (int i = 1; i  seedBytes.length; ++i)
 s = (s  8) | ((long)(seedBytes[i])  0xffL);
 return s ^ mix64(System.currentTimeMillis()) ^
mix64(System.nanoTime());
 }

 private static byte[] initialSeedBytes() {
 String pp = java.security.AccessController.doPrivileged(
 new sun.security.action.GetPropertyAction(
 java.util.secureRandomSeed));
 boolean secureRandomSeed = (pp != null 
pp.equalsIgnoreCase(true));
 if (secureRandomSeed)
 return java.security.SecureRandom.getSeed(8);
 final byte[] seedBytes = new byte[8];
 File seedSource = new File(/dev/urandom);
 if (seedSource.exists()) {
 try (FileInputStream stream = new FileInputStream(seedSource)) {
 if (stream.read(seedBytes) == 8)
 return seedBytes;
 } catch (IOException ignore) { }
 }
 new java.security.SecureRandom().nextBytes(seedBytes);
 return seedBytes;
 }


So on platforms lacking /dev/urandom special file (Windows only?), the 
fall-back would be to use SecureRandom.nextBytes() whatever default 
SecureRandom PRNG is configured to be on such platform. This might be a 
user-supplied SecureRandom PRNG. The user might want it's own default 
SecureRandom but not use it for TLR's seed computation (unless requested 
by java.util.secureRandomSeed system property).


I would rather use SecureRandom.generateSeed() instance method instead 
of SecureRandom.nextBytes(). Why? Because every SecureRandom instance 
has to initialize it's seed 1st before getBytes() can provide the next 
random bytes from the PRNG. Since we only need the 1st 8 bytes from the 
SecureRandom instance to initialize TLR's seeder, we might as well 
directly call the SecureRandom.generateSeed() method. That's one reason. 
The other is the peculiar initialization of default SecureRandom 
algorithm on Windows (see below)...


Even if user does not provide it's own default SecureRandom PRNG, there 
are basically two algorithms that are used by default on OpenJDK:


On Solaris/Linux/Mac/AIX:

the NativePRNG algorithm from SUN provider (implemented by 
sun.security.provider.NativePRNG) which uses /dev/random for getBytes() 
and /dev/urandom (or whatever is configured with java.security.egd or 
securerandom.source system properties) for generateSeed(), and


On Windows:

the SHA1PRNG algorithm from SUN provider (implemented by 
sun.security.provider.SecureRandom) which uses SHA1 message digest for 
generating random numbers with seed computed by gathering system entropy...



The most problematic one is the default on Windows platform (the 
platform that does not have the /dev/urandom special file and would be 
used as a fall-back by your proposal) - 
sun.security.provider.SecureRandom. This one seeds itself by 
constructing an instance of itself with the result returned from 
SeedGenerator.getSystemEntropy() method. This method, among other 
things, uses networking code to gather system entropy:


...
md.update
(InetAddress.getLocalHost().toString().getBytes());
...

This is problematic since it not only initializes NameService providers 
but also uses them to resolve local host name. This can block for 
several seconds on unusual configurations and was the main motivation to 
replace similar code in TLR with code that uses 
NetworkInterface.getHardwareAddress() instead. Using 
SecureRandom.generateSeed() instead of SecureRandom.getBytes() does not 
invoke SeedGenerator.getSystemEntropy(), but uses just 
SeedGenerator.generateSeed(), which by default on Windows uses 
ThreadedSeedGenerator.getSeedBytes()...


I showed how we could suppress NameService providers initialization 
while still using NetworkInterface.getHardwareAddress() for TLR's seeder 
initialization. If that's not enough and we would like to get-away 
without using networking code at all, then I propose the following:

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

2014-06-24 Thread Coleen Phillimore


On 6/24/14, 4:41 AM, Frederic Parain wrote:

Hi Coleen,

It seems that there's still a reference to JVM_GetClassLoader
in file jdk/src/share/native/common/check_code.c. The code looks
like dead code, but it would be nice to clean it up.


I removed this code.  There are no other instances of the macro 
BROKEN_JAVAC.  I update the copyrights when I commit the changeset.


http://cr.openjdk.java.net/~coleenp/6642881_jdk_5/

Thanks!
Coleen


Thanks,

Fred

On 06/24/2014 01:45 AM, 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.

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

Thanks,
Coleen

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

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

Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung mandy.ch...@oracle.com wrote:


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

Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore
coleen.phillim...@oracle.com wrote:

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

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

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



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


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

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


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

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


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

David
-



cheers
/Joel









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

2014-06-24 Thread Paul Sandoz

On Jun 23, 2014, at 10:23 PM, Mandy Chung mandy.ch...@oracle.com wrote:

 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/
 

Looks ok to me.

Paul.


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

2014-06-24 Thread huizhe wang


On 6/24/2014 12:19 AM, Alan Bateman wrote:

On 24/06/2014 01:14, huizhe wang wrote:

:



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.
If there is a test failure then whoever runs into it will need to 
decypher what the test is about. I would think that it would at least 
be helpful to have a short summary at the top to give some indication 
as to what it is doing. Are we even sure that all the issues are 
accessible to all in JIRA?


If there is a test failure, we can narrow it down to the cause pretty 
quickly since we have a context, that is, the current patch. Rather than 
some comments, we generally rely on the test code itself to identify the 
culprit or browse the original bug report to understand the whole story 
(when change happened, the whole history and etc.).


Furthermore, they don't really fail often, in fact, some of the tests 
have never failed because of the nature of related fixes. For someone 
like Patrick who doesn't have knowledge of the original fixes or bug 
reports to go into them and try to figure out a proper write-up, it may 
not be an easy task. There are hundreds of tests, plus the function 
tests that did not have comments either. I'm just not sure it's worth it 
to spend all that time (and then, not used in most of the cases :-) ).


-Joe



-Alan




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

2014-06-24 Thread Jason Mehrens
Daniel,


With regard to JDK-4420020, in the original webrev Jim was incorrectly using 
java.io.File.canWrite() but that webrev was replaced by the current version. 
The NIO.2 code performs the effective access checks correctly.


With regard to JDK-6244047 my concern was that checking the permissions and 
preconditions up front is a small 'TOCTOU' race and redundant because the next 
step after that is to open the FileChannel. I would assume both CREATE or 
CREATE_NEW plus WRITE would perform the effective access checks on open.

The use of delete on exit is a deal breaker because you can't undo a 
registration on close of the FileHandler which can trigger an OOME. See 
https://bugs.openjdk.java.net/browse/JDK-4872014. I've used custom proxy 
handlers that generate a file name based off of the LogRecord which results in 
generating lots of file names and opening and close the FileHandler on demand.


If the behavior is reverted to use CREATE instead of CREATE_NEW then we have to 
deal with JDK-8031438 because any user code can lock a file ahead of opening 
the FileHandler.

Jason




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

 On 6/23/14 4:53 PM, Alan Bateman wrote:
 On 23/06/2014 10:48, Daniel Fuchs wrote:
 :

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

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

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

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

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

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


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

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

 Thanks Alan!

 -- daniel


 -Alan.

 

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

2014-06-24 Thread Daniel Fuchs

Hi Jason,

Thanks for the feedback!

On 6/24/14 7:08 PM, Jason Mehrens wrote:

Daniel,


With regard to JDK-4420020, in the original webrev Jim was incorrectly using 
java.io.File.canWrite() but that webrev was replaced by the current version. 
The NIO.2 code performs the effective access checks correctly.


Thanks.


With regard to JDK-6244047 my concern was that checking the permissions and 
preconditions up front is a small 'TOCTOU' race and redundant because the next 
step after that is to open the FileChannel. I would assume both CREATE or 
CREATE_NEW plus WRITE would perform the effective access checks on open.


OK.


The use of delete on exit is a deal breaker because you can't undo a 
registration on close of the FileHandler which can trigger an OOME. See 
https://bugs.openjdk.java.net/browse/JDK-4872014. I've used custom proxy 
handlers that generate a file name based off of the LogRecord which results in 
generating lots of file names and opening and close the FileHandler on demand.


ah. h. I didn't think there would be that many FileHandlers...
Well - I guess that if we find a way to reuse the zombie
lock files then we don't really need the delete on exit.
Someone creating a FileHandler programmatically should be responsible
for closing it - so if an application creates FileHandlers without
closing them then it's a bug in the application.


If the behavior is reverted to use CREATE instead of CREATE_NEW then we have to 
deal with JDK-8031438 because any user code can lock a file ahead of opening 
the FileHandler.


Yes - I discovered that while writing a test for my suggested fix ;-)
Catching OverlappingFileLockException in FileHandler.openFiles
and setting available to false when it's thrown fixes the issue.

So let's just remove the deleteOnExit  add the catch for 
OverlappingFileLockException to my patch and we should be

good.

On the other hand we could just use replace CREATE_NEW by
CREATE but I have some misgivings about the part that
sets available to true when tryLock throws an IOException.
I'm not sure we should be doing that if we haven't actually
created the lock file. So I think I'd prefer keeping the
two steps behavior - first try CREATE_NEW - then attempt
to use CREATE if CREATE_NEW failed...
I'm not sure CREATE will check the access rights for writing
in the directory if the file already exists - so checking
the directory for write access in that case is probably
the right thing to do.

Would you agree?

-- daniel



Jason





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

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

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

:

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


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


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

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


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


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



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


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

Thanks Alan!

-- daniel



-Alan.







Re: JDK 9 RFR of JDK-8048014: Update java.lang.SafeVararags for private methods

2014-06-24 Thread Paul Benedict
Will there be another ticket to update the section (9.6.4.7.) in JLS 9?


Cheers,
Paul


On Tue, Jun 24, 2014 at 12:27 PM, Lance Andersen lance.ander...@oracle.com
wrote:

 +1
 On Jun 24, 2014, at 1:13 PM, Joe Darcy joe.da...@oracle.com wrote:

  Hello,
 
  Please review the libraries update portion of allowing @SafeVarargs on
 private instance methods:
 
 JDK-8048014: Update java.lang.SafeVararags for private methods
 
  The patch is
 
  diff -r 6c26f18d9bc0 src/share/classes/java/lang/SafeVarargs.java
  --- a/src/share/classes/java/lang/SafeVarargs.javaMon Jun 23
 12:12:30 2014 -0700
  +++ b/src/share/classes/java/lang/SafeVarargs.javaTue Jun 24
 10:03:21 2014 -0700
  @@ -1,5 +1,5 @@
  /*
  - * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights
 reserved.
  + * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights
 reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
  @@ -45,7 +45,7 @@
   * li  the declaration is a fixed arity method or constructor
   *
   * li the declaration is a variable arity method that is neither
  - * {@code static} nor {@code final}.
  + * {@code static} nor {@code final} nor {@code private}.
   *
   * /ul
   *
 
  The bulk of the change to allow @SafeVarargs on private methods,
 including the tests, are over in the langtools repo with javac updates. The
 javac changes have been reviewed and approved on compiler-dev:
 
 
 http://mail.openjdk.java.net/pipermail/compiler-dev/2014-June/008855.html
 
  Thanks,
 
  -Joe




 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: JDK 9 RFR of JDK-8048014: Update java.lang.SafeVararags for private methods

2014-06-24 Thread Joe Darcy

On 06/24/2014 10:43 AM, Paul Benedict wrote:

Will there be another ticket to update the section (9.6.4.7.) in JLS 9?


JDK-8047159: 9.6.4.7: Allow @SafeVarargs on private methods
https://bugs.openjdk.java.net/browse/JDK-8047159

Cheers,

-Joe




Cheers,
Paul


On Tue, Jun 24, 2014 at 12:27 PM, Lance Andersen 
lance.ander...@oracle.com mailto:lance.ander...@oracle.com wrote:


+1
On Jun 24, 2014, at 1:13 PM, Joe Darcy joe.da...@oracle.com
mailto:joe.da...@oracle.com wrote:

 Hello,

 Please review the libraries update portion of allowing
@SafeVarargs on private instance methods:

JDK-8048014: Update java.lang.SafeVararags for private methods

 The patch is

 diff -r 6c26f18d9bc0 src/share/classes/java/lang/SafeVarargs.java
 --- a/src/share/classes/java/lang/SafeVarargs.javaMon Jun 23
12:12:30 2014 -0700
 +++ b/src/share/classes/java/lang/SafeVarargs.javaTue Jun 24
10:03:21 2014 -0700
 @@ -1,5 +1,5 @@
 /*
 - * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All
rights reserved.
 + * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All
rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or
modify it
 @@ -45,7 +45,7 @@
  * li  the declaration is a fixed arity method or constructor
  *
  * li the declaration is a variable arity method that is neither
 - * {@code static} nor {@code final}.
 + * {@code static} nor {@code final} nor {@code private}.
  *
  * /ul
  *

 The bulk of the change to allow @SafeVarargs on private methods,
including the tests, are over in the langtools repo with javac
updates. The javac changes have been reviewed and approved on
compiler-dev:


http://mail.openjdk.java.net/pipermail/compiler-dev/2014-June/008855.html

 Thanks,

 -Joe




Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com mailto:lance.ander...@oracle.com









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

2014-06-24 Thread Mike Duigou

On Jun 24 2014, at 01:46 , Chris Hegarty chris.hega...@oracle.com wrote:

 Looks good to me Mike.
 
 I agree with Paul’s comment that the javadoc change will never be seen in the 
 public docs, but I still think it is a reasonable addition for future 
 maintainers.
 
 Trivially, you should probably add @SuppressWarnings(unchecked”) to 
 typeCheck(Object).

Done.


 
 -Chris.
 
 On 24 Jun 2014, at 01:42, Mike Duigou mike.dui...@oracle.com wrote:
 
 Hello all;
 
 This changeset corrects a reported problem with the lists returned by 
 Collections.checkedList(). Since Java 8 the replaceAll() method on checked 
 lists has erroneously allowed the operator providing replacements to provide 
 illegal replacement values which are then stored, unchecked into the wrapped 
 list.
 
 This changeset adds a check on the proposed replacement value and throws a 
 ClassCastException if the replacement value is incompatible with the list. 
 Additionally the javadoc is updated to inform users that a 
 ClassCastException may result if the proposed replacement is unacceptable.
 
 Note that this changeset takes the strategy of failing when the illegal 
 value is encountered. Replacements of earlier items in the list are retained.
 
 jbsbug: https://bugs.openjdk.java.net/browse/JDK-8047795
 webrev: http://cr.openjdk.java.net/~mduigou/JDK-8047795/0/webrev/
 
 This change will be backported to Java 8.
 
 Mike
 



Re: A question about Depth of container annotations

2014-06-24 Thread Joe Darcy

Hello,

The behavior you're interested in is defined in the AnnotatedElement 
specification:


http://docs.oracle.com/javase/8/docs/api/java/lang/reflect/AnnotatedElement.html

In brief, none of the methods in AnnotatedElement look through more than 
one level of containment. (Likewise, when writing repeated annotation in 
source code, the compiler will only create one level of containers.)


You may be interested in reviewing design discussions of this feature 
that occurred on


http://mail.openjdk.java.net/mailman/listinfo/enhanced-metadata-spec-discuss

Cheers,

-Joe

On 06/24/2014 01:21 AM, deven you wrote:

Hi All,

I have a question about repeated annotation depth. If this is not the
proper mailing list group, please tell me where I should send it to.

My question will be about the depth of container annotations. For instance,
assume there are 3 annotations.
- RepeatedAnn
- ContainerAnn
- ContainerContainerAnn

So, ContainerContainerAnn can have ContainerAnn and that can also have
RepeatbleAnn in it.

In this case, get*AnnotationsByType(RepeatableAnn) APIs wont return
repeteableAnns in depth 2.

Java docs don't talk about the depth. I wonder if the get*AnnotationsByType
api should return the annotations of all depth?

If we have below annotations:
@Retention(RetentionPolicy.RUNTIME)
@Repeatable(ContainerAnn.class)
@interface RepeatableAnn { String value(); }

@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Repeatable(ContainerContainerAnn.class)
@interface ContainerAnn { RepeatableAnn[] value(); }

@Inherited
@Retention(RetentionPolicy.RUNTIME)
@interface ContainerContainerAnn { ContainerAnn[] value(); }

And the main class is annotated by :

@ContainerAnn({@RepeatableAnn(Parent-3)})
@ContainerAnn({@RepeatableAnn(Parent-4)})
public class Test { ...

when we call getAnnotationsByType(RepeatableAnn.class) on this class, we
get nothing because RepeatableAnn is contained by ContainerAnn which is
also contained by ContainerContainerAnn. In other words, RepeatableAnn is
in depth 2 and get*AnnotationsByType APIs only searches depth 1. The test
results are:

/home/deven/jdk8_20/jdk1.8.0_20/bin$ java repeated.Test
CLASS = repeated.Test
getDeclaredAnnotationsByType(RepeatableAnn.class)
getDeclaredAnnotationsByType(ContainerAnn.class)

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)])

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])
getDeclaredAnnotationsByType(ContainerContainerAnn.class)

@repeated.ContainerContainerAnn(value=[@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)]),
@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])])
getAnnotationsByType(RepeatableAnn.class)
getAnnotationsByType(ContainerAnn.class)

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)])

@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])
getAnnotationsByType(ContainerContainerAnn.class)

@repeated.ContainerContainerAnn(value=[@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-3)]),
@repeated.ContainerAnn(value=[@repeated.RepeatableAnn(value=Parent-4)])])




RFR: 8048021: Remove @version tag in jaxp repo

2014-06-24 Thread Henry Jen

Hi,

As a follow up task, please review the trivial webrev to remove @version 
tags used with SCM, which is likely to be meaningless to javadoc readers.


In this webrev, we only remove @version with $Id or $Revision for .jave 
or package.html cause appearance to javadoc.


We keep those in .properties files also those @version tags with 
specific version information.


Cheers,
Henry


On 06/20/2014 05:24 PM, Henry Jen wrote:

Looks like it's pretty easy to do a find and replace in files in jaxp
folder. If this is desired, we can do it in a separate webrev to be
clear on changeset history?

Cheers,
Henry

On 06/20/2014 03:36 PM, huizhe wang wrote:

Hi Henry,

Is it possible to add to the @since tool/script to remove repository
revision notes?  They appear in the API document as if they reflect
meaningful version. An example in
http://cr.openjdk.java.net/~henryjen/jdk9/8047723/0/webrev/src/javax/xml/parsers/DocumentBuilderFactory.java.udiff.html



   * @version $Revision: 1.9 $, $Date: 2010/05/25 16:19:44 $



Re: RFR: 8048021: Remove @version tag in jaxp repo

2014-06-24 Thread huizhe wang

Looks good. Thanks for the extra work!

I think your webrev is here: 
http://cr.openjdk.java.net/~henryjen/jdk9/8048021/0/webrev/


Cheers,
Joe

On 6/24/2014 1:00 PM, Henry Jen wrote:

Hi,

As a follow up task, please review the trivial webrev to remove 
@version tags used with SCM, which is likely to be meaningless to 
javadoc readers.


In this webrev, we only remove @version with $Id or $Revision for 
.jave or package.html cause appearance to javadoc.


We keep those in .properties files also those @version tags with 
specific version information.


Cheers,
Henry


On 06/20/2014 05:24 PM, Henry Jen wrote:

Looks like it's pretty easy to do a find and replace in files in jaxp
folder. If this is desired, we can do it in a separate webrev to be
clear on changeset history?

Cheers,
Henry

On 06/20/2014 03:36 PM, huizhe wang wrote:

Hi Henry,

Is it possible to add to the @since tool/script to remove repository
revision notes?  They appear in the API document as if they reflect
meaningful version. An example in
http://cr.openjdk.java.net/~henryjen/jdk9/8047723/0/webrev/src/javax/xml/parsers/DocumentBuilderFactory.java.udiff.html 





   * @version $Revision: 1.9 $, $Date: 2010/05/25 16:19:44 $