date:20170124

RFR of JDK-8173326: Problem list java/rmi/registry/readTest/CodebaseTest.java on Windows

2017-01-24 Thread Hamlin Li


Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8173326

Thank you
-Hamlin


diff -r a468135ebe8e test/ProblemList.txt
--- a/test/ProblemList.txtTue Jan 24 18:41:36 2017 -0800
+++ b/test/ProblemList.txtTue Jan 24 23:08:59 2017 -0800
@@ -203,6 +203,8 @@

 sun/rmi/rmic/newrmic/equivalence/run.sh 8145980 generic-all

+java/rmi/registry/readTest/CodebaseTest.java 8173324 windows-all
+
 

 # jdk_security

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

2017-01-24 Thread Frank Yuan

Thank you, Christoph

Frank

> -Original Message-
> From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> Sent: Tuesday, January 24, 2017 6:30 PM
> To: Frank Yuan
> Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net
> Subject: RE: RFR (JAXP) 8169827: 
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> 
> Hi,
> 
> I submitted my fix under Bug https://bugs.openjdk.java.net/browse/JDK-8173261:
> http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/620d0c38581f
> 
> I also added a comment to https://bugs.openjdk.java.net/browse/JDK-8169827
> 
> Best regards
> Christoph
> 
> 
> > -Original Message-
> > From: Frank Yuan [mailto:frank.y...@oracle.com]
> > Sent: Dienstag, 24. Januar 2017 09:45
> > To: Langer, Christoph 
> > Cc: 'Daniel Fuchs' ; core-libs-dev@openjdk.java.net
> > Subject: RE: RFR (JAXP) 8169827:
> > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> >
> > > From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> > > To: Frank Yuan
> > > Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net
> > > Subject: RE: RFR (JAXP) 8169827:
> > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > >
> > > Hi Frank,
> > >
> > > thanks for your comments.
> > >
> > > I already thought it might be better to create a new bug for my repair 
> > > work -
> > as 8169827 actually is about the intermittent
> > failure which will
> > > perhaps not be solved with my changes. So I'll create a new item and 
> > > submit
> > my changes for that one.
> > >
> > > Furthermore, I would also support creating Java code to do the copy work
> > instead of using the shell script, though doing this is
> > rather out of
> > > scope for me, at the moment.
> >
> > Just a suggestion, please feel free to leave it:)
> >
> > >This should be done in a common library, e.g.
> > /test/javax/xml/jaxp/libs/jaxp/library. I also see that the same
> > > requirement exists for tests in the jdk repo - unfortunately there is no 
> > > shared
> > testing library with 'jdk'. So we'll probably end
> > up with some
> > > duplicated code :(
> > >
> > > By the way: In the jaxp test tree, there exists a lib
> > /test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff
> > seems to be
> > > obsolete for jaxp testing and/or significantly behind jdk's 
> > > test/lib/testlibrary. I
> > guess here's some room for
> > cleanup/modernization.
> > >
> >
> > Yes, testlibrary is a copy from jdk repo, there are a few JAXP tests using 
> > the
> > library, so we didn't keep it synced with JDK. In my
> > mind, we don't need spend too much effort on it unless it impacts JAXP 
> > tests.
> >
> > > As for the idea with the links - it sounds interesting, but it also adds 
> > > more
> > complexity. And maybe it is not such a good idea for
> > Windows
> > > systems...
> >
> > I didn't test it in Windows, please leave it to me. I will update to you 
> > and Joe
> > when I have the result. Thanks
> >
> > Frank
> >
> > >
> > > Best regards
> > > Christoph
> > >
> > > > -Original Message-
> > > > From: Frank Yuan [mailto:frank.y...@oracle.com]
> > > > Sent: Dienstag, 24. Januar 2017 08:59
> > > > To: Langer, Christoph ; 'Daniel Fuchs'
> > > > ; core-libs-dev@openjdk.java.net
> > > > Subject: RE: RFR (JAXP) 8169827:
> > > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > > >
> > > > Hi Christoph
> > > >
> > > > Many thanks for your effort!
> > > >
> > > > It's really better than the old version! However I have 2 comments 
> > > > although
> > I
> > > > am not a reviewer(as you often stated :))
> > > > 1. we could also write java code to copy/delete JDK.
> > > > 2. 8169827 is to track PropertiesTest failed in copying JDK 
> > > > intermittently. I
> > > > suggest to use another bug for your improvement
> > > > because I am not sure if this patch can improve the issue of 8169827.
> > > >
> > > > To Christoph and Daniel
> > > >
> > > > Btw, Christoph's patch inspired me. You know, PropertiesTest copies a 
> > > > JDK
> > in
> > > > order to isolate the file change of the JDK, actually
> > > > there are some other similar tests(to test some other JDK property or 
> > > > config
> > > > files) in JDK repo, they take the same way as well as
> > > > have similar code...
> > > > I have another idea for this test, that is we can only make symbolic 
> > > > link to
> > the
> > > > JDK files/directories except conf directory, create
> > > > a directory named with "conf" under the new jdk directory, and then make
> > > > symbolic link to the files/directories in conf to the real
> > > > jdk/conf except jaxp.properties. This will reduce the most of copying 
> > > > work
> > and
> > > > may reduce the risk of copying work. What's your
> > > > opinion?
> > > >
> > > > Thanks
> > > > Frank
> > > >
> > > > > -Original Message-
> > > > > From: Langer, Christoph

Re: Cleanup String.replace() occurences with single characters

2017-01-24 Thread Claes Redestad


Hi,

String::replace(CharSequence, CharSequence) does not use a regex, but
String::replace(char, char) can still be slightly more efficient,

As this is relatively new code that wasn't around when 8044461 was done
I'm sure we can get it cleaned up.

Thanks!

/Claes

On 2017-01-24 20:33, Christoph Dreis wrote:

Hey,



similar to https://bugs.openjdk.java.net/browse/JDK-8044461 I noticed two
(newly introduced) places where we could avoid the regex overhead

when replacing single chars.



I'd be happy if this is sponsored.



Cheers,

Christoph



=== PATCH 

# User Christoph Dreis < 
christoph.dr...@freenet.de>

Cleanup String.replace() occurences with single characters



diff --git
a/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java
b/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java

--- a/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java

+++ b/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java

