Theoretical data race on java.util.logging.Handler.sealed

2013-12-03 Thread Peter Levart

Hi,

While browsing the code of java.util.logging.Handler, I noticed a 
theoretical possibility that a security check in a j.u.l.StreamHandler 
be circumvented using a data race.


There is a plain boolean instance field 'sealed' in j.u.l.Handler that 
is pre-initialized to 'true' in field initializer. StreamHandler 
sublcass' constructors overwrite this value with 'false' at the 
beginning, then issue some operations which circumvent security checks, 
and finally they reset the 'sealed' value back to 'true' at the end.


If a reference to an instance of StreamHandler or subclass is passed to 
some thread without synchronization via data-race, this thread can see 
'true' or 'false' as the possible values of 'sealed' variable, thus it 
is possible to circumvent security checks.


One possibility to fix this is moving the field to StreamHandler and 
making it final:


http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.01/

Just making the field volatile might not work. There is an ongoing 
debate on concurrency-interest which suggests that volatile fields are 
not exceptional in constructors like final fields are...



Regards, Peter



Re: Theoretical data race on java.util.logging.Handler.sealed

2013-12-03 Thread Peter Levart

On 12/03/2013 09:51 AM, Peter Levart wrote:

Hi,

While browsing the code of java.util.logging.Handler, I noticed a 
theoretical possibility that a security check in a j.u.l.StreamHandler 
be circumvented using a data race.


There is a plain boolean instance field 'sealed' in j.u.l.Handler that 
is pre-initialized to 'true' in field initializer. StreamHandler 
sublcass' constructors overwrite this value with 'false' at the 
beginning, then issue some operations which circumvent security 
checks, and finally they reset the 'sealed' value back to 'true' at 
the end.


If a reference to an instance of StreamHandler or subclass is passed 
to some thread without synchronization via data-race, this thread can 
see 'true' or 'false' as the possible values of 'sealed' variable, 
thus it is possible to circumvent security checks.


One possibility to fix this is moving the field to StreamHandler and 
making it final:


http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.01/

Just making the field volatile might not work. There is an ongoing 
debate on concurrency-interest which suggests that volatile fields are 
not exceptional in constructors like final fields are...



Regards, Peter



The proposed patch is not complete. There are several subclasses of 
StreamHandler in the java.util.logging package that also need a way to 
bypass security checks for some operations in their constructors. So 
here's the updated webrev which updates them with the same code as 
StreamHandler. This means that there are two copies of 'sealed' flag in 
object of type ConsoleHandler, for example, but only the one declared in 
ConsoleHandler is relevant for governing access checks:


http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.02/

Before filing the bug, I'm asking the list whether this can be 
considered a bug...


Regards, Peter



Re: RFR [6968459] JNDI timeout fails before timeout is reached

2013-12-03 Thread Peter Levart

On 11/29/2013 09:06 PM, Ivan Gerasimov wrote:

Thank you Alan for the reply!

On 29.11.2013 21:03, Alan Bateman wrote:

On 19/11/2013 17:58, Ivan Gerasimov wrote:

Hello all!

Would you please help review a fix for the bug?
https://bugs.openjdk.java.net/browse/JDK-6968459

It was reported that creating new InitialLdapContext() can fail with 
javax.naming.NamingException: LDAP response read timed out, timeout 
used:3ms, even though the specified timeout hadn't been elapsed.


The fix was provided by the filer of the bug some time ago.
Here's the webrev with this fix:
http://cr.openjdk.java.net/~igerasim/6968459/0/webrev/


I haven't seen any replies to this but I've cc'ed Vinnie and Xuelei 
as they are more familiar with this area.


If I understand correctly then the issue is that the timeout handling 
doesn't take account of wakeups when aren't any BerDecoders to 
dequeue. The changes mean it will retry the wait with a decreasing 
timeout until a reply is received or the timeout elapses. That seems 
reasonable, assuming the time doesn't change :-)   You might find the 
code is a bit clearer if you have a remaining time as that would 
allow you get rid of timedOut, timeOut and endTime.



I modified the patch in the way you suggest.
http://cr.openjdk.java.net/~igerasim/6968459/1/webrev/

The timeOut variable now holds the remaining time.
If the system time had changed back, we start counting from the 
beginning.
If it had changed forward, we have no way to catch it and the timeout 
gets elapsed earlier.


I see the patch doesn't come with a test. Is there any test 
infrastructure for testing LDAP without require a complete server?



I didn't find anything like that, that's why I set 'noreg-hard' label.

Sincerely yours,
Ivan



-Alan.






Hi Ivan,

From quick look I notice a danger that Connection.readReply() pauses 
(for the timeOut time) in spite of the fact that a reply is ready and 
enqueued before waiting.


Imagine a situation where the 1st try of obtaining result returns null:

 450 // Get the reply if it is already available.
 451 BerDecoder rber = ldr.getReplyBer();


...after that, but before reaching the following lines:

 471 synchronized (ldr) {
 472 ldr.wait(timeOut);
 473 }
 

The other thread enqueues a reply (LdapRequest.addReplyBer()) because 
the code in readReply() is executing without holding a lock on ldr. 
When this thread (executing readReply()) finally enters synchronized 
block and waits, it will wait until wait() times out or another reply is 
enqueued which will issue another notify(). This can lead to unnecessary 
pauses. Old code does not exhibit this problem, because it re-checks the 
readiness of a reply immediately after entering the synchronized block.



Regards, Peter



Re: Review Request for 8029216: (jdeps) Provide a specific option to report JDK internal APIs

2013-12-03 Thread Alan Bateman

On 02/12/2013 21:49, Mandy Chung wrote:

On 11/27/13 4:04 AM, Alan Bateman wrote:

On 26/11/2013 23:59, Mandy Chung wrote:
This is a simple patch that adds a new jdeps -jdkinternals option to 
make it easier for developers to find dependencies on the JDK 
internal APIs:

