Review Request: JDK-8170633 backslashes in, gensrc/module-info.java comments need escaping

[Bradford Wetmore]

> The build tool generating module-info.java includes the path name of
> the source files in the comment for diagnosis where backslash
> character needs escaping.  This patch prints URI rather than file
> path.

As submitter and having tested the build patch, looks good to me.

Brad

Re: Review Request: JDK-8170633 backslashes in gensrc/module-info.java comments need escaping

[Alan Bateman]

On 02/12/2016 17:39, Mandy Chung wrote:

The build tool generating module-info.java includes the path name of the source 
files in the comment for diagnosis where backslash character needs escaping.  
This patch prints URI rather than file path.



Putting the URI in the comment looks fine.

-Alan

Re: Review Request: JDK-8170633 backslashes in gensrc/module-info.java comments need escaping

[Paul Sandoz]

> On 2 Dec 2016, at 09:39, Mandy Chung  wrote:
> 
> The build tool generating module-info.java includes the path name of the 
> source files in the comment for diagnosis where backslash character needs 
> escaping.  This patch prints URI rather than file path.
> 

+1

Paul.

> Mandy
> 
> diff --git a/make/src/classes/build/tools/module/GenModuleInfoSource.java 
> b/make/src/classes/build/tools/module/GenModuleInfoSource.java
> --- a/make/src/classes/build/tools/module/GenModuleInfoSource.java
> +++ b/make/src/classes/build/tools/module/GenModuleInfoSource.java
> @@ -146,9 +146,10 @@
> for (String l : lines) {
> writer.println(l);
> if (l.trim().startsWith("module ")) {
> -writer.format("// source file: %s%n", sourceFile);
> +// print URI rather than file path to avoid escape
> +writer.format("// source file: %s%n", 
> sourceFile.toUri());
> for (Path file: extraFiles) {
> -writer.format("//  %s%n", file);
> +writer.format("//  %s%n", 
> file.toUri());
> }
> break;
> }
> 

Review Request: JDK-8170633 backslashes in gensrc/module-info.java comments need escaping

[Mandy Chung]

The build tool generating module-info.java includes the path name of the source 
files in the comment for diagnosis where backslash character needs escaping.  
This patch prints URI rather than file path.

Mandy

diff --git a/make/src/classes/build/tools/module/GenModuleInfoSource.java 
b/make/src/classes/build/tools/module/GenModuleInfoSource.java
--- a/make/src/classes/build/tools/module/GenModuleInfoSource.java
+++ b/make/src/classes/build/tools/module/GenModuleInfoSource.java
@@ -146,9 +146,10 @@
 for (String l : lines) {
 writer.println(l);
 if (l.trim().startsWith("module ")) {
-writer.format("// source file: %s%n", sourceFile);
+// print URI rather than file path to avoid escape
+writer.format("// source file: %s%n", 
sourceFile.toUri());
 for (Path file: extraFiles) {
-writer.format("//  %s%n", file);
+writer.format("//  %s%n", 
file.toUri());
 }
 break;
 }

RE: [NEW BUG]: Reduce allocations in sun.reflect.annotation.AnnotationInvocationHandler.invoke()

[Christoph Dreis]

Hey,

>>> doing a bit of digging it appears a similar improvement - along with a
>>> lot of other things - was suggested and filed a few years back:
>>> https://bugs.openjdk.java.net/browse/JDK-8029019
>> Really sorry for missing that!
>>

If https://bugs.openjdk.java.net/browse/JDK-8029019 is decided to be solved
I'd like to add the following addition to that:

= PATCH ===
# User Christoph Dreis 
Avoid allocations in AnnotationType constructor coming from
Method.getParameterTypes()

diff --git
a/src/java.base/share/classes/sun/reflect/annotation/AnnotationType.java
b/src/java.base/share/classes/sun/reflect/annotation/AnnotationType.java
--- a/src/java.base/share/classes/sun/reflect/annotation/AnnotationType.java
+++ b/src/java.base/share/classes/sun/reflect/annotation/AnnotationType.java
@@ -121,7 +121,7 @@
 if (Modifier.isPublic(method.getModifiers()) &&
 Modifier.isAbstract(method.getModifiers()) &&
 !method.isSynthetic()) {
-if (method.getParameterTypes().length != 0) {
+if (method.getParameterCount() != 0) {
 throw new IllegalArgumentException(method + " has
params");
 }
 String name = method.getName();

Re: Review Request: JDK-8170660 RMI regression test failures due to missing @build TestLibrary

[Lance Andersen]

+1

> On Dec 2, 2016, at 12:27 PM, Mandy Chung  wrote:
> 
> @build TestLibrary in ContextWithNullProperties test was inadvertently 
> removed from JDK-8169231 [1].  This patch puts it back.
> 
> 
> diff --git 
> a/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
>  
> b/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
> --- 
> a/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
> +++ 
> b/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
> @@ -28,6 +28,7 @@
>  * @modules jdk.naming.rmi/com.sun.jndi.rmi.registry java.rmi/sun.rmi.registry
>  * java.rmi/sun.rmi.server java.rmi/sun.rmi.transport 
> java.rmi/sun.rmi.transport.tcp
>  * @library ../../../../../../java/rmi/testlibrary
> + * @build TestLibrary
>  * @compile --add-modules jdk.naming.rmi ContextWithNullProperties.java
>  * @run main ContextWithNullProperties
>  */
> 
> Mandy
> [1] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/7ee327a10059

 
  

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

Review Request: JDK-8170660 RMI regression test failures due to missing @build TestLibrary

[Mandy Chung]

@build TestLibrary in ContextWithNullProperties test was inadvertently removed 
from JDK-8169231 [1].  This patch puts it back.


diff --git 
a/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java 
b/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
--- 
a/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
+++ 
b/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
@@ -28,6 +28,7 @@
  * @modules jdk.naming.rmi/com.sun.jndi.rmi.registry java.rmi/sun.rmi.registry
  * java.rmi/sun.rmi.server java.rmi/sun.rmi.transport 
java.rmi/sun.rmi.transport.tcp
  * @library ../../../../../../java/rmi/testlibrary
+ * @build TestLibrary
  * @compile --add-modules jdk.naming.rmi ContextWithNullProperties.java
  * @run main ContextWithNullProperties
  */

Mandy
[1] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/7ee327a10059

Re: RFR: JDK-8075577: java.time does not support HOST provider

[Roger Riggs]

Hi Rachna,

Ok,  looks fine.

Thanks, Roger


On 12/1/2016 8:20 AM, Rachna Goel wrote:


Hi Roger,

Thanks for the review.   Updated patch is : 
http://cr.openjdk.java.net/~rgoel/JDK-8075577/webrev.02/


Please find some of my comments inlined.


On 30/11/16 10:17 PM, Roger Riggs wrote:

Hi Rachna,

macosx//HostLocaleProviderAdapterImpl.java:

 - line 172-177,  might be a good place to use the new 
ConcurrentMap.computeIfAbsent

I could have replaced those lines with :
dateFormatPatternsMap.computeIfAbsent(locale,
k -> new SoftReference<>(new 
AtomicReferenceArray<>(5 * 5)));


But, there are two check made on line no 173. (ref == null ) which 
will be checked by computeIfAbsent, But about second 
(dateFormatPatterns = ref.get()) == null)

will not be checked,  if those lines replaced.


 - line 197: you could pre-size the StringBuilder since the target 
length can be estimated

   (unless they are all less than the default 16)

pre-sized it with initial " jrepattern" string.


macosx && windows//HostLocaleProviderAdapterImpl.java
 -  toJavaTimeDateTimePattern()  - is there a way to avoid having two 
copies of this function?

Perhaps as a static method in JavaTimeDateTimePatternImpl.java.

There seems to be no way to avoid having two copies.
Implementations of "HostLocaleProviderAdapterImpl" for different HOST 
Environments are loaded at run time by HOSTLocaleProviderAdapter.
JavaTimeDateTimePatternImpl is an implementation of 
"JavaTimeDateTimePatternProvider" for JRE LocaleProviderAdapter.




The noreg-hard label on issue indicates testing is difficult and 
specific to OS and host
configuration but it also seems unusual to have this much code and 
not have a regression test.


I am in touch with I18n SQE on writing tests for HOST Providers. But 
testing scope, Golden data are yet to be discussed.
If Masayoshi is satisfied and you have tested it in the target 
configuration then
perhaps it is not worthwhile to invest in a special case regression 
test.


Thanks, Roger

On 11/30/2016 4:39 AM, Masayoshi Okutsu wrote:

Looks good to me.

Masayoshi


On 11/22/2016 6:30 PM, Rachna Goel wrote:

Hi,

Please review fix for JDK-8075577.

Bug : https://bugs.openjdk.java.net/browse/JDK-8075577

webrev : http://cr.openjdk.java.net/~rgoel/JDK-8075577/webrev.01/

Fix is to introduce new private spi 
"sun.text.spi.JavaTimeDateTimePatternProvider.java"  to retrieve 
LocaleProvider specific Date/Time Patterns for "java.time" .


Thanks,
Rachna

Re: On what issues could I help clean up for JDK 9

[dalibor topic]

Community-wide 'starter issues' are a better idea in theory, then they 
are in practice.


Typically the theory behind them is to mark some low priority issues for 
someone else to fix. But in practice, not all low priority fixes are 
welcome at all times. See 
http://mail.openjdk.java.net/pipermail/adoption-discuss/2016-August/001422.html 
for a longer explanation why that's the case.


In practice, it's a better idea to focus new contributors' attention not 
on what they can do for the large projects with schedules, processes and 
all that good, complicated stuff that enables releases to happen, like 
JDK 9 or JDK 8 Updates, but on what they can do in the projects that are 
in a more exploratory phase, such as Valhalla. Beside exploration of new 
ideas being more fun for new contributors, they are also often eager for 
the kind of feedback on the new ideas and designs, that comes from 
playing with the new toys and often enough, breaking them in interesting 
ways.


cheers,
dalibor topic

On 02.12.2016 13:53, Patrick Reinhart wrote:

What was the outcome of that discussion?

I seem to miss that one. My question comes from the past presentation I gave 
about contributing to the OpenJDK. And one of the main things was not only to 
do some local hacking but instead try to solve some small issues, that else 
would not be fixed because of other more important things.

-Patrick


Am 02.12.2016 um 11:45 schrieb Martijn Verburg :

There's no JBS query that I know of (I think in the distant past we discussed 
adding a low hanging fruit 'Duke' tag?).

Cheers,
Martijn




--
 Dalibor Topic | Principal Product Manager
Phone: +494089091214  | Mobile: +491737185961


ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

 Oracle is committed to developing
practices and products that help protect the environment

Re: RFR: 8170595: Optimize Class.isAnonymousClass

[Claes Redestad]

Hi,

On 2016-12-01 22:25, Claes Redestad wrote:

On 12/01/2016 10:21 AM, Mandy Chung wrote:
On Dec 1, 2016, at 9:52 AM, Claes Redestad 
 wrote:


Hi,

good suggestion, this tidies up a bit while not affecting score:

http://cr.openjdk.java.net/~redestad/8170595/webrev.02
I like this better.  It may be useful to add a private isTopLevel 
Class() for  getSimpleBinaryName to call that will benefit 
getSimpleName and isMemberClass?


Good idea, but this seems like an independent optimization we should 
do as a separate follow-up, no?


Found that both isMemberClass and isLocalClass can be optimized in a 
vein very similar to isAnonymousClass. isTopLevelClass helps
getSimpleBinaryName a bit, and makes sense to implement for 
completeness. After deeper testing it's clear that one of the
costlier operations in this code is getDeclaringClass0, which may spend 
plenty of time (~30ns/op for String.class) in the VM walking the

list of inner classes to find the enclosing class - if any.

A very small improvement can also be attained by restructuring code so 
that we don't instantiate the EnclosingMethodInfo
unless it's needed, but keep the checking to keep semantically identical 
behavior:


http://cr.openjdk.java.net/~redestad/8170595/webrev.03/

This brings significant improvements to some variants:

Baseline:
Benchmark   Mode  CntScoreError  Units
Clazz.isAnonymousClass_Anon avgt   25  211.010 ± 22.974  ns/op
Clazz.isAnonymousClass_Regular  avgt   25  132.198 ± 23.817  ns/op
Clazz.isMemberClass_Anonavgt   25  336.149 ± 43.215  ns/op
Clazz.isMemberClass_Regular avgt   25   92.502 ±  9.804  ns/op
Clazz.isLocalClass_Anon avgt   25  326.158 ± 34.051  ns/op
Clazz.isLocalClass_Regular  avgt   25   32.086 ±  2.750  ns/op

Patch:
Benchmark   Mode  CntScoreError  Units
Clazz.isAnonymousClass_Anon avgt   25  174.526 ± 16.292  ns/op
Clazz.isAnonymousClass_Regular  avgt   25   33.136 ±  2.636  ns/op
Clazz.isMemberClass_Anonavgt   25  115.029 ±  9.464  ns/op
Clazz.isMemberClass_Regular avgt   25   85.706 ±  3.984  ns/op
Clazz.isLocalClass_Anon avgt   25  177.411 ± 22.927  ns/op
Clazz.isLocalClass_Regular  avgt   25   31.940 ±  2.346  ns/op

Since the changes are all similar but still not too expansive, I'm 
leaning towards just keeping this

change in one bug.

Thanks!

potential error in fdlibm asin

[Lindenmaier, Goetz]

[resent after subscribing core-libs-dev]
Hi, 

I'm looking at some strange code in the asin implementation:

Basically the code in
http://hg.openjdk.java.net/jdk9/dev/jdk/file/bc6c31fd98cf/src/java.base/share/native/libfdlibm/e_asin.c
after line 101 is indented wrong.

But I think in truth the "else" statement should be deleted.

"t = x*x" is only executed if "if (ix<0x3e40)" is wrong.
 
If "if (ix<0x3e40)" is true and "if(huge+x>one)" is wrong, 
t=0 is used for statement "p = t*(pS0+t*(pS1+t*(pS2+t*(pS3+t*(pS4+t*pS5"
resulting in p=0.
 
In fdlibm of netlib the initialization t=0 is missing, but it has the same 
strange
indentation.
http://www.netlib.org/fdlibm/e_asin.c
 
I found one copy of this code in the internet where the "else" is removed,
but it's not completely the same:
https://github.com/jerryscript-project/jerryscript/blob/master/jerry-libm/asin.c
 
Does anybody have the knowledge to tell me what's wrong?  
Does anybody know how to contribute a fix to netlib fdlibm?
 
Thanks for your help,
  Goetz.

potential error in fdlibm asin

[Lindenmaier, Goetz]

Hi,

I'm looking at some strange code in the asin implementation:

Basically the code in
http://hg.openjdk.java.net/jdk9/dev/jdk/file/bc6c31fd98cf/src/java.base/share/native/libfdlibm/e_asin.c
after line 101 is indented wrong.

But I think in truth the "else" statement should be deleted.


"t = x*x" is only executed if "if (ix<0x3e40)" is wrong.



If "if (ix<0x3e40)" is true and "if(huge+x>one)" is wrong,

t=0 is used for statement "p = t*(pS0+t*(pS1+t*(pS2+t*(pS3+t*(pS4+t*pS5"

resulting in p=0.



In fdlibm of netlib the initialization t=0 is missing, but it has the same 
strange

indentation.

http://www.netlib.org/fdlibm/e_asin.c



I found one copy of this code in the internet where the "else" is removed,

but it's not completely the same:

https://github.com/jerryscript-project/jerryscript/blob/master/jerry-libm/asin.c



Does anybody have the knowledge to tell me what's wrong?

Does anybody know how to contribute a fix to netlib fdlibm?



Thanks for your help,

  Goetz.

Re: On what issues could I help clean up for JDK 9

[Claes Redestad]

Hi,

I'd suggest start looking at requests for code cleanup, performance 
optimizations or similar that doesn't
alter any public semantics (such as adding public APIs or subtly 
changing the behavior of existing

ones).

A few simple JBS searches lists at least some things that look tempting:

"Cleanup":
https://bugs.openjdk.java.net/issues/?jql=text%20~%20%22Cleanup%22%20%26%26%20status%20in%20(Open)%20and%20assignee%20is%20EMPTY

"Optimize":
https://bugs.openjdk.java.net/issues/?jql=text%20~%20%22Cleanup%22%20%26%26%20status%20in%20(Open)%20and%20assignee%20is%20EMPTY

Some, mostly hotspot engineers, use the "starter" label to mark issues 
thought to be good candidates for someone new to the project (this 
doesn't necessarily

mean "easy"):
https://bugs.openjdk.java.net/issues/?jql=labels%20in%20(%22starter%22)%20%26%26%20status%20in%20(Open)

Hope this helps!

/Claes

On 2016-12-01 22:58, Patrick Reinhart wrote:

Hi there,

I wanted to ask if there is a simple JBS query to find small  clean up parts to 
help with?

Seems to me a good starting point for some „hacking-on-the-OpenJDK-sessions“


-Patrick

Re: On what issues could I help clean up for JDK 9

[Patrick Reinhart]

What was the outcome of that discussion? 

I seem to miss that one. My question comes from the past presentation I gave 
about contributing to the OpenJDK. And one of the main things was not only to 
do some local hacking but instead try to solve some small issues, that else 
would not be fixed because of other more important things.

-Patrick

> Am 02.12.2016 um 11:45 schrieb Martijn Verburg :
> 
> There's no JBS query that I know of (I think in the distant past we discussed 
> adding a low hanging fruit 'Duke' tag?).
> 
> Cheers,
> Martijn

Re: RFR: JDK-8170629 Remove code duplication in test makefiles

[Erik Joelsson]

Looks good.

/Erik

On 2016-12-02 10:24, Magnus Ihse Bursie wrote:
There is a lot of common code that has been duplicated in 
*/test/Makefile. For jdk, hotspot, jaxp and nashorn, most of the code 
in the corresponding files are identical. (The odd-man-out is 
langtools which is quite different.)


These files should be unified to share a single code base, to 
facilitate further improvements to the test makefiles.


The intent of this bug is a pure refactoring. The duplication should 
go, but all functionality should remain exactly the same. This will 
leave room for some future improvements to the code, but sets a clear 
limit for this first step. The consolidated code base thus contains a 
fair amount of if-blocks, but I hope to be able to address most of 
these later on, by adjusting the individual component to adhere more 
to the standard behavior.


To minimize the amount of ifdefs in the shared code, I allowed for a 
few changes in behavior. I do not believe these will cause any changes 
in the real world, and to the extent that they do, they could be 
considered bug fixes.


Behavior that have changed due to unification:
* jaxp now defaults ABS_JDK_IMAGE to images/jdk instead of j2sdk.
* jaxp now sets TEST_SELECTION to $(TESTDIRS) if it exists.
* jaxp and hotspot now get additional option e:JDK8_HOME=${JT_JAVA}
* hotspot now sets JAVA_VM_ARGS to $(JPRT_PRODUCT_VM_ARGS) if it exists.
* hotspot now sets JAVA_ARGS to $(JPRT_PRODUCT_ARGS) if it exists.

Bug: https://bugs.openjdk.java.net/browse/JDK-8170629
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8170629-remove-test-makefile-duplication/webrev.01


/Magnus

Re: On what issues could I help clean up for JDK 9

[Martijn Verburg]

There's no JBS query that I know of (I think in the distant past we
discussed adding a low hanging fruit 'Duke' tag?).

Cheers,
Martijn

On 1 December 2016 at 21:58, Patrick Reinhart  wrote:

> Hi there,
>
> I wanted to ask if there is a simple JBS query to find small  clean up
> parts to help with?
>
> Seems to me a good starting point for some „hacking-on-the-OpenJDK-
> sessions“
>
>
> -Patrick

Re: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192

[Daniel Fuchs]

On 01/12/16 17:25, Joe Wang wrote:

Thanks Christoph, Daniel! I've updated the test, removing the comments.

As for 80 chars line limit, or slightly longer than that is fine with
me. Given our codebase, it would be an enormous undertaking. I'm
therefore fine with taking care of only the extremely long ones or any
that impedes the reviews, that is, side-by-side view.


+1

best regards,

-- daniel



Best,
Joe

On 12/1/16, 2:25 AM, Daniel Fuchs wrote:

Hi Joe,

I agree with Christoph's comments below.

+1

best regards,

-- daniel

On 01/12/16 07:40, Langer, Christoph wrote:

Hi Joe,

to me this looks good.

Did you already remove the cleanups from
http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see
a lot of them any more...

A few minor points:

It seems you still have left some debugging code in
test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java:


59 while (xmlStreamReader.hasNext()) {
  60 int event = xmlStreamReader.next();
  61 if (event == XMLStreamConstants.START_ELEMENT) {
  62 if
(xmlStreamReader.getLocalName().equals("body"))// && bMessage)

-> remove the && bMessage)

  63 {
  64 String elementText =
xmlStreamReader.getElementText();
  65 //System.out.println("elementText=" +
elementText + "EndOfElementText");

-> the commented System.out.println statement should be removed at
all, I suggest

  66 // fail if elementText contains ""
  67
Assert.assertTrue(!elementText.contains(""), "Fail:
elementText contains ");
  68 }
  69 }
  70 }