@@ -39,7 +39,7 @@

 public static String getPackageName(String name) {

 int index = name.lastIndexOf('/');

 if (index != -1) {

-return name.substring(0, index).replace("/", ".");

+return name.substring(0, index).replace('/', '.');

 } else {

 return "";

 }

diff --git a/src/java.base/share/classes/jdk/internal/module/ModulePath.java
b/src/java.base/share/classes/jdk/internal/module/ModulePath.java

--- a/src/java.base/share/classes/jdk/internal/module/ModulePath.java

+++ b/src/java.base/share/classes/jdk/internal/module/ModulePath.java

@@ -495,7 +495,7 @@

 Attributes attrs = man.getMainAttributes();

 String mainClass = attrs.getValue(Attributes.Name.MAIN_CLASS);

 if (mainClass != null)

-builder.mainClass(mainClass.replace("/", "."));

+builder.mainClass(mainClass.replace('/', '.'));

 }



 return builder.build();

Re: RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-24 Thread Mandy Chung

> On Jan 24, 2017, at 10:20 AM, Henry Jen  wrote:
> 
> Hi,
> 
> Please review the webrev[1] that add support for JAVA_OPTIONS environment 
> variable. The bug[2] describes how JAVA_OPTIONS works.
> 
> [1] http://cr.openjdk.java.net/~henryjen/jdk9/8170832/4/webrev/

This looks quite good.  A couple of minor comments:

 503 // Must be after expansion so we can caught if main class 
specified @argfile

typo: s/caught/catch.  It’d be clear to simply say:
 // Check if Main class specified after argument checked.
 // This check must be done after expansion.

  42 #define JAVA_OPTIONS “JAVA_OPTIONS"

I think java.h is the appropriate file to declare this instead of jli_util.h.

emessages.h
  Would it be clearer to rename ARG_INFO to ARG_INFO_ENVVAR?

thanks
Mandy

Cleanup String.replace() occurences with single characters

2017-01-24 Thread Christoph Dreis

Hey,

 

similar to https://bugs.openjdk.java.net/browse/JDK-8044461 I noticed two
(newly introduced) places where we could avoid the regex overhead

when replacing single chars.

 

I'd be happy if this is sponsored.

 

Cheers,

Christoph

 

=== PATCH 

# User Christoph Dreis < 
christoph.dr...@freenet.de>

Cleanup String.replace() occurences with single characters

 

diff --git
a/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java
b/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java

--- a/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java

+++ b/src/java.base/share/classes/jdk/internal/loader/ResourceHelper.java

@@ -39,7 +39,7 @@

 public static String getPackageName(String name) {

 int index = name.lastIndexOf('/');

 if (index != -1) {

-return name.substring(0, index).replace("/", ".");

+return name.substring(0, index).replace('/', '.');

 } else {

 return "";

 }

diff --git a/src/java.base/share/classes/jdk/internal/module/ModulePath.java
b/src/java.base/share/classes/jdk/internal/module/ModulePath.java

--- a/src/java.base/share/classes/jdk/internal/module/ModulePath.java

+++ b/src/java.base/share/classes/jdk/internal/module/ModulePath.java

@@ -495,7 +495,7 @@

 Attributes attrs = man.getMainAttributes();

 String mainClass = attrs.getValue(Attributes.Name.MAIN_CLASS);

 if (mainClass != null)

-builder.mainClass(mainClass.replace("/", "."));

+builder.mainClass(mainClass.replace('/', '.'));

 }

 

 return builder.build();

RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

2017-01-24 Thread Henry Jen

Hi,

Please review the webrev[1] that add support for JAVA_OPTIONS environment 
variable. The bug[2] describes how JAVA_OPTIONS works.

[1] http://cr.openjdk.java.net/~henryjen/jdk9/8170832/4/webrev/
[2] https://bugs.openjdk.java.net/browse/JDK-8170832

Cheers,
Henry

Re: RFR: 8029019: (ann) Optimize annotation handling in core reflection

2017-01-24 Thread Chris Hegarty

Peter, 

> On 2017-01-17 15:10, Peter Levart wrote:
>> Hi,
>> 
>> This is a preview of a patch that addresses the following issue:
>> 
>>https://bugs.openjdk.java.net/browse/JDK-8029019