http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8029216/webrev.00/
This looks good (and I can't think of a better name for the option). 
Do you think this needs a test as otherwise this option will not be 
exercised?


Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8029216/webrev.01/

I added the new test cases.  This also updates -v to include the from 
class in the o and the verbose mode prints more information.

Thanks for adding the test. It all looks good now.

-Alan.


Re: RFR [6968459] JNDI timeout fails before timeout is reached

2013-12-03 Thread Ivan Gerasimov

Hi Peter!

Thank you for your review!

You are right, the patch changed the behavior of the code.
I've reverted back all the unnecessary changes. This should minimize the 
risk.


I've also made another correction: After decrementing the remaining 
timeOut, the startTime should be set to currTime.


Would you please help review the updated webrev:
http://cr.openjdk.java.net/~igerasim/6968459/2/webrev/

Sincerely yours,
Ivan



From quick look I notice a danger that Connection.readReply() pauses 
(for the timeOut time) in spite of the fact that a reply is ready and 
enqueued before waiting.


Imagine a situation where the 1st try of obtaining result returns null:

 450 // Get the reply if it is already available.
 451 BerDecoder rber = ldr.getReplyBer();


...after that, but before reaching the following lines:

 471 synchronized (ldr) {
 472 ldr.wait(timeOut);
 473 }


The other thread enqueues a reply (LdapRequest.addReplyBer()) 
because the code in readReply() is executing without holding a lock on 
ldr. When this thread (executing readReply()) finally enters 
synchronized block and waits, it will wait until wait() times out or 
another reply is enqueued which will issue another notify(). This can 
lead to unnecessary pauses. Old code does not exhibit this problem, 
because it re-checks the readiness of a reply immediately after 
entering the synchronized block.



Regards, Peter







Re: RFR [6968459] JNDI timeout fails before timeout is reached

2013-12-03 Thread Vincent Ryan
Hello Ivan,

Thanks for the updated patch. I would like to see a testcase along with this 
fix, since it is
modifying a critical component of the LDAP client code. An LDAP server may not 
even be
required in order to exercise the timeouts.

Thanks.


On 3 Dec 2013, at 14:35, Ivan Gerasimov ivan.gerasi...@oracle.com wrote:

 Hi Peter!
 
 Thank you for your review!
 
 You are right, the patch changed the behavior of the code.
 I've reverted back all the unnecessary changes. This should minimize the risk.
 
 I've also made another correction: After decrementing the remaining timeOut, 
 the startTime should be set to currTime.
 
 Would you please help review the updated webrev:
 http://cr.openjdk.java.net/~igerasim/6968459/2/webrev/
 
 Sincerely yours,
 Ivan
 
 
 From quick look I notice a danger that Connection.readReply() pauses (for 
 the timeOut time) in spite of the fact that a reply is ready and enqueued 
 before waiting.
 
 Imagine a situation where the 1st try of obtaining result returns null:
 
 450 // Get the reply if it is already available.
 451 BerDecoder rber = ldr.getReplyBer();
 
 
 ...after that, but before reaching the following lines:
 
 471 synchronized (ldr) {
 472 ldr.wait(timeOut);
 473 }
 
 
 The other thread enqueues a reply (LdapRequest.addReplyBer()) because the 
 code in readReply() is executing without holding a lock on ldr. When 
 this thread (executing readReply()) finally enters synchronized block and 
 waits, it will wait until wait() times out or another reply is enqueued 
 which will issue another notify(). This can lead to unnecessary pauses. Old 
 code does not exhibit this problem, because it re-checks the readiness of a 
 reply immediately after entering the synchronized block.
 
 
 Regards, Peter
 
 
 
 



Re: RFR [6968459] JNDI timeout fails before timeout is reached

2013-12-03 Thread Peter Levart

On 12/03/2013 03:35 PM, Ivan Gerasimov wrote:

Hi Peter!

Thank you for your review!

You are right, the patch changed the behavior of the code.
I've reverted back all the unnecessary changes. This should minimize 
the risk.


I've also made another correction: After decrementing the remaining 
timeOut, the startTime should be set to currTime.


Would you please help review the updated webrev:
http://cr.openjdk.java.net/~igerasim/6968459/2/webrev/

Sincerely yours,
Ivan


Hi Ivan,

That's better. You could move the initial request for ldr.getReplyBer() 
(line 447) out of while loop, since it is not needed in the loop (all 
further ldr.getReplyBer() calls in loop are performed in synchronized 
block already - line 465). else { break; } (line 469) is not needed in 
that case. I would also make timeOut variable long to avoid overflows. 
Simplified logic could look like this:



BerDecoder readReply(LdapRequest ldr)
throws IOException, NamingException {
long timeOut = (readTimeout  0) ?
readTimeout : 15 * 1000; // Default read timeout of 15 sec

long currTime = System.currentTimeMillis();

BerDecoder rber = ldr.getReplyBer();

while (rber == null  timeOut  0L) {
long startTime = currTime;
// If socket closed, don't even try
synchronized (this) {
if (sock == null) {
throw new ServiceUnavailableException(host + : + 
port +

; socket closed);
}
}
synchronized (ldr) {
// check if condition has changed since our last check
rber = ldr.getReplyBer();
if (rber == null) {
try {
ldr.wait(timeOut);
} catch (InterruptedException ex) {
throw new InterruptedNamingException(
Interrupted during LDAP operation);
}
}
}
currTime = System.currentTimeMillis();
if (startTime  currTime) {
timeOut -= (currTime - startTime);
}
// else system time must have changed backwards (or not at all)
}

if (rber == null  readTimeout  0) {
removeRequest(ldr);
throw new NamingException(LDAP response read timed out, 
timeout used:

+ readTimeout + ms. );
}

return rber;
}


What do you think?

Regards, Peter





From quick look I notice a danger that Connection.readReply() pauses 
(for the timeOut time) in spite of the fact that a reply is ready and 
enqueued before waiting.


Imagine a situation where the 1st try of obtaining result returns null:

 450 // Get the reply if it is already available.
 451 BerDecoder rber = ldr.getReplyBer();


...after that, but before reaching the following lines:

 471 synchronized (ldr) {
 472 ldr.wait(timeOut);
 473 }


The other thread enqueues a reply (LdapRequest.addReplyBer()) 
because the code in readReply() is executing without holding a lock 
on ldr. When this thread (executing readReply()) finally enters 
synchronized block and waits, it will wait until wait() times out or 
another reply is enqueued which will issue another notify(). This can 
lead to unnecessary pauses. Old code does not exhibit this problem, 
because it re-checks the readiness of a reply immediately after 
entering the synchronized block.



Regards, Peter









hg: jdk8/tl/jdk: 8029127: Redirected POST request throws IllegalStateException on HttpURLConnection.getInputStream

2013-12-03 Thread michael . x . mcmahon
Changeset: e01c6e0bf8ae
Author:michaelm
Date:  2013-12-03 17:29 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e01c6e0bf8ae

8029127: Redirected POST request throws IllegalStateException on 
HttpURLConnection.getInputStream
Reviewed-by: alanb, chegar

! src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java
+ test/sun/net/www/protocol/http/RedirectOnPost.java



JDK 8 RFR of javax.script doclint fixes

2013-12-03 Thread Joe Darcy

Hello,

Please review the patch before which addresses a handful of doclint 
issues in javax.script.


Thanks,

-Joe

diff -r c11553506228 src/share/classes/javax/script/ScriptEngineFactory.java
--- a/src/share/classes/javax/script/ScriptEngineFactory.javaTue Dec 
03 08:53:23 2013 +0100
+++ b/src/share/classes/javax/script/ScriptEngineFactory.javaTue Dec 
03 09:48:11 2013 -0800

@@ -144,7 +144,7 @@
  * Returns a String which can be used to invoke a method of a Java 
object using the syntax
  * of the supported scripting language.  For instance, an 
implementation for a Javascript

  * engine might be;
- * p
+ *
  * pre{@code
  * public String getMethodCallSyntax(String obj,
  *   String m, String... args) {
@@ -180,7 +180,7 @@
  * Returns a String that can be used as a statement to display the 
specified String  using
  * the syntax of the supported scripting language.  For instance, 
the implementation for a Perl

  * engine might be;
- * p
+ *
  * precode
  * public String getOutputStatement(String toDisplay) {
  *  return print( + toDisplay + );
@@ -198,7 +198,7 @@
 /**
  * Returns a valid scripting language executable program with 
given statements.

  * For instance an implementation for a PHP engine might be:
- * p
+ *
  * pre{@code
  * public String getProgram(String... statements) {
  *  String retval = ?\n;



Re: JDK 8 RFR of javax.script doclint fixes

2013-12-03 Thread Mike Duigou
Approved.

On Dec 3 2013, at 09:49 , Joe Darcy joe.da...@oracle.com wrote:

 Hello,
 
 Please review the patch before which addresses a handful of doclint issues in 
 javax.script.
 
 Thanks,
 
 -Joe
 
 diff -r c11553506228 src/share/classes/javax/script/ScriptEngineFactory.java
 --- a/src/share/classes/javax/script/ScriptEngineFactory.javaTue Dec 03 
 08:53:23 2013 +0100
 +++ b/src/share/classes/javax/script/ScriptEngineFactory.javaTue Dec 03 
 09:48:11 2013 -0800
 @@ -144,7 +144,7 @@
  * Returns a String which can be used to invoke a method of a Java object 
 using the syntax
  * of the supported scripting language.  For instance, an implementation 
 for a Javascript
  * engine might be;
 - * p
 + *
  * pre{@code
  * public String getMethodCallSyntax(String obj,
  *   String m, String... args) {
 @@ -180,7 +180,7 @@
  * Returns a String that can be used as a statement to display the 
 specified String  using
  * the syntax of the supported scripting language.  For instance, the 
 implementation for a Perl
  * engine might be;
 - * p
 + *
  * precode
  * public String getOutputStatement(String toDisplay) {
  *  return print( + toDisplay + );
 @@ -198,7 +198,7 @@
 /**
  * Returns a valid scripting language executable program with given 
 statements.
  * For instance an implementation for a PHP engine might be:
 - * p
 + *
  * pre{@code
  * public String getProgram(String... statements) {
  *  String retval = ?\n;
 



Re: JDK 8 RFR of javax.script doclint fixes

2013-12-03 Thread Chris Hegarty
Looks fine to me Joe.

-Chris.

On 3 Dec 2013, at 17:49, Joe Darcy wrote:

 Hello,
 
 Please review the patch before which addresses a handful of doclint issues in 
 javax.script.
 
 Thanks,
 
 -Joe
 
 diff -r c11553506228 src/share/classes/javax/script/ScriptEngineFactory.java
 --- a/src/share/classes/javax/script/ScriptEngineFactory.javaTue Dec 03 
 08:53:23 2013 +0100
 +++ b/src/share/classes/javax/script/ScriptEngineFactory.javaTue Dec 03 
 09:48:11 2013 -0800
 @@ -144,7 +144,7 @@
  * Returns a String which can be used to invoke a method of a Java object 
 using the syntax
  * of the supported scripting language.  For instance, an implementation 
 for a Javascript
  * engine might be;
 - * p
 + *
  * pre{@code
  * public String getMethodCallSyntax(String obj,
  *   String m, String... args) {
 @@ -180,7 +180,7 @@
  * Returns a String that can be used as a statement to display the 
 specified String  using
  * the syntax of the supported scripting language.  For instance, the 
 implementation for a Perl
  * engine might be;
 - * p
 + *
  * precode
  * public String getOutputStatement(String toDisplay) {
  *  return print( + toDisplay + );
 @@ -198,7 +198,7 @@
 /**
  * Returns a valid scripting language executable program with given 
 statements.
  * For instance an implementation for a PHP engine might be:
 - * p
 + *
  * pre{@code
  * public String getProgram(String... statements) {
  *  String retval = ?\n;
 



Re: JDK 8 RFR of javax.script doclint fixes

2013-12-03 Thread roger riggs

Hi Joe,

looks fine,

Not a reviewer, Roger

On 12/3/2013 12:49 PM, Joe Darcy wrote:

Hello,

Please review the patch before which addresses a handful of doclint 
issues in javax.script.


Thanks,

-Joe

diff -r c11553506228 
src/share/classes/javax/script/ScriptEngineFactory.java
--- a/src/share/classes/javax/script/ScriptEngineFactory.java Tue Dec 
03 08:53:23 2013 +0100
+++ b/src/share/classes/javax/script/ScriptEngineFactory.java Tue Dec 
03 09:48:11 2013 -0800

@@ -144,7 +144,7 @@
  * Returns a String which can be used to invoke a method of a 
Java object using the syntax
  * of the supported scripting language.  For instance, an 
implementation for a Javascript

  * engine might be;
- * p
+ *
  * pre{@code
  * public String getMethodCallSyntax(String obj,
  *   String m, String... args) {
@@ -180,7 +180,7 @@
  * Returns a String that can be used as a statement to display 
the specified String  using
  * the syntax of the supported scripting language.  For instance, 
the implementation for a Perl

  * engine might be;
- * p
+ *
  * precode
  * public String getOutputStatement(String toDisplay) {
  *  return print( + toDisplay + );
@@ -198,7 +198,7 @@
 /**
  * Returns a valid scripting language executable program with 
given statements.

  * For instance an implementation for a PHP engine might be:
- * p
+ *
  * pre{@code
  * public String getProgram(String... statements) {
  *  String retval = ?\n;





Re: JDK 8 RFR of javax.script doclint fixes

2013-12-03 Thread roger riggs

Hi,

Not specific to your change, but the previous sentences end in ; in 
some cases and : in others.

I thing : colon is more natural.

Roger

On 12/3/2013 12:55 PM, roger riggs wrote:

Hi Joe,

looks fine,

Not a reviewer, Roger

On 12/3/2013 12:49 PM, Joe Darcy wrote:

Hello,

Please review the patch before which addresses a handful of doclint 
issues in javax.script.


Thanks,

-Joe

diff -r c11553506228 
src/share/classes/javax/script/ScriptEngineFactory.java
--- a/src/share/classes/javax/script/ScriptEngineFactory.java Tue Dec 
03 08:53:23 2013 +0100
+++ b/src/share/classes/javax/script/ScriptEngineFactory.java Tue Dec 
03 09:48:11 2013 -0800

@@ -144,7 +144,7 @@
  * Returns a String which can be used to invoke a method of a 
Java object using the syntax
  * of the supported scripting language.  For instance, an 
implementation for a Javascript

  * engine might be;
- * p
+ *
  * pre{@code
  * public String getMethodCallSyntax(String obj,
  *   String m, String... args) {
@@ -180,7 +180,7 @@
  * Returns a String that can be used as a statement to display 
the specified String  using
  * the syntax of the supported scripting language.  For 
instance, the implementation for a Perl

  * engine might be;
- * p
+ *
  * precode
  * public String getOutputStatement(String toDisplay) {
  *  return print( + toDisplay + );
@@ -198,7 +198,7 @@
 /**
  * Returns a valid scripting language executable program with 
given statements.

  * For instance an implementation for a PHP engine might be:
- * p
+ *
  * pre{@code
  * public String getProgram(String... statements) {
  *  String retval = ?\n;







hg: jdk8/tl/langtools: 8028699: Compiler crash during speculative attribution of annotated type

2013-12-03 Thread jan . lahoda
Changeset: a746587a1ff1
Author:jlahoda
Date:  2013-12-03 18:50 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a746587a1ff1

8028699: Compiler crash during speculative attribution of annotated type
Summary: Moving the checkForDeclarationAnnotations check into 
Attr.TypeAnnotationsValidator
Reviewed-by: jjg
Contributed-by: wdi...@gmail.com

! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/comp/Check.java
+ test/tools/javac/annotations/typeAnnotations/failures/CheckForDeclAnnoNPE.java
! 
test/tools/javac/annotations/typeAnnotations/failures/common/arrays/DeclarationAnnotation.java
! 
test/tools/javac/annotations/typeAnnotations/failures/common/arrays/DeclarationAnnotation.out



hg: jdk8/tl/jdk: 8029478: Fix more doclint issues in javax.script

2013-12-03 Thread joe . darcy
Changeset: 1c3d58caa7da
Author:darcy
Date:  2013-12-03 10:07 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1c3d58caa7da

8029478: Fix more doclint issues in javax.script
Reviewed-by: chegar, mduigou

! src/share/classes/javax/script/ScriptEngineFactory.java



RFR: 8028757: CharSequence.subSequence improperly requires a new CharSequence be returned

2013-12-03 Thread Stuart Marks

Hi all,

Please review this small change to the subSequence() method specs of 
CharSequence and String. Essentially this removes the requirement of returning a 
new character sequence at each call. This brings the spec in line with 
String's implementation, which will return 'this' in appropriate circumstances.


No change is necessary for the other CharSequence implementations. 
StringBuilder/Buffer still say new and in fact they always return new String 
objects. CharBuffer defines its exact sharing behavior. Segment returns a new 
Segment with a shared character array, which is (supposedly) immutable.


Diffs appended below.

Thanks,

s'marks


# HG changeset patch
# User smarks
# Date 1386094056 28800
# Node ID c15e257074e48a1927755ff48393baa8f3f3ab0e
# Parent  4d9078b1f25b72071d91acc7e1ce5bb7e91748fb
8028757: CharSequence.subSequence improperly requires a new CharSequence be 
returned

Reviewed-by: XXX

diff -r 4d9078b1f25b -r c15e257074e4 
src/share/classes/java/lang/CharSequence.java
--- a/src/share/classes/java/lang/CharSequence.java Tue Nov 26 14:49:55 
2013 +0900
+++ b/src/share/classes/java/lang/CharSequence.java Tue Dec 03 10:07:36 
2013 -0800
@@ -87,7 +87,7 @@
 char charAt(int index);

 /**
- * Returns a new codeCharSequence/code that is a subsequence of this 
sequence.

+ * Returns a codeCharSequence/code that is a subsequence of this 
sequence.
  * The subsequence starts with the codechar/code value at the 
specified index and
  * ends with the codechar/code value at index ttend - 1/tt.  The 
length

  * (in codechar/codes) of the
diff -r 4d9078b1f25b -r c15e257074e4 src/share/classes/java/lang/String.java
--- a/src/share/classes/java/lang/String.java   Tue Nov 26 14:49:55 2013 +0900
+++ b/src/share/classes/java/lang/String.java   Tue Dec 03 10:07:36 2013 -0800
@@ -1958,7 +1958,7 @@
 }

 /**
- * Returns a new character sequence that is a subsequence of this sequence.
+ * Returns a character sequence that is a subsequence of this sequence.
  *
  * p An invocation of this method of the form
  *


hg: jdk8/tl/langtools: 8029179: javac produces a compile error for valid boolean expressions

2013-12-03 Thread vicente . romero
Changeset: fb8c59cf26c8
Author:vromero
Date:  2013-12-03 18:13 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/fb8c59cf26c8

8029179: javac produces a compile error for valid boolean expressions
Reviewed-by: jjg, jlahoda

! src/share/classes/com/sun/tools/javac/parser/JavacParser.java
+ test/tools/javac/T8029179/CompileErrorForValidBooleanExpTest.java



RFR: 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic

2013-12-03 Thread Henry Jen

Hi,

Please review a small fix that add missing NONNULL characteristic and 
cleanup in javadoc.


Thanks Anthony Vanelverdinghe for reporting of this bug.

http://cr.openjdk.java.net/~henryjen/tl/8029434/0/webrev/

Cheers,
Henry


Re: 8029281: Synchronization issues in Logger and LogManager

2013-12-03 Thread Daniel Fuchs

On 12/3/13 8:36 AM, Peter Levart wrote:

Hi Daniel,

1st sorry for the delay. I promised looking at the patch, but was then
distracted by other things. I think that synchronization in LogManager
is correct now. The fact that Mandy thinks so is also reassuring.


Yes - that's usually a good sign :-)


I wonder if calling LoggerWeakRef.dispose() from
LoggerContext.addLocalLoger() is needed now that you check the
LogNode.loggerRef before clearing in LoggerWeakRef.dispose():


Yes - we could probably call dispose() only when draining the
queue - but I still have the feeling that calling it early
is better. It makes sure that everything is properly cleaned-up
before we modify it.

I also like that we now have a single entry point - 'dispose()' - to
clean up after a gc'ed logger rather than doing half of the job now
and leaving the other half for later (and trusting that the later
code will not do anything stupid).

Thank you again for your reviews and feedback - they are very
helpful!

-- daniel



 if (n.loggerRef == this) {
 n.loggerRef = null;  // clear LogNode's weak
ref to us
 }


The LoggerContext.addLocalLoger() will already overwrite the
namedLoggers Hashtable entry for the logger's name with new LoggerWeakRef:

 namedLoggers.put(name, ref);

...and it will already overwrite the LogNode.loggerRef with new
LoggerWeakRef:

 // Find the new node and its parent.
 LogNode node = getNode(name);
 node.loggerRef = ref;

So the only effect of calling LoggerWeakRef.dispose() from
LoggerContext.addLocalLoger() is a more prompt unlinking of
LoggerWeakRef child, which has already been cleared but not yet
disposed(), from parent logger. This will eventually happen in a call to
LogManager.drainLoggerRefQueueBounded(), called from public
LogManager.addLogger(), which is the only entry point to
LoggerContext.addLocalLoger(). But it's not wrong as it is now when you
also ensure more prompt unlinking of cleared LoggerWeakRef and code is
more uniform and less fragile then it would be if you created and called
a hypothetical disposeFromParent() which only took care of unlinking
from parent logger...

Regards, Peter


On 11/29/2013 12:41 PM, Daniel Fuchs wrote:

Hi,

Here is a new revision that includes Peter's findings.
Thanks again for that Peter!

http://cr.openjdk.java.net/~dfuchs/webrev_8029281/webrev.01/

It has passed all jdk_core tests and running the new tests
in a loop hasn't triggered any of the issues that appeared
before the fix.

-- daniel

On 11/28/13 3:26 PM, Daniel Fuchs wrote:

On 11/27/13 10:34 PM, Peter Levart wrote:

Hi Daniel,

I have started looking at the LogManager change first, and here's the
1st race I found...


Hi Peter,

Thanks a lot for your feedback! see below...


  The new method LoggerWeakRef.dispose() can be called from 3 places:

- LoggerContext.addLocalLogger
- LoggerContext.findLogger
- LogManager.drainLoggerRefQueueBounded which is called from public
method LogManager.addLogger

The 1st and 2nd are guarded by LoggerContext.this lock, but the 3rd is
unguarded.

What can happen is the following:

Thread1:
 LogManager.addLogger(someLogger)
 LogManager.drainLoggerRefQueueBounded() // finds some enqueued
LoggerWeakRef for name == 'X.Y' and...
 LoggerWeakRef.dispose() // this is 1st call to dispose,
so...
 synchronized (LoggerWeakRef.this) {
disposed = true;
 }

---context switch---

Thread2:
 LogManager.addLogger(logger{name=='X.Y'})
 LogManager.drainLoggerRefQueueBounded()  // no enqueued
refs...
 LoggerContext.addLocalLogger(logger{name=='X.Y'}, true)
 LoggerWeakRef ref = namedLoggers.get(name); // returns
a non-null ref ('X.Y' still points to a cleared weak ref)
 if (ref.get() == null) {
 ref.dispose(); // this is a 2nd call to this ref,
so it returns quickly
 }
 // ... namedLoggers gets new entry, etc...
 LogNode node = getNode(name); // get node for 'X.Y'
*node.loggerRef = ref;*

---context switch---

Thread1: (still in LoggerWeakRef.dispose())
 if (node != null) {
*node.loggerRef = null;*  // clear LogNode's weak
ref to
us *!!! NOT QUITE !!!*


... so Thread1 erroneously clears the reference from Node to new
LoggerWeakRef that was just added by Thread2 ...


Damn. I believe that you're right! I overlooked the fact that LogNode is
reused.

Locking on LogNode.context is indeed a good way of solving the issue.
The only method that we will call from within the lock is
LogNode.context.rmoveLoggerRef(name this) - and that already requires
a lock on LogNode.context so it would be safe.

I have also double checked the places where LogNode.loggerRef is set,
and all those places (except in dispose() so far) are already from
within a 

Re: 8029281: Synchronization issues in Logger and LogManager

2013-12-03 Thread Daniel Fuchs

Hi Mandy,

I have updated the webrev taking your comments into account.

http://cr.openjdk.java.net/~dfuchs/webrev_8029281/webrev.02/

Changes are small and outlined below:

On 12/3/13 1:49 AM, Mandy Chung wrote:

On 11/29/2013 3:41 AM, Daniel Fuchs wrote:

Hi,

Here is a new revision that includes Peter's findings.
Thanks again for that Peter!

http://cr.openjdk.java.net/~dfuchs/webrev_8029281/webrev.01/



LogManager.java
L156: make props volatile is good.

I think the reason why LoggerWeakRef.node and parentRef don't need
to be volatile because of the synchronized block to check and set
disposed flag that ensures that only one thread will be doing the node
and parentRef cleanup.  I agree that the two lines should be done with
synchronized on the LoggerContext.

  node.context.removeLogger(name);
  node.loggerRef = null;  // clear LogNode's weak ref to us

I don't see anything obvious wrong with this patch.   Have you
considered having the dispose() method simply synchronizes on
node.context (probably need to get the LoggerContext when instantiating
LoggerWeakRef and break dispose method into two - one for the node and
the other for the parentRef)?I'm not proposing to do this in JDK 8
since we are ramping down and get a low risk fix in.   It may be
something to revisit for jdk 9 to a cleaner version.


I believe that nulling the instance variables in LoggerWeakRef
is probably no longer needed - since we now should be calling
dispose only once - but I didn't want to change the code too
much. I agree it would be better to revisit that in 9.



Logger.java
The assert this.loggerBundle == NO_RESOURCE_BUNDLE; in the Logger
constructors doesn't seem to be needed.


OK - removed.


Having an immutable LoggerBundle class is good to ensure the
resourceBundleName and resourceBundle are consistent.

L1930: is lb.userBundle and lb.resourceBundleName null when getting
to this line?  Might be better to rename setupResourceInfo to
setResourceBundleName (that creates a LoggerBundle with a null RB). On
the other hand,  setResourceBundle will instantiate a LoggerBundle with
non-null rbname and RB.  It wasn't obvious to me what does what.


Right. Let's keep the old name for now. We can revisit the naming in 9.
You right that when we reach the last line lb.userBundle must be null,
otherwise we would have either returned or thrown an exception, since
lb.userBundle != null = lb.resourceBundleName != null.

Therefore I added an assert lb.userBundle == null and
replaced the call to
  LoggerBundle.get(name, lb.userBundle)
by
  LoggerBundle.get(name, null)

Hopefully it will make what happens more obvious.


2137 final String rbName = getResourceBundleName();
2138 if (!SYSTEM_LOGGER_RB_NAME.equals(rbName)
2139 
!SYSTEM_LOGGER_RB_NAME.equals(b.getBaseBundleName())) {
2140 return LoggerBundle.get(rbName, b);
2141 } else {
2142 return SYSTEM_BUNDLE;
2143 }

To get to the above line, either lb.userBundle is null or
getResourceBundle() isoverridden.  L2126 already checks that this logger
does not associate with the system rbname.  It seems to me that the
special case here is when the Logger subclass overrides
getResourceBundleName() to returnSYSTEM_LOGGER_RB_NAME or override
getResourceBundle to return the sun.util.logging.resources.logging RB.
I tink that we might just simplify the above and just return
LoggerBundle.get(rbName, b) and no need to worry about those overridden
case which does no use to such a Logger subclass with unmatched rbname
and RB.  Same comment applied to L2157-2165.


Ah. I'm glad to hear that. This is a great simplification.
done.

-- daniel




thanks
Mandy




Re: RFR: 8028757: CharSequence.subSequence improperly requires a new CharSequence be returned

2013-12-03 Thread Mike Duigou
The changes look OK.

The needs of mutable CharSequences to isolate the subsequence changes seems 
adequately handled in the mutable classes.

Mike

On Dec 3 2013, at 10:15 , Stuart Marks stuart.ma...@oracle.com wrote:

 Hi all,
 
 Please review this small change to the subSequence() method specs of 
 CharSequence and String. Essentially this removes the requirement of 
 returning a new character sequence at each call. This brings the spec in 
 line with String's implementation, which will return 'this' in appropriate 
 circumstances.
 
 No change is necessary for the other CharSequence implementations. 
 StringBuilder/Buffer still say new and in fact they always return new 
 String objects. CharBuffer defines its exact sharing behavior. Segment 
 returns a new Segment with a shared character array, which is (supposedly) 
 immutable.
 
 Diffs appended below.
 
 Thanks,
 
 s'marks
 
 
 # HG changeset patch
 # User smarks
 # Date 1386094056 28800
 # Node ID c15e257074e48a1927755ff48393baa8f3f3ab0e
 # Parent  4d9078b1f25b72071d91acc7e1ce5bb7e91748fb
 8028757: CharSequence.subSequence improperly requires a new CharSequence be 
 returned
 Reviewed-by: XXX
 
 diff -r 4d9078b1f25b -r c15e257074e4 
 src/share/classes/java/lang/CharSequence.java
 --- a/src/share/classes/java/lang/CharSequence.java   Tue Nov 26 14:49:55 
 2013 +0900
 +++ b/src/share/classes/java/lang/CharSequence.java   Tue Dec 03 10:07:36 
 2013 -0800
 @@ -87,7 +87,7 @@
 char charAt(int index);
 
 /**
 - * Returns a new codeCharSequence/code that is a subsequence of this 
 sequence.
 + * Returns a codeCharSequence/code that is a subsequence of this 
 sequence.
  * The subsequence starts with the codechar/code value at the 
 specified index and
  * ends with the codechar/code value at index ttend - 1/tt.  The 
 length
  * (in codechar/codes) of the
 diff -r 4d9078b1f25b -r c15e257074e4 src/share/classes/java/lang/String.java
 --- a/src/share/classes/java/lang/String.java Tue Nov 26 14:49:55 2013 +0900
 +++ b/src/share/classes/java/lang/String.java Tue Dec 03 10:07:36 2013 -0800
 @@ -1958,7 +1958,7 @@
 }
 
 /**
 - * Returns a new character sequence that is a subsequence of this 
 sequence.
 + * Returns a character sequence that is a subsequence of this sequence.
  *
  * p An invocation of this method of the form
  *



hg: jdk8/tl/jdk: 8029483: BufferedReader.lines() javadoc typo should be fixed

2013-12-03 Thread henry . jen
Changeset: 1061f4d085b5
Author:henryjen
Date:  2013-12-03 11:37 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1061f4d085b5

8029483: BufferedReader.lines() javadoc typo should be fixed
Reviewed-by: mduigou

! src/share/classes/java/io/BufferedReader.java



Re: RFR: 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic

2013-12-03 Thread Henry Jen

I have separated the fix into two part,

8029434: Spliterator of Stream returned by BufferedReader.lines() should 
have NONNULL characteristic


8029483: BufferedReader.lines() javadoc typo should be fixed

So that we can at least fix the javadoc by jdk8 release if not the 
characteristic.


Cheers,
Henry

On 12/03/2013 10:52 AM, Mike Duigou wrote:

Looks good to me.

Mike

On Dec 3 2013, at 10:36 , Henry Jen henry@oracle.com wrote:


Hi,

Please review a small fix that add missing NONNULL characteristic and cleanup 
in javadoc.

Thanks Anthony Vanelverdinghe for reporting of this bug.

http://cr.openjdk.java.net/~henryjen/tl/8029434/0/webrev/

Cheers,
Henry




Re: RFR: 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic

2013-12-03 Thread Mike Duigou
Go ahead with pushing 8029483 as no approval is required for doc only change.

Mike

On Dec 3 2013, at 11:49 , Henry Jen henry@oracle.com wrote:

 I have separated the fix into two part,
 
 8029434: Spliterator of Stream returned by BufferedReader.lines() should have 
 NONNULL characteristic
 
 8029483: BufferedReader.lines() javadoc typo should be fixed
 
 So that we can at least fix the javadoc by jdk8 release if not the 
 characteristic.
 
 Cheers,
 Henry
 
 On 12/03/2013 10:52 AM, Mike Duigou wrote:
 Looks good to me.
 
 Mike
 
 On Dec 3 2013, at 10:36 , Henry Jen henry@oracle.com wrote:
 
 Hi,
 
 Please review a small fix that add missing NONNULL characteristic and 
 cleanup in javadoc.
 
 Thanks Anthony Vanelverdinghe for reporting of this bug.
 
 http://cr.openjdk.java.net/~henryjen/tl/8029434/0/webrev/
 
 Cheers,
 Henry
 



hg: jdk8/tl/jdk: 8029475: Fix more doclint issues in javax.security

2013-12-03 Thread joe . darcy
Changeset: cd4aabc40f72
Author:darcy
Date:  2013-12-03 11:52 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cd4aabc40f72

8029475: Fix more doclint issues in javax.security
Reviewed-by: juh

! src/share/classes/javax/crypto/Cipher.java
! src/share/classes/javax/crypto/CipherSpi.java
! src/share/classes/javax/crypto/KeyGenerator.java
! src/share/classes/javax/crypto/SealedObject.java
! src/share/classes/javax/net/ssl/SSLEngine.java
! src/share/classes/javax/net/ssl/SSLPermission.java
! src/share/classes/javax/security/auth/PrivateCredentialPermission.java
! src/share/classes/javax/security/auth/kerberos/DelegationPermission.java
! src/share/classes/javax/security/auth/kerberos/ServicePermission.java
! src/share/classes/javax/security/auth/login/LoginContext.java
! src/share/classes/javax/security/auth/x500/X500Principal.java



Re: RFR: 8028757: CharSequence.subSequence improperly requires a new CharSequence be returned

2013-12-03 Thread Alan Bateman

On 03/12/2013 18:15, Stuart Marks wrote:

Hi all,

Please review this small change to the subSequence() method specs of 
CharSequence and String. Essentially this removes the requirement of 
returning a new character sequence at each call. This brings the 
spec in line with String's implementation, which will return 'this' in 
appropriate circumstances.


No change is necessary for the other CharSequence implementations. 
StringBuilder/Buffer still say new and in fact they always return 
new String objects. CharBuffer defines its exact sharing behavior. 
Segment returns a new Segment with a shared character array, which 
is (supposedly) immutable.

Looks good and I think is the best solution for this.

-Alan


Re: RFR: MethodHandles.Lookup. Wrong access verification

2013-12-03 Thread Sebastian Sickelmann
Thanks.

I would like to add the test[1] prior to fixing it.
JDK 7 shows the same behavior.

As i am not an author of any openjdk-project but i signed the OCA some
time ago i would love to find and sponsor for this[1].

— Sebastian

Am 03.12.2013 01:08, schrieb John Rose:
 I appreciate the report and the proposed fixes.  We are looking at
 this seriously.

 — John

 P.S.  The root issue may be an accidental inheritance of private
 methods.   I have been thinking for a while that, in the name of
 compatibility, the JVMS allows an invokevirtual invocation to reach
 a private method, but that this is unfortunate, since invokespecial
 already supplies a more specific way to get to the method.  Likewise,
 an invokeinterface instruction can reach a virtual method of Object,
 while invokevirtual gives a more specific way.  There are a number
 of odd pathways through the linkResolver, to support these extra
 combinations, that I would like to regularize or remove.  I think we
 should investigate tightening up the JVM linkage rules to avoid the
 extra corner cases.  That will probably have the effect of removing
 surprise effects like this one, as we continue to extend the JVM
 linkage rules.


 On Dec 2, 2013, at 1:42 PM, Sebastian Sickelmann
 sebastian.sickelm...@gmx.de mailto:sebastian.sickelm...@gmx.de wrote:

 Hi,

 some days ago i had written[0] that i maybe found a bug in access
 verification in MethodHandles.Lookup.

 Now i produces webrev's for the two repos jdk and hotspot.

 In the jdk webrev [1] I implemented some additional tests.

 In the hotspot webrev [2] I tried to fix the wrong behavior. I am not
 sure if it is the right place to fix this.
 It is more an proof-of-concept--like--reverse-engineering-fix that
 fixes the symptom, and i do not fully
 understand all the things in the patched source. In fact i think that
 we can move line 187 to 182 instead
 of chaning line 245 in methodHandles.cpp. But as i said i do not
 fully understand init_method_MemberName
 and all those CallInfo::vtable_call / CallInfo::itable_call /
 CallInfo::direct_call differences.

 Hope my works helps solving this issue. And i hope it is an issue.
 And i hope my fix isn't breaking anything else
 so that it was enough to test my fix against my new testcase in
 MethodHandlesTest.testFindPrivate

 Kind regards
 Sebastian

 [0]
 http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2013-November/010228.html
 [1]
 https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk8/MethodHandles.Lookup/webrev_jdk/index.html
 [2]
 https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk8/MethodHandles.Lookup/webrev_hotspot/index.html





hg: jdk8/tl/jdk: 8022181: Tune algorithm crossover thresholds in BigInteger

2013-12-03 Thread brian . burkhalter
Changeset: c138b0d33980
Author:bpb
Date:  2013-12-03 12:25 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c138b0d33980

8022181: Tune algorithm crossover thresholds in BigInteger
Summary: Change multiplication, squaring, division, and base conversion 
thresholds to values which retain performance improvement in most cases but 
with a a lower overall risk of regression.
Reviewed-by: darcy

! src/share/classes/java/math/BigInteger.java



Re: 8029281: Synchronization issues in Logger and LogManager

2013-12-03 Thread Mandy Chung

On 12/3/2013 10:57 AM, Daniel Fuchs wrote:

Hi Mandy,

I have updated the webrev taking your comments into account.

http://cr.openjdk.java.net/~dfuchs/webrev_8029281/webrev.02/



Looks much cleaner.  Thanks for the update.

Nit LogManager.java L1050: formatting - one extra space.  No need to 
generate a new webrev.


Mandy


Changes are small and outlined below:

On 12/3/13 1:49 AM, Mandy Chung wrote:

On 11/29/2013 3:41 AM, Daniel Fuchs wrote:

Hi,

Here is a new revision that includes Peter's findings.
Thanks again for that Peter!

http://cr.openjdk.java.net/~dfuchs/webrev_8029281/webrev.01/



LogManager.java
L156: make props volatile is good.

I think the reason why LoggerWeakRef.node and parentRef don't need
to be volatile because of the synchronized block to check and set
disposed flag that ensures that only one thread will be doing the node
and parentRef cleanup.  I agree that the two lines should be done with
synchronized on the LoggerContext.

  node.context.removeLogger(name);
  node.loggerRef = null;  // clear LogNode's weak ref to us

I don't see anything obvious wrong with this patch.   Have you
considered having the dispose() method simply synchronizes on
node.context (probably need to get the LoggerContext when instantiating
LoggerWeakRef and break dispose method into two - one for the node and
the other for the parentRef)?I'm not proposing to do this in JDK 8
since we are ramping down and get a low risk fix in.   It may be
something to revisit for jdk 9 to a cleaner version.


I believe that nulling the instance variables in LoggerWeakRef
is probably no longer needed - since we now should be calling
dispose only once - but I didn't want to change the code too
much. I agree it would be better to revisit that in 9.



Logger.java
The assert this.loggerBundle == NO_RESOURCE_BUNDLE; in the Logger
constructors doesn't seem to be needed.


OK - removed.


Having an immutable LoggerBundle class is good to ensure the
resourceBundleName and resourceBundle are consistent.

L1930: is lb.userBundle and lb.resourceBundleName null when getting
to this line?  Might be better to rename setupResourceInfo to
setResourceBundleName (that creates a LoggerBundle with a null RB). On
the other hand,  setResourceBundle will instantiate a LoggerBundle with
non-null rbname and RB.  It wasn't obvious to me what does what.


Right. Let's keep the old name for now. We can revisit the naming in 9.
You right that when we reach the last line lb.userBundle must be null,
otherwise we would have either returned or thrown an exception, since
lb.userBundle != null = lb.resourceBundleName != null.

Therefore I added an assert lb.userBundle == null and
replaced the call to
  LoggerBundle.get(name, lb.userBundle)
by
  LoggerBundle.get(name, null)

Hopefully it will make what happens more obvious.


2137 final String rbName = getResourceBundleName();
2138 if (!SYSTEM_LOGGER_RB_NAME.equals(rbName)
2139 
!SYSTEM_LOGGER_RB_NAME.equals(b.getBaseBundleName())) {
2140 return LoggerBundle.get(rbName, b);
2141 } else {
2142 return SYSTEM_BUNDLE;
2143 }

To get to the above line, either lb.userBundle is null or
getResourceBundle() isoverridden.  L2126 already checks that this logger
does not associate with the system rbname.  It seems to me that the
special case here is when the Logger subclass overrides
getResourceBundleName() to returnSYSTEM_LOGGER_RB_NAME or override
getResourceBundle to return the sun.util.logging.resources.logging RB.
I tink that we might just simplify the above and just return
LoggerBundle.get(rbName, b) and no need to worry about those overridden
case which does no use to such a Logger subclass with unmatched rbname
and RB.  Same comment applied to L2157-2165.


Ah. I'm glad to hear that. This is a great simplification.
done.

-- daniel




thanks
Mandy






RFR: JDK-8029117: (reflect) clarify javadoc for getMethod(...) and getMethods()

2013-12-03 Thread Joel Borggrén-Franck
Hi

Please review this javadoc fix for Class.getMethod(name, params) and 
Class.getMethods() when called on an interface.

This fix aligns the javadoc with the long standing implementation to not return 
any implicitly declared object methods when calling getMethod(s) on an 
interface.

Bug: https://bugs.openjdk.java.net/browse/JDK-8029117
Webrev: http://cr.openjdk.java.net/~jfranck/8029117/webrev.01/

cheers
/Joel

hg: jdk8/tl/jdk: 8028019: AWT Doclint warning/error cleanup

2013-12-03 Thread roger . riggs
Changeset: 3e95aadb479f
Author:rriggs
Date:  2013-12-03 16:20 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3e95aadb479f

8028019: AWT Doclint warning/error cleanup
Summary: Fix numerious javadoc and html errors and warnings
Reviewed-by: yan

! src/share/classes/java/applet/Applet.java
! src/share/classes/java/applet/AppletContext.java
! src/share/classes/java/awt/AWTPermission.java
! src/share/classes/java/awt/AlphaComposite.java
! src/share/classes/java/awt/BasicStroke.java
! src/share/classes/java/awt/BorderLayout.java
! src/share/classes/java/awt/Button.java
! src/share/classes/java/awt/Checkbox.java
! src/share/classes/java/awt/CheckboxGroup.java
! src/share/classes/java/awt/CheckboxMenuItem.java
! src/share/classes/java/awt/Choice.java
! src/share/classes/java/awt/Component.java
! src/share/classes/java/awt/Container.java
! src/share/classes/java/awt/EventFilter.java
! src/share/classes/java/awt/EventQueue.java
! src/share/classes/java/awt/FileDialog.java
! src/share/classes/java/awt/FlowLayout.java
! src/share/classes/java/awt/Font.java
! src/share/classes/java/awt/Graphics.java
! src/share/classes/java/awt/GridBagConstraints.java
! src/share/classes/java/awt/GridBagLayout.java
! src/share/classes/java/awt/GridLayout.java
! src/share/classes/java/awt/Label.java
! src/share/classes/java/awt/LinearGradientPaint.java
! src/share/classes/java/awt/LinearGradientPaintContext.java
! src/share/classes/java/awt/List.java
! src/share/classes/java/awt/MenuItem.java
! src/share/classes/java/awt/RadialGradientPaint.java
! src/share/classes/java/awt/Scrollbar.java
! src/share/classes/java/awt/SystemColor.java
! src/share/classes/java/awt/SystemTray.java
! src/share/classes/java/awt/TextArea.java
! src/share/classes/java/awt/TextField.java
! src/share/classes/java/awt/Toolkit.java
! src/share/classes/java/awt/WaitDispatchSupport.java
! src/share/classes/java/awt/Window.java
! src/share/classes/java/awt/color/CMMException.java
! src/share/classes/java/awt/color/ColorSpace.java
! src/share/classes/java/awt/color/ICC_ColorSpace.java
! src/share/classes/java/awt/color/ICC_Profile.java
! src/share/classes/java/awt/color/ICC_ProfileGray.java
! src/share/classes/java/awt/color/ICC_ProfileRGB.java
! src/share/classes/java/awt/dnd/DnDEventMulticaster.java
! src/share/classes/java/awt/dnd/DragSourceDropEvent.java
! src/share/classes/java/awt/dnd/DropTarget.java
! src/share/classes/java/awt/event/MouseAdapter.java
! src/share/classes/java/awt/font/FontRenderContext.java
! src/share/classes/java/awt/font/StyledParagraph.java
! src/share/classes/java/awt/geom/AffineTransform.java
! src/share/classes/java/awt/geom/QuadCurve2D.java
! src/share/classes/java/awt/image/BufferStrategy.java
! src/share/classes/java/awt/image/BufferedImage.java
! src/share/classes/java/awt/image/ColorConvertOp.java
! src/share/classes/java/awt/peer/CheckboxPeer.java
! src/share/classes/java/awt/peer/DesktopPeer.java



hg: jdk8/tl/langtools: 8025416: doclet not substituting {@docRoot} in some cases

2013-12-03 Thread bhavesh . x . patel
Changeset: 4cb9de4dd420
Author:bpatel
Date:  2013-12-03 14:21 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/4cb9de4dd420

8025416: doclet not substituting {@docRoot} in some cases
Reviewed-by: jjg

! src/share/classes/com/sun/tools/doclets/formats/html/HtmlDocletWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/TagletWriterImpl.java
! test/com/sun/javadoc/testDocRootLink/TestDocRootLink.java
! test/com/sun/javadoc/testDocRootLink/pkg1/C1.java
! test/com/sun/javadoc/testDocRootLink/pkg1/package.html
! test/com/sun/javadoc/testDocRootLink/pkg2/C2.java
! test/com/sun/javadoc/testDocRootLink/pkg2/package.html



RFR: 8028816: Add value-type notice to Optional* classes

2013-12-03 Thread Mike Duigou
Hello all;

There's been a discussion on the lambda spec experts list 
(http://mail.openjdk.java.net/pipermail/lambda-spec-experts/) about adding a 
notice to the Optional classes about implications of their likely future as 
values. This discussion recently completed so now there's a doc patch to review:

http://cr.openjdk.java.net/~mduigou/JDK-8028816/0/webrev/

I have already reviewed this but will hold off pushing it for a few hours in 
case someone notices a mistake that I did not.

Mike

Re: RFR: 8029055: Map.merge must refuse null values

2013-12-03 Thread Mike Duigou

On Nov 26 2013, at 05:35 , Stephen Colebourne scolebou...@joda.org wrote:

 See the new thread for the general Javadoc issues. I'll focus on null here.
 
 Given a map {Key - null} I find the notion that putIfAbsent() or
 computeIfAbsent() ignore the existing mapping just plain wrong. While
 I can rationalise it (just) it is a horrible reworking of the meaning
 of absent.

We tried it a couple of ways. The interpretation of null mapping as absent 
was the compromise we came up with. 

 Similarly, for computeIfPresent(), my expectation is that the
 mapping is present, and that the null should be an input to the
 function.

This would have meant contradictory interpretations of absent for the different 
methods. 

 Yes, I get that nulls in collections are a pain, but trying to
 redefine the meaning of absent is more of a pain.

The problem is that not all of these methods are new, some are backports from 
java.util.concurrent.ConcurrentMap where there were existing null handling 
semantics (which didn't take into account null values). Producing consistent 
results across implementations seemed most important. This meant favouring the 
null means absent semantic rather than the null might mean absent or might 
mean a null mapping.

 I'm aware that
 ConcurrentMap makes things interesting, but these two just look wrong.
 
 
 So, my rules would be
 - if an existing mapping has a null value, then the mapping is
 present and not absent (only affects methods with those words in
 their name)
 - none of these methods should try to add/update a mapping with a null value

That seems too surprising.

 - null returned from a function means remove (or is invalid)
 - a null value parameter is invalid

I had chosen to add this rule for merge() in my most recent webrev to reduce 
the complexity. It kind of goes against the null is fully supported as a 
value.

 - merge where the existing mapping has a null value should not pass
 the null to the function

This is the same as treating it as absent. 

 
 Stephen
 
 
 
 On 26 November 2013 04:32, Mike Duigou mike.dui...@oracle.com wrote:
 
 On Nov 24 2013, at 16:31 , David Holmes david.hol...@oracle.com wrote:
 
 Hi Mike,
 
 There is still no clear spec for what should happen if the param value is 
 null.
 
 I feel very uncomfortable the status quo of with null being ignored, used 
 for a sentinel and also as value. The relations between null and values in 
 this method are just too complicated.
 
 Currently:
 
 - The provided value may be either null or non-null. Is null a legal value? 
 It depends upon:
- Is there an existing value?
- Does the Map allow null values?
- Does the function allow null values?
 - Existing null values are treated as absent.
- If a null value is passed should we remove this mapping or add it 
 to the map?
  - null might not be accepted by the map
  - The associated value would still be regarded as absent for 
 future operations.
 - The function may return null to signal remove.
 
 In particular I dislike adding a null entry to the map if there is no 
 current mapping (or mapped to null). It seems like it should be invariant 
 whether we end up with an associated value. If null is used to signify 
 remove then map.contains(key) will return variable results depending upon 
 the initial state. Having the behaviour vary based upon whether the map 
 allows null values would be even worse.
 
 So I would like to suggest that we require value to be non-null. I have 
 provisionally updated the spec and impls accordingly.
 
 The parenthesized part is wrong.
 
 I think that's overzealous copying from compute(). I have removed it.
 
 
 Also you have changed the method implementation not just the implDoc so the 
 bug synopsis is somewhat misleading.
 
 I will correct this. More changes were made than I originally expected. New 
 synopsis will be Map.merge implementations should refuse null value param
 
 I have updated the webrev.
 
 http://cr.openjdk.java.net/~mduigou/JDK-8029055/1/webrev/
 
 Thanks,
 
 Mike
 
 
 Thanks,
 David
 
 On 23/11/2013 10:25 AM, Mike Duigou wrote:
 We'll be using https://bugs.openjdk.java.net/browse/JDK-8029055 for this 
 issue.
 
 I've posted a webrev here:
 
 http://cr.openjdk.java.net/~mduigou/JDK-8029055/0/webrev/
 
 There is an identical change in ConcurrentMap's merge().
 
 Mike
 
 On Nov 22 2013, at 16:01 , Henry Jen henry@oracle.com wrote:
 
 
 On 11/21/2013 06:33 PM, David Holmes wrote:
 On 22/11/2013 5:02 AM, Louis Wasserman wrote:
 While I agree that case should be specified, I'm not certain I follow 
 why
 that's what's going on here.
 
 The weird condition is that if oldValue is null, not value; oldValue
 is the
 old result of map.get(key).  The Javadoc seems not just unspecified, but
 actively wrong.  Here's a worked example:
 
 MapString, Integer map = new HashMap();
 map.merge(foo, 3, Integer::plus);
 Integer fooValue = map.get(foo);
 
 Going through the 

Re: Theoretical data race on java.util.logging.Handler.sealed

2013-12-03 Thread Mandy Chung


On 12/3/2013 1:44 AM, Peter Levart wrote:

On 12/03/2013 09:51 AM, Peter Levart wrote:

Hi,

While browsing the code of java.util.logging.Handler, I noticed a 
theoretical possibility that a security check in a 
j.u.l.StreamHandler be circumvented using a data race.


There is a plain boolean instance field 'sealed' in j.u.l.Handler 
that is pre-initialized to 'true' in field initializer. StreamHandler 
sublcass' constructors overwrite this value with 'false' at the 
beginning, then issue some operations which circumvent security 
checks, and finally they reset the 'sealed' value back to 'true' at 
the end.


If a reference to an instance of StreamHandler or subclass is passed 
to some thread without synchronization via data-race, this thread can 
see 'true' or 'false' as the possible values of 'sealed' variable, 
thus it is possible to circumvent security checks.


One possibility to fix this is moving the field to StreamHandler and 
making it final:


http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.01/ 



Just making the field volatile might not work. There is an ongoing 
debate on concurrency-interest which suggests that volatile fields 
are not exceptional in constructors like final fields are...



Regards, Peter



The proposed patch is not complete. There are several subclasses of 
StreamHandler in the java.util.logging package that also need a way to 
bypass security checks for some operations in their constructors. So 
here's the updated webrev which updates them with the same code as 
StreamHandler. This means that there are two copies of 'sealed' flag 
in object of type ConsoleHandler, for example, but only the one 
declared in ConsoleHandler is relevant for governing access checks:


http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.02/

Before filing the bug, I'm asking the list whether this can be 
considered a bug...




This does look a possible data race that might return a partially 
constructed object with sealed = false.  I am not sure how likely we 
will run into this race though.


W.r.t. the patch, it might be better to get rid of the sealed field and 
wrap the calls with doPrivileged with limited privilege (just 
LoggingPermission(control))


Mandy


hg: jdk8/tl/jdk: 8028757: CharSequence.subSequence improperly requires a new CharSequence be returned

2013-12-03 Thread stuart . marks
Changeset: accd6ffd4b3f
Author:smarks
Date:  2013-12-03 15:52 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/accd6ffd4b3f

8028757: CharSequence.subSequence improperly requires a new CharSequence be 
returned
Reviewed-by: alanb, darcy, mduigou

! src/share/classes/java/lang/CharSequence.java
! src/share/classes/java/lang/String.java



hg: jdk8/tl/jdk: 8028351: JWS doesn't get authenticated when using kerberos auth proxy

2013-12-03 Thread weijun . wang
Changeset: e1bc55ddf1ad
Author:weijun
Date:  2013-12-04 09:14 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e1bc55ddf1ad

8028351: JWS doesn't get authenticated when using kerberos auth proxy
Reviewed-by: xuelei

! src/share/classes/com/sun/security/auth/module/Krb5LoginModule.java
! test/sun/security/krb5/auto/KDC.java
+ test/sun/security/krb5/auto/LoginNoPass.java



hg: jdk8/tl/jdk: 8029281: Synchronization issues in Logger and LogManager

2013-12-03 Thread daniel . fuchs
Changeset: 9f624e115c6b
Author:dfuchs
Date:  2013-12-04 01:58 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/9f624e115c6b

8029281: Synchronization issues in Logger and LogManager
Summary: Fixes several race conditions in logging which have been at the root 
cause of intermittent test failures.
Reviewed-by: mchung, plevart

! src/share/classes/java/util/logging/LogManager.java
! src/share/classes/java/util/logging/Logger.java
+ test/java/util/logging/TestLogConfigurationDeadLock.java
+ test/java/util/logging/TestLoggerBundleSync.java



hg: jdk8/tl/jdk: 2 new changesets

2013-12-03 Thread valerie . peng
Changeset: d922c8aba2f8
Author:valeriep
Date:  2013-12-03 17:23 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d922c8aba2f8

8029158: sun/security/pkcs11/Signature/TestDSAKeyLength.java does not compile 
(or run)
Summary: Add the missing library path and skip testing against NSS 1.14 or 
later due to known NSS issue
Reviewed-by: vinnie, ascarpino

! test/sun/security/pkcs11/Signature/TestDSAKeyLength.java

Changeset: 75165f6c1c50
Author:valeriep
Date:  2013-12-03 17:25 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/75165f6c1c50

Merge




JDK 8 RFR 8029501: BigInteger division algorithm selection heuristic is incorrect

2013-12-03 Thread Brian Burkhalter
Hello,

Issue:  https://bugs.openjdk.java.net/browse/JDK-8029501
Webrev: http://cr.openjdk.java.net/~bpb/8029501/webrev/

This patch would change the division algorithm selection heuristic as 
previously described in [1]. Many subsequent performance benchmark runs have 
determined that the threshold offset should be 40 if the division threshold 
itself is 80 [2]. With the existing algorithm selection heuristic, i.e., 
dividend and divisor int-length both above the tB-Z threshold, performance 
regressions of up to 100% will be observed when the int-lengths of the dividend 
and divisor exceed the B-Z threshold but the dividend int-length is less than 
approximately 40 more than the divisor int-length. The proposed patch fixes 
that problem.

Thanks,

Brian

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/023493.html
[2] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/023668.html

Re: JDK 8 RFR 8029501: BigInteger division algorithm selection heuristic is incorrect

2013-12-03 Thread Joseph Darcy

Looks good Brian; thanks,

-Joe

On 12/3/2013 5:33 PM, Brian Burkhalter wrote:

Hello,

Issue:  https://bugs.openjdk.java.net/browse/JDK-8029501
Webrev: http://cr.openjdk.java.net/~bpb/8029501/webrev/

This patch would change the division algorithm selection heuristic as 
previously described in [1]. Many subsequent performance benchmark runs have 
determined that the threshold offset should be 40 if the division threshold 
itself is 80 [2]. With the existing algorithm selection heuristic, i.e., 
dividend and divisor int-length both above the tB-Z threshold, performance 
regressions of up to 100% will be observed when the int-lengths of the dividend 
and divisor exceed the B-Z threshold but the dividend int-length is less than 
approximately 40 more than the divisor int-length. The proposed patch fixes 
that problem.

Thanks,

Brian

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-November/023493.html
[2] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/023668.html




RFR: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted

2013-12-03 Thread Stuart Marks

Hi all,

Please review the following small javadoc change. The StringJoiner doc for a 
couple methods uses i.e. in the first sentence, which screws up the javadoc 
logic that pulls the first sentence into the Method Summary. This is an 
editorial change to fix this up; there is no actual specification change.


Thanks!

s'marks

# HG changeset patch
# User smarks
# Date 1386121046 28800
# Node ID 0bc91b9f6afd9b4dceea31ecd1036e367b365690
# Parent  75165f6c1c505ff43f7fd235a95b2e7955413b78
8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
Reviewed-by: XXX

diff -r 75165f6c1c50 -r 0bc91b9f6afd 
src/share/classes/java/util/StringJoiner.java
--- a/src/share/classes/java/util/StringJoiner.java Tue Dec 03 17:25:28 
2013 -0800
+++ b/src/share/classes/java/util/StringJoiner.java Tue Dec 03 17:37:26 
2013 -0800
@@ -131,7 +131,7 @@
 /**
  * Sets the sequence of characters to be used when determining the string
  * representation of this {@code StringJoiner} and no elements have been
- * added yet, i.e. when it is empty.  A copy of the {@code emptyValue}
+ * added yet, that is, when it is empty.  A copy of the {@code emptyValue}
  * parameter is made for this purpose. Note that once an add method has 
been
  * called, the {@code StringJoiner} is no longer considered empty, even if
  * the element(s) added correspond to the empty {@code String}.
@@ -228,8 +228,8 @@
 }

 /**
- * The length of the {@code StringJoiner} value, i.e. the length of
- * {@code String} representation of the {@code StringJoiner}. Note that if
+ * Returns the length of the {@code String} representation
+ * of this {@code StringJoiner}. Note that if
  * no add methods have been called, then the length of the {@code String}
  * representation (either {@code prefix + suffix} or {@code emptyValue})
  * will be returned. The value should be equivalent to


Re: RFR: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted

2013-12-03 Thread Lance Andersen - Oracle
looks OK
On Dec 3, 2013, at 8:40 PM, Stuart Marks wrote:

 Hi all,
 
 Please review the following small javadoc change. The StringJoiner doc for a 
 couple methods uses i.e. in the first sentence, which screws up the javadoc 
 logic that pulls the first sentence into the Method Summary. This is an 
 editorial change to fix this up; there is no actual specification change.
 
 Thanks!
 
 s'marks
 
 # HG changeset patch
 # User smarks
 # Date 1386121046 28800
 # Node ID 0bc91b9f6afd9b4dceea31ecd1036e367b365690
 # Parent  75165f6c1c505ff43f7fd235a95b2e7955413b78
 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
 Reviewed-by: XXX
 
 diff -r 75165f6c1c50 -r 0bc91b9f6afd 
 src/share/classes/java/util/StringJoiner.java
 --- a/src/share/classes/java/util/StringJoiner.java   Tue Dec 03 17:25:28 
 2013 -0800
 +++ b/src/share/classes/java/util/StringJoiner.java   Tue Dec 03 17:37:26 
 2013 -0800
 @@ -131,7 +131,7 @@
 /**
  * Sets the sequence of characters to be used when determining the string
  * representation of this {@code StringJoiner} and no elements have been
 - * added yet, i.e. when it is empty.  A copy of the {@code emptyValue}
 + * added yet, that is, when it is empty.  A copy of the {@code 
 emptyValue}
  * parameter is made for this purpose. Note that once an add method has 
 been
  * called, the {@code StringJoiner} is no longer considered empty, even if
  * the element(s) added correspond to the empty {@code String}.
 @@ -228,8 +228,8 @@
 }
 
 /**
 - * The length of the {@code StringJoiner} value, i.e. the length of
 - * {@code String} representation of the {@code StringJoiner}. Note that 
 if
 + * Returns the length of the {@code String} representation
 + * of this {@code StringJoiner}. Note that if
  * no add methods have been called, then the length of the {@code String}
  * representation (either {@code prefix + suffix} or {@code emptyValue})
  * will be returned. The value should be equivalent to


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



Re: RFR: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted

2013-12-03 Thread Mike Duigou
Looks fine.

On Dec 3 2013, at 17:40 , Stuart Marks stuart.ma...@oracle.com wrote:

 Hi all,
 
 Please review the following small javadoc change. The StringJoiner doc for a 
 couple methods uses i.e. in the first sentence, which screws up the javadoc 
 logic that pulls the first sentence into the Method Summary. This is an 
 editorial change to fix this up; there is no actual specification change.
 
 Thanks!
 
 s'marks
 
 # HG changeset patch
 # User smarks
 # Date 1386121046 28800
 # Node ID 0bc91b9f6afd9b4dceea31ecd1036e367b365690
 # Parent  75165f6c1c505ff43f7fd235a95b2e7955413b78
 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
 Reviewed-by: XXX
 
 diff -r 75165f6c1c50 -r 0bc91b9f6afd 
 src/share/classes/java/util/StringJoiner.java
 --- a/src/share/classes/java/util/StringJoiner.java   Tue Dec 03 17:25:28 
 2013 -0800
 +++ b/src/share/classes/java/util/StringJoiner.java   Tue Dec 03 17:37:26 
 2013 -0800
 @@ -131,7 +131,7 @@
 /**
  * Sets the sequence of characters to be used when determining the string
  * representation of this {@code StringJoiner} and no elements have been
 - * added yet, i.e. when it is empty.  A copy of the {@code emptyValue}
 + * added yet, that is, when it is empty.  A copy of the {@code 
 emptyValue}
  * parameter is made for this purpose. Note that once an add method has 
 been
  * called, the {@code StringJoiner} is no longer considered empty, even if
  * the element(s) added correspond to the empty {@code String}.
 @@ -228,8 +228,8 @@
 }
 
 /**
 - * The length of the {@code StringJoiner} value, i.e. the length of
 - * {@code String} representation of the {@code StringJoiner}. Note that 
 if
 + * Returns the length of the {@code String} representation
 + * of this {@code StringJoiner}. Note that if
  * no add methods have been called, then the length of the {@code String}
  * representation (either {@code prefix + suffix} or {@code emptyValue})
  * will be returned. The value should be equivalent to



Re: RFR: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted

2013-12-03 Thread Joseph Darcy

Hi Stuart,

Looks good; thanks,

-Joe

On 12/3/2013 5:40 PM, Stuart Marks wrote:

Hi all,

Please review the following small javadoc change. The StringJoiner doc 
for a couple methods uses i.e. in the first sentence, which screws 
up the javadoc logic that pulls the first sentence into the Method 
Summary. This is an editorial change to fix this up; there is no 
actual specification change.


Thanks!

s'marks

# HG changeset patch
# User smarks
# Date 1386121046 28800
# Node ID 0bc91b9f6afd9b4dceea31ecd1036e367b365690
# Parent  75165f6c1c505ff43f7fd235a95b2e7955413b78
8029489: StringJoiner spec for setEmptyValue() and length() is 
malformatted

Reviewed-by: XXX

diff -r 75165f6c1c50 -r 0bc91b9f6afd 
src/share/classes/java/util/StringJoiner.java
--- a/src/share/classes/java/util/StringJoiner.javaTue Dec 03 
17:25:28 2013 -0800
+++ b/src/share/classes/java/util/StringJoiner.javaTue Dec 03 
17:37:26 2013 -0800

@@ -131,7 +131,7 @@
 /**
  * Sets the sequence of characters to be used when determining 
the string
  * representation of this {@code StringJoiner} and no elements 
have been
- * added yet, i.e. when it is empty.  A copy of the {@code 
emptyValue}
+ * added yet, that is, when it is empty.  A copy of the {@code 
emptyValue}
  * parameter is made for this purpose. Note that once an add 
method has been
  * called, the {@code StringJoiner} is no longer considered 
empty, even if

  * the element(s) added correspond to the empty {@code String}.
@@ -228,8 +228,8 @@
 }

 /**
- * The length of the {@code StringJoiner} value, i.e. the length of
- * {@code String} representation of the {@code StringJoiner}. 
Note that if

+ * Returns the length of the {@code String} representation
+ * of this {@code StringJoiner}. Note that if
  * no add methods have been called, then the length of the {@code 
String}
  * representation (either {@code prefix + suffix} or {@code 
emptyValue})

  * will be returned. The value should be equivalent to




hg: jdk8/tl/jdk: 8028397: Undo the lenient MIME BASE64 decoder support change (JDK-8025003) and remove methods de/encode(buf, buf)

2013-12-03 Thread xueming . shen
Changeset: 301d76b8cb55
Author:sherman
Date:  2013-12-03 17:44 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/301d76b8cb55

8028397: Undo the lenient MIME BASE64 decoder support change (JDK-8025003) and 
remove methods de/encode(buf, buf)
Summary: updated the spec and implementation as requested
Reviewed-by: alanb

! src/share/classes/java/util/Base64.java
! test/java/util/Base64/Base64GetEncoderTest.java
! test/java/util/Base64/TestBase64.java
! test/java/util/Base64/TestBase64Golden.java



hg: jdk8/tl/jdk: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted

2013-12-03 Thread stuart . marks
Changeset: c6b6b515cf4f
Author:smarks
Date:  2013-12-03 18:19 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c6b6b515cf4f

8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
Reviewed-by: darcy, lancea, mduigou

! src/share/classes/java/util/StringJoiner.java



Re: RFR: 8028816: Add value-type notice to Optional* classes

2013-12-03 Thread Mike Duigou

On Dec 3 2013, at 18:06 , Stuart Marks stuart.ma...@oracle.com wrote:

 Overall looks fine.
 
 If you're listing yourself as the reviewer, jcheck will object if you're also 
 the changeset author. Instead of listing Brian Goetz in Contributed-by, make 
 him the changeset author instead. Using MQ, do hg qref -u briangoetz.

OK. I am reluctant to use the -u option but don't want to anger jcheck.

 
 The gist of the paragraph being added to each class is,
 
Use of identity-sensitive operations ... on instances of class
may have unpredictable effects and should be avoided.
 
 The phrase unpredictable effects strikes me oddly. This phrase is also used 
 at the very end of the HTML doc. It makes it sound as if using an 
 identity-sensitive operation might have side effects. That won't be the case, 
 as far as I know. Using such an operation will indeed have unpredictable 
 results. That phrase is used at the beginning of the last paragraph of the 
 HTML doc, and it makes much more sense to me than unpredictable effects.

I prefer results to effects as well.

Mike

 
 s'marks
 
 
 
 On 12/3/13 2:21 PM, Mike Duigou wrote:
 Hello all;
 
 There's been a discussion on the lambda spec experts list 
 (http://mail.openjdk.java.net/pipermail/lambda-spec-experts/) about adding a 
 notice to the Optional classes about implications of their likely future as 
 values. This discussion recently completed so now there's a doc patch to 
 review:
 
 http://cr.openjdk.java.net/~mduigou/JDK-8028816/0/webrev/
 
 I have already reviewed this but will hold off pushing it for a few hours in 
 case someone notices a mistake that I did not.
 
 Mike
 



JDK 8 RFR for JDK-8023471, , Add compatibility note to AnnotatedElement

2013-12-03 Thread Joe Darcy

Hello,

Please review the patch below to address

JDK-8023471 Add compatibility note to AnnotatedElement

Thanks,

-Joe

diff -r cd4aabc40f72 
src/share/classes/java/lang/reflect/AnnotatedElement.java
--- a/src/share/classes/java/lang/reflect/AnnotatedElement.java Tue Dec 
03 11:52:18 2013 -0800
+++ b/src/share/classes/java/lang/reflect/AnnotatedElement.java Tue Dec 
03 21:23:16 2013 -0800

@@ -135,7 +135,63 @@
  * annotations on iE/i are directly present on iE/i in place
  * of their container annotation, in the order in which they appear in
  * the value element of the container annotation.
-
+ *
+ * pThere are several compatibility concerns to keep in mind if an
+ * annotation type iT/i is emnot/em repeatable in one release
+ * of a library and retrofitted to be repeatable in a subsequent
+ * release.
+ *
+ * ul
+ *
+ * liIf an annotation of type iT/i is present on an
+ * element and iT/i is made repeatable and more annotations of
+ * type iT/i are added to the element:
+ *
+ *
+ * ul
+ *
+ * li The addition of the additional annotations is both source
+ * compatible and binary compatible. That is, the source containing
+ * the element and its annotations will still compile and the class
+ * file resulting from the compilation will continue to link.
+ *
+ * liThe addition of the additional annotations is emnot/em
+ * behaviorally compatible with respect to the {@code
+ * get[Declared]Annotation(ClassT)} methods and {@code
+ * get[Declared]Annotations()} methods, because those methods will now
+ * only see a container annotations on the element and not see an
+ * annotation of type iT/i.
+ *
+ * /ul
+ *
+ * liIf an annotation type iTC/i is present
+ * on an element, then making some other annotation type iT/i
+ * repeatable with iTC/i as its containing annotation type then:
+ *
+ * ul
+ *
+ * liThe change to iT/iis source and binary compatible with
+ * existing uses of annotations of type iT/i as well as
+ * annotations of type iTC/i.
+ *
+ * liThe change to iT/i is behaviorally compatible with respect
+ * to the {@code get[Declared]Annotation(ClassT)} (called with an
+ * argument of iT/i or iTC/i) and {@code
+ * get[Declared]Annotations()} methods because the results of the
+ * methods will not change due to iTC/i becoming the containing
+ * annotation type for iT/i.
+ *
+ * liThe change to iT/i is emnot/em behaviorally compatible
+ * with respect to the {@code
+ * get[Declared]AnnotationsByType(ClassT)} methods, because those
+ * methods will now recognize an annotation of type iTC/i as a
+ * container annotation and look through it to expose annotations of
+ * type iT/i.
+ *
+ * /ul
+ *
+ * /ul
+ *
  * pIf an annotation returned by a method in this interface contains
  * (directly or indirectly) a {@link Class}-valued member referring to
  * a class that is not accessible in this VM, attempting to read the class



hg: jdk8/tl/jdk: 8028816: Add value-type notice to Optional* classes

2013-12-03 Thread mike . duigou
Changeset: 2aae624bb833
Author:briangoetz
Date:  2013-12-03 21:22 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2aae624bb833

8028816: Add value-type notice to Optional* classes
Reviewed-by: mduigou, smarks
Contributed-by: bitterf...@gmail.com

+ src/share/classes/java/lang/doc-files/ValueBased.html
! src/share/classes/java/util/Optional.java
! src/share/classes/java/util/OptionalDouble.java
! src/share/classes/java/util/OptionalInt.java
! src/share/classes/java/util/OptionalLong.java



Initial with JDK-7168267

2013-12-03 Thread Tristan Yan
Hi Stuart

I am working on https://bugs.openjdk.java.net/browse/JDK-7168267. This bug is 
asking performance improvement for RMI test. Because this would involve 
different RMI tests. I'd like to use this cr as an umbrella bug, create sub-cr 
for different test. Then I can make progress on sub-cr. Please let me know your 
opinion on this.

Thank you.

 

Tristan Yan(Haibo Yan)

 


hg: jdk8/tl/langtools: 3 new changesets

2013-12-03 Thread lana . steuck
Changeset: 1f6ffcd56363
Author:cl
Date:  2013-11-28 08:24 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/1f6ffcd56363

Added tag jdk8-b118 for changeset 4fd6a7ff8c06

! .hgtags

Changeset: 43a80d75d06e
Author:lana
Date:  2013-12-03 10:47 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/43a80d75d06e

Merge


Changeset: 1b69023743be
Author:lana
Date:  2013-12-03 23:10 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/1b69023743be

Merge




hg: jdk8/tl/corba: 2 new changesets

2013-12-03 Thread lana . steuck
Changeset: 5029f982dfae
Author:cl
Date:  2013-11-28 08:22 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/corba/rev/5029f982dfae

Added tag jdk8-b118 for changeset d6820a414f18

! .hgtags

Changeset: 379fc7609beb
Author:lana
Date:  2013-12-03 10:46 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/corba/rev/379fc7609beb

Merge




hg: jdk8/tl: 2 new changesets

2013-12-03 Thread lana . steuck
Changeset: 06d512d44c31
Author:cl
Date:  2013-11-28 08:22 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/06d512d44c31

Added tag jdk8-b118 for changeset 0a6db1aac998

! .hgtags

Changeset: 9e90215673be
Author:lana
Date:  2013-12-03 10:46 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/9e90215673be

Merge