Other than that I'm wondering if the 80 chars line limit shall be
held which is not completely true in StreamReaderTest.java. But I
know how difficult that is, while keeping the code still readable.
Especially with the long speaking Class and method names ;-)

Best regards
Christoph



-Original Message-
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net]
On Behalf
Of Joe Wang
Sent: Mittwoch, 30. November 2016 22:21
To: core-libs-dev@openjdk.java.net
Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
corrupt
content when size of element is > 8192

Hi,

Please review an one-line fix and a bunch of cleanups.

The reported issue was caused by a missed setting when the
XMLStreamReader initializes the XML 1.1 scanner, so while the changeset
involved 350 lines, the fix is really just the following:

XMLStreamReaderImpl.java:
@@ -605,11 +604,12 @@
...
+fEntityScanner.registerListener(fScanner);


All other changes are cleanups, warnings. And BTW, warnings in the jaxp
repo have come down from 5230 in 2015 to 3265, a result of a bit of
cleanups here and there when we touch those classes. Still a long
way to
go, and it shows we may need to have a few dedicated patches.

JBS: https://bugs.openjdk.java.net/browse/JDK-8167340
webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/

Thanks,
Joe

Re: RFR of JDK-8170644: java/rmi/registry/interfaceHash/InterfaceHash.java failed intermittently with "Port already in use"