This very good work. I have not done a complete review, but what
jumps out at me is that you have removed some of the validation
code and checks from AnnotationInvocationHandler ( in one place
there is an assert and a comment that “good” data is expected.
AnnotationInvocationHandler has shown up in many interesting
places, I wonder if it is wise to remove these.

-Chris.

>> But it is also an attempt to clean-up failure reporting for invalid
>> annotation types. Here's the 1st webrev:
>> 
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/8029019_Annotations.optimization/webrev.01/
>> 
>> 
>> With this patch applied, running the following benchmark:
>> 
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/8029019_Annotations.optimization/AnnotationsBench.java
>> 
>> 
>> Produces the following results:
>> 
>> Original:
>> 
>> Benchmark Mode  Cnt Score Error  Units
>> AnnotationsBench.a1_PrimitiveMember   avgt   10 20.790 ±   1.317  ns/op
>> AnnotationsBench.a2_ObjectMember  avgt   10 21.550 ±   0.580  ns/op
>> AnnotationsBench.a3_PrimitiveArrayMember  avgt   10 28.314 ±   1.477  ns/op
>> AnnotationsBench.a4_ObjectArrayMember avgt   10 27.094 ±   0.574  ns/op
>> AnnotationsBench.a5_AnnotationTypeavgt   10 13.047 ±   0.983  ns/op
>> AnnotationsBench.a6_HashCode  avgt   10 158.001 ±   9.347
>> ns/op
>> AnnotationsBench.a7_Equalsavgt   10 663.921 ±  27.256
>> ns/op
>> AnnotationsBench.a8_ToString  avgt   10 1772.901 ± 107.355
>> ns/op
>> 
>> Patched:
>> 
>> Benchmark Mode  Cnt ScoreError  Units
>> AnnotationsBench.a1_PrimitiveMember   avgt   10 8.547 ±  0.100  ns/op
>> AnnotationsBench.a2_ObjectMember  avgt   10 8.352 ±  0.340  ns/op
>> AnnotationsBench.a3_PrimitiveArrayMember  avgt   10 17.526 ±  0.696  ns/op
>> AnnotationsBench.a4_ObjectArrayMember avgt   10 15.639 ±  0.116  ns/op
>> AnnotationsBench.a5_AnnotationTypeavgt   10 4.692 ±  0.154  ns/op
>> AnnotationsBench.a6_HashCode  avgt   10 142.954 ±  7.460  ns/op
>> AnnotationsBench.a7_Equalsavgt   10 184.535 ±  0.917  ns/op
>> AnnotationsBench.a8_ToString  avgt   10 1806.685 ± 96.370
>> ns/op
>> 
>> 
>> Allocation rates are also reduced:
>> 
>> Original:
>> 
>> AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt   10
>> 16.000 ±   0.001B/op
>> AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt   1016.000
>> ±   0.001B/op
>> AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt
>> 1064.000 ±   0.001B/op
>> AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt   10
>> 48.000 ±   0.001B/op
>> AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt   10
>> 16.000 ±   0.001B/op
>> AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt   10   128.000 ±
>> 0.001B/op
>> AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt   10   120.001 ±
>> 0.001B/op
>> AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt   10  5200.002 ±
>> 0.001B/op
>> 
>> Patched:
>> 
>> AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt   10≈
>> 10⁻⁵  B/op
>> AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt   10≈
>> 10⁻⁵  B/op
>> AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt
>> 1048.000 ±   0.001B/op
>> AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt   10
>> 32.000 ±   0.001B/op
>> AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt   10≈
>> 10⁻⁵  B/op
>> AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt   1064.000 ±
>> 0.001B/op
>> AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt   1024.000 ±
>> 0.001B/op
>> AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt   10  5136.002 ±
>> 0.001B/op
>> 
>> 
>> As for the failure reporting: requesting an annotation for invalid
>> annotation type now always produces AnnotationFormatError. Previously,
>> some invalid annotations were simply skipped, some produced
>> AnnotationFormatError and for some of them, AnnotationFormatError was
>> raised only when evaluating Annotation's equals() method.
>> 
>> I would like to discuss this failure behavior more in-depth.
>> 
>> Regards, Peter
>>

Re: RFR: 8029019: (ann) Optimize annotation handling in core reflection

2017-01-24 Thread Claes Redestad


Hi Peter,

thanks for the thorough examination of this issue. After going through
the patch I do like what I see, but I have a few comments:

AnnotationInvocationHandler:
in invoke, cleaning up and replacing the explicit AssertionError should
be fine, but perhaps convert it to an assert that the number of
arguments is 1 when methodName is "equals" and 0 otherwise?

Adding @Stable is fine if that has a performance impact, but I think
we might preserve clarity of intent if you kept the fields as final and
kept using Unsafe to deserialize properly.

AnnotationSupport:
I dislike that this code was already catching Throwable, and that
you're now copying that approach.  Could the new the catch clause mimic
the one that previously wrapped the entire method?

AnnotationType:
accessibleMembers might not need to be volatile.

validateAnnotationMethod:
the block: label and break block seems unnecessary. I'd not worry
about optimizing for such invalid cases and simply run through all the
checks and set valid = false multiple times if so be it.

Thanks!

/Claes

On 2017-01-17 15:10, Peter Levart wrote:

Hi,

This is a preview of a patch that addresses the following issue:

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

But it is also an attempt to clean-up failure reporting for invalid
annotation types. Here's the 1st webrev:

http://cr.openjdk.java.net/~plevart/jdk9-dev/8029019_Annotations.optimization/webrev.01/


With this patch applied, running the following benchmark:

http://cr.openjdk.java.net/~plevart/jdk9-dev/8029019_Annotations.optimization/AnnotationsBench.java


Produces the following results:

Original:

Benchmark Mode  Cnt Score Error  Units
AnnotationsBench.a1_PrimitiveMember   avgt   10 20.790 ±   1.317  ns/op
AnnotationsBench.a2_ObjectMember  avgt   10 21.550 ±   0.580  ns/op
AnnotationsBench.a3_PrimitiveArrayMember  avgt   10 28.314 ±   1.477  ns/op
AnnotationsBench.a4_ObjectArrayMember avgt   10 27.094 ±   0.574  ns/op
AnnotationsBench.a5_AnnotationTypeavgt   10 13.047 ±   0.983  ns/op
AnnotationsBench.a6_HashCode  avgt   10 158.001 ±   9.347
ns/op
AnnotationsBench.a7_Equalsavgt   10 663.921 ±  27.256
ns/op
AnnotationsBench.a8_ToString  avgt   10 1772.901 ± 107.355
ns/op

Patched:

Benchmark Mode  Cnt ScoreError  Units
AnnotationsBench.a1_PrimitiveMember   avgt   10 8.547 ±  0.100  ns/op
AnnotationsBench.a2_ObjectMember  avgt   10 8.352 ±  0.340  ns/op
AnnotationsBench.a3_PrimitiveArrayMember  avgt   10 17.526 ±  0.696  ns/op
AnnotationsBench.a4_ObjectArrayMember avgt   10 15.639 ±  0.116  ns/op
AnnotationsBench.a5_AnnotationTypeavgt   10 4.692 ±  0.154  ns/op
AnnotationsBench.a6_HashCode  avgt   10 142.954 ±  7.460  ns/op
AnnotationsBench.a7_Equalsavgt   10 184.535 ±  0.917  ns/op
AnnotationsBench.a8_ToString  avgt   10 1806.685 ± 96.370
ns/op


Allocation rates are also reduced:

Original:

AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt   10
16.000 ±   0.001B/op
AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt   1016.000
±   0.001B/op
AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt
1064.000 ±   0.001B/op
AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt   10
48.000 ±   0.001B/op
AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt   10
16.000 ±   0.001B/op
AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt   10   128.000 ±
0.001B/op
AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt   10   120.001 ±
0.001B/op
AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt   10  5200.002 ±
0.001B/op

Patched:

AnnotationsBench.a1_PrimitiveMember:·gc.alloc.rate.norm avgt   10≈
10⁻⁵  B/op
AnnotationsBench.a2_ObjectMember:·gc.alloc.rate.norm avgt   10≈
10⁻⁵  B/op
AnnotationsBench.a3_PrimitiveArrayMember:·gc.alloc.rate.norm avgt
1048.000 ±   0.001B/op
AnnotationsBench.a4_ObjectArrayMember:·gc.alloc.rate.norm avgt   10
32.000 ±   0.001B/op
AnnotationsBench.a5_AnnotationType:·gc.alloc.rate.norm avgt   10≈
10⁻⁵  B/op
AnnotationsBench.a6_HashCode:·gc.alloc.rate.norm avgt   1064.000 ±
0.001B/op
AnnotationsBench.a7_Equals:·gc.alloc.rate.norm avgt   1024.000 ±
0.001B/op
AnnotationsBench.a8_ToString:·gc.alloc.rate.norm avgt   10  5136.002 ±
0.001B/op


As for the failure reporting: requesting an annotation for invalid
annotation type now always produces AnnotationFormatError. Previously,
some invalid annotations were simply skipped, some produced
AnnotationFormatError and for some of them, AnnotationFormatError was
raised only when evaluating Annotation's equals() method.

I would like to discuss this failure behavior more in-depth.

Regards, Peter

Re: (JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow

2017-01-24 Thread Daniel Fuchs


Hi,

Thanks for all the feedback.

I have updated the webrev
http://cr.openjdk.java.net/~dfuchs/webrev_8173111/webrev.01

The test now uses assertEquals as suggested by Lance and Aleksej
(thanks for the suggestion).
I also fixed the missing white space in the code that Christoph
spotted at line 67.
I kept the main method (useful when you tweak the test in
NetBeans).

I'm going to launch it again through our test system and if
successful I'll consider it reviewed and push it, unless I
hear otherwise by then :-)

best regards,

-- daniel



On 23/01/17 17:48, Daniel Fuchs wrote:

Hi,

Please find below a fix for:

8173111: Excessive recursion in EventFilterSupport when filtering
 over large number of XML events can cause StackOverflow
https://bugs.openjdk.java.net/browse/JDK-8173111

The fix is almost trivial: it replaces a recursion by a loop in
two places.

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

The test will fail without the fix (at least on my machine)
and pass with it (on my machine and in our test system).

best regards,

-- daniel

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

2017-01-24 Thread Langer, Christoph

Hi,

I submitted my fix under Bug https://bugs.openjdk.java.net/browse/JDK-8173261: 
http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/620d0c38581f

I also added a comment to https://bugs.openjdk.java.net/browse/JDK-8169827

Best regards
Christoph


> -Original Message-
> From: Frank Yuan [mailto:frank.y...@oracle.com]
> Sent: Dienstag, 24. Januar 2017 09:45
> To: Langer, Christoph 
> Cc: 'Daniel Fuchs' ; core-libs-dev@openjdk.java.net
> Subject: RE: RFR (JAXP) 8169827:
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> 
> > From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> > To: Frank Yuan
> > Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net
> > Subject: RE: RFR (JAXP) 8169827:
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> >
> > Hi Frank,
> >
> > thanks for your comments.
> >
> > I already thought it might be better to create a new bug for my repair work 
> > -
> as 8169827 actually is about the intermittent
> failure which will
> > perhaps not be solved with my changes. So I'll create a new item and submit
> my changes for that one.
> >
> > Furthermore, I would also support creating Java code to do the copy work
> instead of using the shell script, though doing this is
> rather out of
> > scope for me, at the moment.
> 
> Just a suggestion, please feel free to leave it:)
> 
> >This should be done in a common library, e.g.
> /test/javax/xml/jaxp/libs/jaxp/library. I also see that the same
> > requirement exists for tests in the jdk repo - unfortunately there is no 
> > shared
> testing library with 'jdk'. So we'll probably end
> up with some
> > duplicated code :(
> >
> > By the way: In the jaxp test tree, there exists a lib
> /test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff
> seems to be
> > obsolete for jaxp testing and/or significantly behind jdk's 
> > test/lib/testlibrary. I
> guess here's some room for
> cleanup/modernization.
> >
> 
> Yes, testlibrary is a copy from jdk repo, there are a few JAXP tests using the
> library, so we didn't keep it synced with JDK. In my
> mind, we don't need spend too much effort on it unless it impacts JAXP tests.
> 
> > As for the idea with the links - it sounds interesting, but it also adds 
> > more
> complexity. And maybe it is not such a good idea for
> Windows
> > systems...
> 
> I didn't test it in Windows, please leave it to me. I will update to you and 
> Joe
> when I have the result. Thanks
> 
> Frank
> 
> >
> > Best regards
> > Christoph
> >
> > > -Original Message-
> > > From: Frank Yuan [mailto:frank.y...@oracle.com]
> > > Sent: Dienstag, 24. Januar 2017 08:59
> > > To: Langer, Christoph ; 'Daniel Fuchs'
> > > ; core-libs-dev@openjdk.java.net
> > > Subject: RE: RFR (JAXP) 8169827:
> > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > >
> > > Hi Christoph
> > >
> > > Many thanks for your effort!
> > >
> > > It's really better than the old version! However I have 2 comments 
> > > although
> I
> > > am not a reviewer(as you often stated :))
> > > 1. we could also write java code to copy/delete JDK.
> > > 2. 8169827 is to track PropertiesTest failed in copying JDK 
> > > intermittently. I
> > > suggest to use another bug for your improvement
> > > because I am not sure if this patch can improve the issue of 8169827.
> > >
> > > To Christoph and Daniel
> > >
> > > Btw, Christoph's patch inspired me. You know, PropertiesTest copies a JDK
> in
> > > order to isolate the file change of the JDK, actually
> > > there are some other similar tests(to test some other JDK property or 
> > > config
> > > files) in JDK repo, they take the same way as well as
> > > have similar code...
> > > I have another idea for this test, that is we can only make symbolic link 
> > > to
> the
> > > JDK files/directories except conf directory, create
> > > a directory named with "conf" under the new jdk directory, and then make
> > > symbolic link to the files/directories in conf to the real
> > > jdk/conf except jaxp.properties. This will reduce the most of copying work
> and
> > > may reduce the risk of copying work. What's your
> > > opinion?
> > >
> > > Thanks
> > > Frank
> > >
> > > > -Original Message-
> > > > From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> > > > To: Daniel Fuchs; Frank Yuan; core-libs-dev@openjdk.java.net
> > > > Subject: RE: RFR (JAXP) 8169827:
> > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > > >
> > > > Hi Daniel,
> > > >
> > > > thanks for checking/reviewing. So I'll submit with removing the
> > > ProblemList.txt entry and I'll also remove the intermittent flag.
> > > >
> > > > Sounds fair to check later if problems will still show up. Although I 
> > > > have
> the
> > > feeling that the issue of
> > > > https://bugs.openjdk.java.net/browse/JDK-8147431 might appear again...
> >

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