[Daniel Fuchs]

Hi Hamlin,

Looks good to me!

best regards,

-- daniel

On 02/12/16 08:25, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8170644
webrev: http://cr.openjdk.java.net/~mli/8170644/webrev.00/

Thank you
-Hamlin

Re: RFR: 8169495: Add a method to set an Authenticator on a HttpURLConnection.

[Chris Hegarty]

On 10/11/16 15:12, Daniel Fuchs wrote:

Hi,

Please find below a patch for:

https://bugs.openjdk.java.net/browse/JDK-8169495
8169495: Add a method to set an Authenticator on a HttpURLConnection.


Thank you Daniel. This addresses a long standing request to
have a per-instance Authenticator.


webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8169495/webrev.00


We exchanged a few mails off-line, and I am happy with what
is in the latest webrev.

-Chris.


The public API changes are in java.net.HttpURLConnection and
java.net.Authenticator. For backward compatibility reason the
new HttpURLConnection::setAuthenticator method is not abstract,
but is instead implemented to throw UOE unless overridden.

Again for compatibility reasons, if no authenticator is explicitly
supplied to the connection then the behavior is unchanged: the
default authenticator will be invoked, if needed, at the time
credentials are requested through the HTTP protocol.

Here is the description of the new HttpURLConnection::setAuthenticator
method:

 /**
+ * Supplies an {@link java.net.Authenticator Authenticator} to be used
+ * when authentication is requested through the HTTP protocol for
+ * this {@code HttpURLConnection}.
+ * If no authenticator is supplied, the
+ * {@linkplain Authenticator#setDefault(java.net.Authenticator)
default
+ * authenticator} will be used.
+ *
+ * @implNote The default behavior of this method is to throw
+ *   {@link UnsupportedOperationException}. Concrete
+ *   implementations of {@code HttpURLConnection}
+ *   which support supplying an {@code Authenticator} for a
+ *   specific {@code HttpURLConnection} instance should
+ *   override this method to implement a different behavior.
+ *
+ * @implSpec Depending on authentication schemes, an implementation
+ *   may or may not need to use the provided authenticator
+ *   to obtain a password. For instance, an implementation
that
+ *   relies on third-party security libraries may still
invoke the
+ *   default authenticator if these libraries are configured
+ *   to do so.
+ *   Likewise, an implementation that supports transparent
+ *   NTLM authentication may let the system attempt
+ *   to connect using the system user credentials first,
+ *   before invoking the provided authenticator.
+ *   
+ *   However, if an authenticator is specifically provided,
+ *   then the underlying connection may only be reused for
+ *   {@code HttpURLConnection} instances which share the same
+ *   {@code Authenticator} instance, and authentication
information,
+ *   if cached, may only be reused for an {@code
HttpURLConnection}
+ *   sharing that same {@code Authenticator}.
+ *
+ * @param auth The {@code Authenticator} that should be used by this
+ *   {@code HttpURLConnection}.
+ *
+ * @throws  UnsupportedOperationException if setting an
Authenticator is
+ *  not supported by the underlying implementation.
+ * @throws  IllegalStateException if URLConnection is already
connected.
+ * @throws  NullPointerException if the supplied {@code auth} is
{@code null}.
+ * @since 9
+ */
+public void setAuthenticator(Authenticator auth) {
+throw new UnsupportedOperationException("Supplying an
authenticator"
++ " is not supported by " + this.getClass());
+}