2017-01-24 Thread Luke Hutchison

Thank you Remi and David for your detailed explanations, this was very
helpful, and I understand the issues now. Please go ahead and close the
Jira bug I created for this (JDK-8173252), I see this is WAI.

On Tue, Jan 24, 2017 at 12:39 AM,  wrote:

> Here is your code slightly modified, with no lambda, that deadlock too:

Thanks, I discovered that I hit a deadlock with anonymous inner classes
when I access fields in the enclosing class too.

On Tue, Jan 24, 2017 at 1:39 AM, David Holmes 
 wrote:

> I have to second Remi's view here - hidden concurrency is an accident
> waiting to happen, far too many things can go wrong if the users of your
> API don't know that new threads can be involved.

FastClasspathScanner only uses concurrency for its own internal processing.
The standard API that most users call is synchronous, and blocks on
response. The user-provided lambdas in question are run single-threaded on
a separate thread, but it does not execute concurrently with the main
thread (which is blocked), precisely to prevent surprises. If the user
calls the async API, they should understand about concurrency. I agree that
in general the burden falls to the user to read the docs and understand the
threading considerations of a given library.

In the end I decided to throw an exception if there is a "" line in
the stacktrace. This is better than the possibility of hitting a deadlock.
The following makes me think though that I should refactor my code to
ensure that, for the synchronous API, user-supplied methods are always
executed on the calling thread:

If the lambda code is called on the main thread and accesses the LambdaBug
> class it will see it as a recursive initialization attempt and simply
> return.

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

2017-01-24 Thread David Holmes

I have to second Remi's view here - hidden concurrency is an accident 
waiting to happen, far too many things can go wrong if the users of your 
API don't know that new threads can be involved.


It's not wrong, per-se, to start threads from a static initializer - 
just wrong to do something that has to wait, in the static initializer, 
for that other thread to do something, which in turn depends on being 
able to access the class being initialized.


In this case, as Remi explained (thanks!), the desugared lambda-code 
becomes a static method on the enclosing class, and executing a static 
method requires that the class is initialized.


David

On 24/01/2017 6:39 PM, fo...@univ-mlv.fr wrote:





*De: *"Luke Hutchison" 
*À: *"Remi Forax" 
*Cc: *"David Holmes" ,
core-libs-dev@openjdk.java.net
*Envoyé: *Mardi 24 Janvier 2017 09:13:17
*Objet: *Re: Calling a lambda expression from a new thread before
the main method is run causes the thread to lock up

On Tue, Jan 24, 2017 at 12:02 AM, Remi Forax > wrote:

a worker thread of the executor will try to execute the code of
the static method but because the static initializer is not
finished, the worker thread has to wait


But what is the worker thread waiting for in the case of the lambda
that it is not waiting for in the case of the AIC? I assume this is
due to the lexical scoping of the lambda?


it's not directly related to the lexical scoping of a lambda, it's
related to the fact that the body of a lambda is not inside the class
created when you transform a lambda to a class that implements the
functional interface but the body of a lambda is part of the class that
declares the lambda.


As a rule of thumb, never starts a thread in a static block, you
will get this classical deadlock AND your code is hard to test
because static blocks tend to  be executed in a random order
(i.e. class loading is lazy so execution of static blocks are
lazy too).


This is the problem: I am a library author, and a user of my library
ran into this problem. The user did not necessarily know that I was
launching new threads in my library, and as a library author, I did
not know I needed to advise users that they should not call my
library from static blocks. It seems to be quite a big problem that
a static block is not a standard execution context and can lead to
deadlocks.


Users should always know when they starts a thread, otherwise they may
send you objects that are not thread safe, multi-threading is not
something you can hide to your users.
Concurrency is already hard, hidden concurrency is harder :)

static blocks are executed with a kind of class lock to prevent several
threads to initialize the same class, so like any synchronized-like
block, they should be as small as possible.


Is there any way I can detect this in my code? I guess I could just
get a stacktrace, and look for "" or similar somewhere in the
stacktrace, then throw an exception if called in this context? Is
this a reliable enough test for being run in a static initializer block?


The real name of a static block is , not  which is used
for constructor.
If your API clearly states that you will start a thread, the burden to
fix deadlocks is transferred to your users :)
The extreme of that is that some people use the javadoc to cover their
own ass, "yes, i know this behavior is stupid but look, it's written in
the javadoc".


Here is your code slightly modified, with no lambda, that deadlock too:

import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class NoLambdaBug {
  static {
startUp();
  }

  static void iamstatic() {
System.out.println("Inner class method executed");
  }

  private static void startUp() {
System.out.println("Entering startUp()");
ExecutorService es = Executors.newSingleThreadExecutor();
try {
  Callable callable = new Callable() {
@Override
public Void call() throws Exception {
  iamstatic();
  return null;
}
  };
  es.submit(callable).get();
} catch (InterruptedException | ExecutionException e) {
  throw new RuntimeException(e);
} finally {
  es.shutdown();
}
System.out.println("Exiting startUp()");
  }

  public static void main(String[] args) {
System.out.println("Exiting main");
  }
}

Rémi

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

2017-01-24 Thread David Holmes


On 24/01/2017 5:21 PM, Luke Hutchison wrote:

On Mon, Jan 23, 2017 at 10:48 PM, David Holmes > wrote:

On 24/01/2017 2:41 PM, Luke Hutchison wrote:

If you run the code below, the active JVM thread (in the
ExecutorService)
locks up when the lambda expression is called. The Eclipse
debugger is not
able to stop the locked-up thread, or get a stacktrace beyond
the call into
the lambda.


That looks like a variation of the classic class initialization
deadlock problem. Your main thread is blocked in
es.submit(callable).get(); while still within the static initializer
of the LambdaBug class. If the executor thread also needs to ensure
LambdaBug is initialized (which it will in the lambda case) then it
will wait for the main thread to complete the initialization. Hence
deadlock.