best regards,

-- daniel

Re: RFR of JDK-8078587: java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java fails intermittently with Port already in use

[Chris Hegarty]

On 02/12/16 09:39, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8078587
webrev: http://cr.openjdk.java.net/~mli/8078587/webrev.00/


Reviewed.

-Chris.


Thank you
-Hamlin


diff -r 08f81d321087
test/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java

---
a/test/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java
Fri Dec 02 01:11:33 2016 -0800
+++
b/test/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java
Fri Dec 02 01:35:59 2016 -0800
@@ -43,7 +43,6 @@
  *  java.rmi/sun.rmi.transport.tcp
  * @build TestLibrary JavaVM LeaseCheckInterval_Stub SelfTerminator
  * @run main/othervm LeaseCheckInterval
- * @key intermittent
  */

 import java.rmi.Remote;
@@ -88,9 +87,8 @@
 UnicastRemoteObject.exportObject(obj);
 System.err.println("exported remote object");

-int registryPort = TestLibrary.getUnusedRandomPort();
-Registry localRegistry =
-LocateRegistry.createRegistry(registryPort);
+Registry localRegistry =
TestLibrary.createRegistryOnEphemeralPort();
+int registryPort = TestLibrary.getRegistryPort(localRegistry);
 System.err.println("created local registry");

 localRegistry.bind(BINDING, obj);