Thanks for the explanation -- although I don't understand why a lambda
expression requires its defining class to be initialized, whereas an
anonymous inner class does not. The lambda body does not refer to the
LambdaBug class. This must be due to an implicit reference to the
defining class being added by the compiler for purposes of capturing the
lexical scope (with this issue not getting triggered in the case of
anonymous inner classes due to scoping differences)? However, why is the
containing class reference even used in this case, when variables in the
containing scope are not referenced?


I'd have to look at the exact code being generated for the lambda, but 
the enclosing class plays as special role with lambdas and so needs to 
be initialized before it can be used. There may be some scope for 
adjusting this.



If this is due to lexical scoping, shouldn't the compiler be able to add
some runtime code to detect cases of this kind of deadlock, and throw a
RuntimeException, or potentially even detect some of these cases statically?


Deadlock detection is out of scope for javac.


Why is it possible to directly call lambdas inside a static initializer
block without causing a deadlock (in other words, why is there no
deadlock as long as the lambdas are not executed on a different thread
while the main thread blocks)? The calling class is still not completely
initialized when calling lambdas directly in a static initializer block
on the main thread.


You need two threads for a deadlock. If the lambda code is called on the 
main thread and accesses the LambdaBug class it will see it as a 
recursive initialization attempt and simply return.


David

Re: RFR of JDK-8171142: jdk_rmi registry test fail to clean up on failure

2017-01-24 Thread Hamlin Li


Hi Roger, David,

Thank you for reviewing, modified as you suggested, and the code is pushed.

-Hamlin


On 2017/1/23 23:21, Roger Riggs wrote:

Hi Hamlin,

test/javax/rmi/PortableRemoteObject/ConcurrentHashMapTest.java:

  line 130: trivial, but please add a space in "!=null"...

Note: on Windows there is no difference between Process.destroy vs 
Process.destroyForcibly,

but on Linux it it is the difference between kill -15 and kill -9.

Using destroyForcibly would be more certain as to the death of the 
subprocess.


Otherwise looks fine

Thanks, Roger


On 1/23/2017 1:38 AM, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8171142

webrev: http://cr.openjdk.java.net/~mli/8171142/webrev.00/


Thank you

-Hamlin

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

2017-01-24 Thread Frank Yuan

> From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> To: Frank Yuan
> Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net
> Subject: RE: RFR (JAXP) 8169827: 
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> 
> Hi Frank,
> 
> thanks for your comments.
> 
> I already thought it might be better to create a new bug for my repair work - 
> as 8169827 actually is about the intermittent
failure which will
> perhaps not be solved with my changes. So I'll create a new item and submit 
> my changes for that one.
> 
> Furthermore, I would also support creating Java code to do the copy work 
> instead of using the shell script, though doing this is
rather out of
> scope for me, at the moment. 

Just a suggestion, please feel free to leave it:)

>This should be done in a common library, e.g. 
>/test/javax/xml/jaxp/libs/jaxp/library. I also see that the same
> requirement exists for tests in the jdk repo - unfortunately there is no 
> shared testing library with 'jdk'. So we'll probably end
up with some
> duplicated code :(
> 
> By the way: In the jaxp test tree, there exists a lib 
> /test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff
seems to be
> obsolete for jaxp testing and/or significantly behind jdk's 
> test/lib/testlibrary. I guess here's some room for
cleanup/modernization.
> 

Yes, testlibrary is a copy from jdk repo, there are a few JAXP tests using the 
library, so we didn't keep it synced with JDK. In my
mind, we don't need spend too much effort on it unless it impacts JAXP tests.

> As for the idea with the links - it sounds interesting, but it also adds more 
> complexity. And maybe it is not such a good idea for
Windows
> systems...

I didn't test it in Windows, please leave it to me. I will update to you and 
Joe when I have the result. Thanks

Frank

> 
> Best regards
> Christoph
> 
> > -Original Message-
> > From: Frank Yuan [mailto:frank.y...@oracle.com]
> > Sent: Dienstag, 24. Januar 2017 08:59
> > To: Langer, Christoph ; 'Daniel Fuchs'
> > ; core-libs-dev@openjdk.java.net
> > Subject: RE: RFR (JAXP) 8169827:
> > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> >
> > Hi Christoph
> >
> > Many thanks for your effort!
> >
> > It's really better than the old version! However I have 2 comments although 
> > I
> > am not a reviewer(as you often stated :))
> > 1. we could also write java code to copy/delete JDK.
> > 2. 8169827 is to track PropertiesTest failed in copying JDK intermittently. 
> > I
> > suggest to use another bug for your improvement
> > because I am not sure if this patch can improve the issue of 8169827.
> >
> > To Christoph and Daniel
> >
> > Btw, Christoph's patch inspired me. You know, PropertiesTest copies a JDK in
> > order to isolate the file change of the JDK, actually
> > there are some other similar tests(to test some other JDK property or config
> > files) in JDK repo, they take the same way as well as
> > have similar code...
> > I have another idea for this test, that is we can only make symbolic link 
> > to the
> > JDK files/directories except conf directory, create
> > a directory named with "conf" under the new jdk directory, and then make
> > symbolic link to the files/directories in conf to the real
> > jdk/conf except jaxp.properties. This will reduce the most of copying work 
> > and
> > may reduce the risk of copying work. What's your
> > opinion?
> >
> > Thanks
> > Frank
> >
> > > -Original Message-
> > > From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> > > To: Daniel Fuchs; Frank Yuan; core-libs-dev@openjdk.java.net
> > > Subject: RE: RFR (JAXP) 8169827:
> > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > >
> > > Hi Daniel,
> > >
> > > thanks for checking/reviewing. So I'll submit with removing the
> > ProblemList.txt entry and I'll also remove the intermittent flag.
> > >
> > > Sounds fair to check later if problems will still show up. Although I 
> > > have the
> > feeling that the issue of
> > > https://bugs.openjdk.java.net/browse/JDK-8147431 might appear again...
> > >
> > > Best regards
> > > Christoph
> > >
> > > > -Original Message-
> > > > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> > > > Sent: Montag, 23. Januar 2017 17:12
> > > > To: Langer, Christoph ; Frank Yuan
> > > > ; core-libs-dev@openjdk.java.net
> > > > Subject: Re: RFR (JAXP) 8169827:
> > > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > > >
> > > > Hi Christoph,
> > > >
> > > > Thanks for fixing this test!
> > > >
> > > > I imported your patch, modified ProblemList.txt to not skip the test,
> > > > and sent it through our test system, and I'm happy to report that
> > > > the test was run on all available platforms with no failure.
> > > >
> > > > So I think you should simply remove the line from ProblemList.txt
> >

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

2017-01-24 Thread forax

> De: "Luke Hutchison" 
> À: "Remi Forax" 
> Cc: "David Holmes" , core-libs-dev@openjdk.java.net
> Envoyé: Mardi 24 Janvier 2017 09:13:17
> Objet: Re: Calling a lambda expression from a new thread before the main 
> method
> is run causes the thread to lock up

> On Tue, Jan 24, 2017 at 12:02 AM, Remi Forax < fo...@univ-mlv.fr > wrote:

>> a worker thread of the executor will try to execute the code of the static
>> method but because the static initializer is not finished, the worker thread
>> has to wait
> But what is the worker thread waiting for in the case of the lambda that it is
> not waiting for in the case of the AIC? I assume this is due to the lexical
> scoping of the lambda?

it's not directly related to the lexical scoping of a lambda, it's related to 
the fact that the body of a lambda is not inside the class created when you 
transform a lambda to a class that implements the functional interface but the 
body of a lambda is part of the class that declares the lambda. 

>> As a rule of thumb, never starts a thread in a static block, you will get 
>> this
>> classical deadlock AND your code is hard to test because static blocks tend 
>> to
>> be executed in a random order (i.e. class loading is lazy so execution of
>> static blocks are lazy too).

> This is the problem: I am a library author, and a user of my library ran into
> this problem. The user did not necessarily know that I was launching new
> threads in my library, and as a library author, I did not know I needed to
> advise users that they should not call my library from static blocks. It seems
> to be quite a big problem that a static block is not a standard execution
> context and can lead to deadlocks.

Users should always know when they starts a thread, otherwise they may send you 
objects that are not thread safe, multi-threading is not something you can hide 
to your users. 
Concurrency is already hard, hidden concurrency is harder :) 

static blocks are executed with a kind of class lock to prevent several threads 
to initialize the same class, so like any synchronized-like block, they should 
be as small as possible. 

> Is there any way I can detect this in my code? I guess I could just get a
> stacktrace, and look for "" or similar somewhere in the stacktrace, then
> throw an exception if called in this context? Is this a reliable enough test
> for being run in a static initializer block?

The real name of a static block is , not  which is used for 
constructor. 
If your API clearly states that you will start a thread, the burden to fix 
deadlocks is transferred to your users :) 
The extreme of that is that some people use the javadoc to cover their own ass, 
"yes, i know this behavior is stupid but look, it's written in the javadoc". 

Here is your code slightly modified, with no lambda, that deadlock too: 

import java.util.concurrent.Callable; 
import java.util.concurrent.ExecutionException; 
import java.util.concurrent.ExecutorService; 
import java.util.concurrent.Executors; 

public class NoLambdaBug { 
static { 
startUp(); 
} 

static void iamstatic() { 
System.out.println("Inner class method executed"); 
} 

private static void startUp() { 
System.out.println("Entering startUp()"); 
ExecutorService es = Executors.newSingleThreadExecutor(); 
try { 
Callable callable = new Callable() { 
@Override 
public Void call() throws Exception { 
iamstatic(); 
return null; 
} 
}; 
es.submit(callable).get(); 
} catch (InterruptedException | ExecutionException e) { 
throw new RuntimeException(e); 
} finally { 
es.shutdown(); 
} 
System.out.println("Exiting startUp()"); 
} 

public static void main(String[] args) { 
System.out.println("Exiting main"); 
} 
} 

Rémi

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

2017-01-24 Thread Langer, Christoph

Hi Frank,

thanks for your comments.

I already thought it might be better to create a new bug for my repair work - 
as 8169827 actually is about the intermittent failure which will perhaps not be 
solved with my changes. So I'll create a new item and submit my changes for 
that one.

Furthermore, I would also support creating Java code to do the copy work 
instead of using the shell script, though doing this is rather out of scope for 
me, at the moment. This should be done in a common library, e.g. 
/test/javax/xml/jaxp/libs/jaxp/library. I also see that the same requirement 
exists for tests in the jdk repo - unfortunately there is no shared testing 
library with 'jdk'. So we'll probably end up with some duplicated code :(

By the way: In the jaxp test tree, there exists a lib 
/test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff 
seems to be obsolete for jaxp testing and/or significantly behind jdk's 
test/lib/testlibrary. I guess here's some room for cleanup/modernization.

As for the idea with the links - it sounds interesting, but it also adds more 
complexity. And maybe it is not such a good idea for Windows systems...

Best regards
Christoph