RFR of JDK-8078587: java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java fails intermittently with Port already in use

[Hamlin Li]

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8078587
webrev: http://cr.openjdk.java.net/~mli/8078587/webrev.00/

Thank you
-Hamlin


diff -r 08f81d321087 
test/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java
--- 
a/test/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java 
Fri Dec 02 01:11:33 2016 -0800
+++ 
b/test/java/rmi/server/Unreferenced/leaseCheckInterval/LeaseCheckInterval.java 
Fri Dec 02 01:35:59 2016 -0800

@@ -43,7 +43,6 @@
  *  java.rmi/sun.rmi.transport.tcp
  * @build TestLibrary JavaVM LeaseCheckInterval_Stub SelfTerminator
  * @run main/othervm LeaseCheckInterval
- * @key intermittent
  */

 import java.rmi.Remote;
@@ -88,9 +87,8 @@
 UnicastRemoteObject.exportObject(obj);
 System.err.println("exported remote object");

-int registryPort = TestLibrary.getUnusedRandomPort();
-Registry localRegistry =
-LocateRegistry.createRegistry(registryPort);
+Registry localRegistry = 
TestLibrary.createRegistryOnEphemeralPort();

+int registryPort = TestLibrary.getRegistryPort(localRegistry);
 System.err.println("created local registry");

 localRegistry.bind(BINDING, obj);