> -Original Message-
> From: Frank Yuan [mailto:frank.y...@oracle.com]
> Sent: Dienstag, 24. Januar 2017 08:59
> To: Langer, Christoph ; 'Daniel Fuchs'
> ; core-libs-dev@openjdk.java.net
> Subject: RE: RFR (JAXP) 8169827:
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> 
> Hi Christoph
> 
> Many thanks for your effort!
> 
> It's really better than the old version! However I have 2 comments although I
> am not a reviewer(as you often stated :))
> 1. we could also write java code to copy/delete JDK.
> 2. 8169827 is to track PropertiesTest failed in copying JDK intermittently. I
> suggest to use another bug for your improvement
> because I am not sure if this patch can improve the issue of 8169827.
> 
> To Christoph and Daniel
> 
> Btw, Christoph's patch inspired me. You know, PropertiesTest copies a JDK in
> order to isolate the file change of the JDK, actually
> there are some other similar tests(to test some other JDK property or config
> files) in JDK repo, they take the same way as well as
> have similar code...
> I have another idea for this test, that is we can only make symbolic link to 
> the
> JDK files/directories except conf directory, create
> a directory named with "conf" under the new jdk directory, and then make
> symbolic link to the files/directories in conf to the real
> jdk/conf except jaxp.properties. This will reduce the most of copying work and
> may reduce the risk of copying work. What's your
> opinion?
> 
> Thanks
> Frank
> 
> > -Original Message-
> > From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> > To: Daniel Fuchs; Frank Yuan; core-libs-dev@openjdk.java.net
> > Subject: RE: RFR (JAXP) 8169827:
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> >
> > Hi Daniel,
> >
> > thanks for checking/reviewing. So I'll submit with removing the
> ProblemList.txt entry and I'll also remove the intermittent flag.
> >
> > Sounds fair to check later if problems will still show up. Although I have 
> > the
> feeling that the issue of
> > https://bugs.openjdk.java.net/browse/JDK-8147431 might appear again...
> >
> > Best regards
> > Christoph
> >
> > > -Original Message-
> > > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> > > Sent: Montag, 23. Januar 2017 17:12
> > > To: Langer, Christoph ; Frank Yuan
> > > ; core-libs-dev@openjdk.java.net
> > > Subject: Re: RFR (JAXP) 8169827:
> > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
> > >
> > > Hi Christoph,
> > >
> > > Thanks for fixing this test!
> > >
> > > I imported your patch, modified ProblemList.txt to not skip the test,
> > > and sent it through our test system, and I'm happy to report that
> > > the test was run on all available platforms with no failure.
> > >
> > > So I think you should simply remove the line from ProblemList.txt
> > > (no need for a new webrev).
> > > If it fails again on more exotic platform we'll simply add it
> > > back to ProblemList.txt for those platforms where it fails
> > > (I guess it could happen if there's not enough disk space).
> > >
> > > Otherwise I have looked over the changes you proposed and they
> > > do seem OK to me.
> > >
> > > +1
> > >
> > > best regards,
> > >
> > > -- daniel
> > >
> > > On 23/01/17 10:03, Langer, Christoph wrote:
> > > > Hi,
> > > >
> > > > while working on jaxp changes and running jtreg tests I found that test
> > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh does not work. I then
> saw
> > > that this was already reported with bug 8169827. But, as I had already
> spent
> > > some time to fix this test I'd like to contribute my fix:
> > > >
> > > > Bug:

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

2017-01-24 Thread Luke Hutchison

On Tue, Jan 24, 2017 at 12:02 AM, Remi Forax  wrote:

> a worker thread of the executor will try to execute the code of the static
> method but because the static initializer is not finished, the worker
> thread has to wait

But what is the worker thread waiting for in the case of the lambda that it
is not waiting for in the case of the AIC? I assume this is due to the
lexical scoping of the lambda?

As a rule of thumb, never starts a thread in a static block, you will get
> this classical deadlock AND your code is hard to test because static blocks
> tend to  be executed in a random order (i.e. class loading is lazy so
> execution of static blocks are lazy too).
>

This is the problem: I am a library author, and a user of my library ran
into this problem. The user did not necessarily know that I was launching
new threads in my library, and as a library author, I did not know I needed
to advise users that they should not call my library from static blocks. It
seems to be quite a big problem that a static block is not a standard
execution context and can lead to deadlocks.

Is there any way I can detect this in my code? I guess I could just get a
stacktrace, and look for "" or similar somewhere in the stacktrace,
then throw an exception if called in this context? Is this a reliable
enough test for being run in a static initializer block?

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

2017-01-24 Thread Remi Forax

- Mail original -
> De: "Luke Hutchison" 
> À: "David Holmes" 
> Cc: core-libs-dev@openjdk.java.net
> Envoyé: Mardi 24 Janvier 2017 08:21:39
> Objet: Re: Calling a lambda expression from a new thread before the main  
> method is run causes the thread to lock up

> On Mon, Jan 23, 2017 at 10:48 PM, David Holmes 
> wrote:
> 
>> On 24/01/2017 2:41 PM, Luke Hutchison wrote:
>>
>>> If you run the code below, the active JVM thread (in the ExecutorService)
>>> locks up when the lambda expression is called. The Eclipse debugger is not
>>> able to stop the locked-up thread, or get a stacktrace beyond the call
>>> into
>>> the lambda.
>>>
>>
>> That looks like a variation of the classic class initialization deadlock
>> problem. Your main thread is blocked in es.submit(callable).get(); while
>> still within the static initializer of the LambdaBug class. If the executor
>> thread also needs to ensure LambdaBug is initialized (which it will in the
>> lambda case) then it will wait for the main thread to complete the
>> initialization. Hence deadlock.
>>
> 
> Thanks for the explanation -- although I don't understand why a lambda
> expression requires its defining class to be initialized, whereas an
> anonymous inner class does not. The lambda body does not refer to the
> LambdaBug class. This must be due to an implicit reference to the defining
> class being added by the compiler for purposes of capturing the lexical
> scope (with this issue not getting triggered in the case of anonymous inner
> classes due to scoping differences)? However, why is the containing class
> reference even used in this case, when variables in the containing scope
> are not referenced?

The body of a lambda is desugared by the compiler into a static method, so when 
you submit the callable, a worker thread of the executor will try to execute 
the code of the static method but because the static initializer is not 
finished, the worker thread has to wait, then you call get() inside the static 
block that waits that the worker thread to finish its work => deadlock.

In case of an inner class, your code is in the inner class so the worker thread 
don't wait the static block to be finished, so no deadlock

As a rule of thumb, never starts a thread in a static block, you will get this 
classical deadlock AND your code is hard to test because static blocks tend to  
be executed in a random order (i.e. class loading is lazy so execution of 
static blocks are lazy too). 

> 
> If this is due to lexical scoping, shouldn't the compiler be able to add
> some runtime code to detect cases of this kind of deadlock, and throw a
> RuntimeException, or potentially even detect some of these cases statically?
> 
> Why is it possible to directly call lambdas inside a static initializer
> block without causing a deadlock (in other words, why is there no deadlock
> as long as the lambdas are not executed on a different thread while the
> main thread blocks)? The calling class is still not completely initialized
> when calling lambdas directly in a static initializer block on the main
> thread.

cheers,
Rémi

RFR of JDK-8173326: Problem list java/rmi/registry/readTest/CodebaseTest.java on Windows

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

Re: Cleanup String.replace() occurences with single characters

Re: RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

Cleanup String.replace() occurences with single characters

RFR: 8170832: Add a new launcher environment variable JAVA_OPTIONS

Re: RFR: 8029019: (ann) Optimize annotation handling in core reflection

Re: RFR: 8029019: (ann) Optimize annotation handling in core reflection

Re: (JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

Re: RFR of JDK-8171142: jdk_rmi registry test fail to clean up on failure

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

Re: Calling a lambda expression from a new thread before the main method is run causes the thread to lock up

19 matches

Site Navigation

Mail list logo

Footer information