RFR: JDK-8170629 Remove code duplication in test makefiles

[Magnus Ihse Bursie]

There is a lot of common code that has been duplicated in 
*/test/Makefile. For jdk, hotspot, jaxp and nashorn, most of the code in 
the corresponding files are identical. (The odd-man-out is langtools 
which is quite different.)


These files should be unified to share a single code base, to facilitate 
further improvements to the test makefiles.


The intent of this bug is a pure refactoring. The duplication should go, 
but all functionality should remain exactly the same. This will leave 
room for some future improvements to the code, but sets a clear limit 
for this first step. The consolidated code base thus contains a fair 
amount of if-blocks, but I hope to be able to address most of these 
later on, by adjusting the individual component to adhere more to the 
standard behavior.


To minimize the amount of ifdefs in the shared code, I allowed for a few 
changes in behavior. I do not believe these will cause any changes in 
the real world, and to the extent that they do, they could be considered 
bug fixes.


Behavior that have changed due to unification:
* jaxp now defaults ABS_JDK_IMAGE to images/jdk instead of j2sdk.
* jaxp now sets TEST_SELECTION to $(TESTDIRS) if it exists.
* jaxp and hotspot now get additional option e:JDK8_HOME=${JT_JAVA}
* hotspot now sets JAVA_VM_ARGS to $(JPRT_PRODUCT_VM_ARGS) if it exists.
* hotspot now sets JAVA_ARGS to $(JPRT_PRODUCT_ARGS) if it exists.

Bug: https://bugs.openjdk.java.net/browse/JDK-8170629
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8170629-remove-test-makefile-duplication/webrev.01


/Magnus

Re: RFR of JDK-8080550: java/rmi/server/useCustomRef/UseCustomRef.java failed with java.net.BindException intermittently

[Chris Hegarty]

On 02/12/16 09:06, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8080550
webrev: http://cr.openjdk.java.net/~mli/8080550/


Looks good Hamlin.

-Chris.



Thank you
-Hamlin


diff -r 99dd72e05060 test/java/rmi/server/useCustomRef/UseCustomRef.java
--- a/test/java/rmi/server/useCustomRef/UseCustomRef.javaFri Dec 02
00:47:00 2016 -0800
+++ b/test/java/rmi/server/useCustomRef/UseCustomRef.javaFri Dec 02
01:05:36 2016 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1998, 2016, 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
@@ -86,7 +86,7 @@

 System.err.println("creating Registry...");

-registry = TestLibrary.createRegistryOnUnusedPort();
+registry = TestLibrary.createRegistryOnEphemeralPort();
 int port = TestLibrary.getRegistryPort(registry);
 /*
  * create object with custom ref and bind in registry

RFR of JDK-8080550: java/rmi/server/useCustomRef/UseCustomRef.java failed with java.net.BindException intermittently

[Hamlin Li]

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8080550
webrev: http://cr.openjdk.java.net/~mli/8080550/

Thank you
-Hamlin


diff -r 99dd72e05060 test/java/rmi/server/useCustomRef/UseCustomRef.java
--- a/test/java/rmi/server/useCustomRef/UseCustomRef.javaFri Dec 02 
00:47:00 2016 -0800
+++ b/test/java/rmi/server/useCustomRef/UseCustomRef.javaFri Dec 02 
01:05:36 2016 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1998, 2016, 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
@@ -86,7 +86,7 @@

 System.err.println("creating Registry...");

-registry = TestLibrary.createRegistryOnUnusedPort();
+registry = TestLibrary.createRegistryOnEphemeralPort();
 int port = TestLibrary.getRegistryPort(registry);
 /*
  * create object with custom ref and bind in registry

Re: RFR of JDK-8153916: com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java failed with BindException

[Chris Hegarty]

On 02/12/16 08:42, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8153916
webrev: http://cr.openjdk.java.net/~mli/8153916/webrev.00/


Looks good. Thanks Hamlin.

-Chris.


Thank you
-Hamlin

diff -r 35c87712682f
test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java

---
a/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
Fri Dec 02 10:56:46 2016 +0800
+++
b/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
Fri Dec 02 00:39:44 2016 -0800
@@ -37,7 +37,7 @@

 public class ContextWithNullProperties {
 public static void main(String[] args) throws Exception {
-Registry registry = TestLibrary.createRegistryOnUnusedPort();
+Registry registry = TestLibrary.createRegistryOnEphemeralPort();
 int registryPort = TestLibrary.getRegistryPort(registry);
 System.out.println("Connecting to the default Registry...");
 // Connect to the default Registry.

RFR of JDK-8153916: com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java failed with BindException

[Hamlin Li]

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8153916
webrev: http://cr.openjdk.java.net/~mli/8153916/webrev.00/

Thank you
-Hamlin

diff -r 35c87712682f 
test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java
--- 
a/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java 
Fri Dec 02 10:56:46 2016 +0800
+++ 
b/test/com/sun/jndi/rmi/registry/RegistryContext/ContextWithNullProperties.java 
Fri Dec 02 00:39:44 2016 -0800

@@ -37,7 +37,7 @@

 public class ContextWithNullProperties {
 public static void main(String[] args) throws Exception {
-Registry registry = TestLibrary.createRegistryOnUnusedPort();
+Registry registry = TestLibrary.createRegistryOnEphemeralPort();
 int registryPort = TestLibrary.getRegistryPort(registry);
 System.out.println("Connecting to the default Registry...");
 // Connect to the default Registry.

RFR of JDK-8170644: java/rmi/registry/interfaceHash/InterfaceHash.java failed intermittently with "Port already in use"

[Hamlin Li]

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8170644
webrev: http://cr.openjdk.java.net/~mli/8170644/webrev.00/

Thank you
-Hamlin

RE: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192

[Langer, Christoph]

Hi Joe,

thanks, looks fine now.

In 
test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java, 
the brace in line 63 could still move one line up, but this really is 
nit-picking ;-)

Best regards
Christoph


> -Original Message-
> From: Joe Wang [mailto:huizhe.w...@oracle.com]
> Sent: Donnerstag, 1. Dezember 2016 18:25
> To: Daniel Fuchs 
> Cc: Langer, Christoph ; core-libs-
> d...@openjdk.java.net
> Subject: Re: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
> corrupt content when size of element is > 8192
> 
> Thanks Christoph, Daniel! I've updated the test, removing the comments.
> 
> As for 80 chars line limit, or slightly longer than that is fine with
> me. Given our codebase, it would be an enormous undertaking. I'm
> therefore fine with taking care of only the extremely long ones or any
> that impedes the reviews, that is, side-by-side view.
> 
> Best,
> Joe
> 
> On 12/1/16, 2:25 AM, Daniel Fuchs wrote:
> > Hi Joe,
> >
> > I agree with Christoph's comments below.
> >
> > +1
> >
> > best regards,
> >
> > -- daniel
> >
> > On 01/12/16 07:40, Langer, Christoph wrote:
> >> Hi Joe,
> >>
> >> to me this looks good.
> >>
> >> Did you already remove the cleanups from
> >> http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see
> >> a lot of them any more...
> >>
> >> A few minor points:
> >>
> >> It seems you still have left some debugging code in
> >>
> test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest
> .java:
> >>
> >> 59 while (xmlStreamReader.hasNext()) {
> >>   60 int event = xmlStreamReader.next();
> >>   61 if (event == XMLStreamConstants.START_ELEMENT) {
> >>   62 if
> >> (xmlStreamReader.getLocalName().equals("body"))// && bMessage)
> >>
> >> -> remove the && bMessage)
> >>
> >>   63 {
> >>   64 String elementText =
> >> xmlStreamReader.getElementText();
> >>   65 //System.out.println("elementText=" +
> >> elementText + "EndOfElementText");
> >>
> >> -> the commented System.out.println statement should be removed at
> >> all, I suggest
> >>
> >>   66 // fail if elementText contains ""
> >>   67
> >> Assert.assertTrue(!elementText.contains(""), "Fail:
> >> elementText contains ");
> >>   68 }
> >>   69 }
> >>   70 }
> >>
> >> Other than that I'm wondering if the 80 chars line limit shall be
> >> held which is not completely true in StreamReaderTest.java. But I
> >> know how difficult that is, while keeping the code still readable.
> >> Especially with the long speaking Class and method names ;-)
> >>
> >> Best regards
> >> Christoph
> >>
> >>
> >>> -Original Message-
> >>> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net]
> >>> On Behalf
> >>> Of Joe Wang
> >>> Sent: Mittwoch, 30. November 2016 22:21
> >>> To: core-libs-dev@openjdk.java.net
> >>> Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
> >>> corrupt
> >>> content when size of element is > 8192
> >>>
> >>> Hi,
> >>>
> >>> Please review an one-line fix and a bunch of cleanups.
> >>>
> >>> The reported issue was caused by a missed setting when the
> >>> XMLStreamReader initializes the XML 1.1 scanner, so while the changeset
> >>> involved 350 lines, the fix is really just the following:
> >>>
> >>> XMLStreamReaderImpl.java:
> >>> @@ -605,11 +604,12 @@
> >>> ...
> >>> +fEntityScanner.registerListener(fScanner);
> >>>
> >>>
> >>> All other changes are cleanups, warnings. And BTW, warnings in the jaxp
> >>> repo have come down from 5230 in 2015 to 3265, a result of a bit of
> >>> cleanups here and there when we touch those classes. Still a long
> >>> way to
> >>> go, and it shows we may need to have a few dedicated patches.
> >>>
> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8167340
> >>> webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/
> >>>
> >>> Thanks,
> >>> Joe
> >