Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Vladimir Ivanov]

Shilpi,

_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF:
  LF_HIDDEN_SIG
  LF_COMPILED_SIG
  FORCEINLINE_SIG
  DONTINLINE_SIG

Otherwise, looks fine.

Best regards,
Vladimir Ivanov

On 2/17/16 5:47 PM, shilpi rastogi wrote:

Hi All,

Please review fix for the following bug-

https://bugs.openjdk.java.net/browse/JDK-8144931
http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/


Thanks,
Shilpi

Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata

[Kevin Walls]

Hi Cheleswer,

Looks good to me.

Thanks
Kevin
(Also, as one of the comments was that there may be no real cost to 
using default stack sizes here (on most systems...?), having 
jdk.lang.processReaperUseDefaultStackSize gives us a way to test that 
widely, and one day the 32k may be able to disappear.)





On 17/02/2016 15:17, cheleswer sahu wrote:

Hi,
I have made changes in the property name 
(jdk.lang.processReaperUseDefaultStackSize) and code as suggested in 
the earlier reviews.


 --- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java 
2016-02-17 18:48:10.08163 +0530

@@ -81,9 +81,8 @@
 ThreadGroup systemThreadGroup = tg;

 ThreadFactory threadFactory = grimReaper -> {
-// Our thread stack requirement is quite modest.
-Thread t = new Thread(systemThreadGroup, grimReaper,
-"process reaper", 32768);
+long stackSize = 
Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") ? 0 : 
32768;
+Thread t = new Thread(systemThreadGroup, 
grimReaper, "process reaper", stackSize);

 t.setDaemon(true);
 // A small attempt (probably futile) to avoid 
priority inversion

 t.setPriority(Thread.MAX_PRIORITY);

I would really like to thanks "Martin" for the patch 
(http://cr.openjdk.java.net/~martin/webrevs/openjdk9/tls-size-guarantee/) 
which he has provided. Since we support a wider range of glibc 
versions and platform using patch will
require more testing and work. I think the use of system property for 
this issue will be safer at this point of time.


Regards,
Cheleswer


On 1/19/2016 5:40 PM, David Holmes wrote:

On 19/01/2016 9:53 PM, Kevin Walls wrote:

|
Hi Cheleswer, I think Martin is suggesting something like:
|

// Use a modest stack size, unless requested otherwise:
long stackSize = 
Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0 : 32768;


Someone from core-libs needs to chime on what the appropriate 
namespace for such a property would be.


David
-

Thread t = new Thread(systemThreadGroup, grimReaper, "process 
reaper", stackSize);


|||
If that tests OK for you, it may be the way we can go ahead with the
point fix for this.

Regards
Kevin
|
On 18/01/2016 16:52, Martin Buchholz wrote:

Historical note - I never liked having a reaper thread for each
subprocess, but no other reliable implementation is known. Given the
potential for many reaper threads, I at least wanted to keep the
memory waste low.

On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
 wrote:


+   Thread t = null;
+   if
(Boolean.getBoolean("processReaperUseDefaultStackSize")) {
+   t = new Thread(systemThreadGroup, grimReaper,
"process reaper");
+} else {
+   // Our thread stack requirement is quite 
modest.

+   t = new Thread(systemThreadGroup, grimReaper,
"process reaper", 32768);
+}

If we do end up using this strategy, cleaner would be to use
https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#Thread-java.lang.ThreadGroup-java.lang.Runnable-java.lang.String-long- 


stackSize - the desired stack size for the new thread, or zero to
indicate that this parameter is to be ignored.

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[shilpi.rast...@oracle.com]

Thank You Vladimir!

I have done the changes. Please review the updated patch-

http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/

Regards,
Shilpi

On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:

Shilpi,

_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF:
  LF_HIDDEN_SIG
  LF_COMPILED_SIG
  FORCEINLINE_SIG
  DONTINLINE_SIG

Otherwise, looks fine.

Best regards,
Vladimir Ivanov

On 2/17/16 5:47 PM, shilpi rastogi wrote:

Hi All,

Please review fix for the following bug-

https://bugs.openjdk.java.net/browse/JDK-8144931
http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/


Thanks,
Shilpi

Re: RFR 9: 8149750 Decouple sun.misc.Signal from the base module

[Chris Hegarty]

On 16 Feb 2016, at 21:38, Roger Riggs  wrote:

> Hi Chris,
> 
> Webrev updated in place:
>http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/

Thanks Roger,  I’m happy with the source changes.

-Chris.

> On 2/15/2016 6:22 AM, Chris Hegarty wrote:
>> On 12/02/16 17:47, Roger Riggs wrote: 
>>> Please review moving the functionality of sun.misc.Signal to 
>>> jdk.internal.misc and 
>>> creating a wrapper sun.misc.Signal for existing external uses. 
>>> 
>>> +++ This time including the hotspot changes to update the target of the 
>>> upcalls. 
>>> 
>>> A new replacement API will be considered separately. 
>>> 
>>> The update includes a test that passes with or without the changes. 
>>> (Except for an NPE instead of SEGV if null is passed). 
>>> 
>>> Webrev: 
>>>jdk: http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/ 
>> 
>> Overall looks ok, and satisfies the requirement of JEP 260. 
>> 
>> It took me a while to satisfy myself that it is ok to "ignore" 
>> the passed Signal in the wrapper's 'handle' methods. The assumption 
>> is that this wrapper's signal field and the passes signal are, MUST, 
>> represent the same underlying signal. Maybe an assert to make this 
>> explicit? 
> The jdk.internal.misc.Signal passed to the wrapper's methods needs to be 
> mapped to the corresponding sun.misc.Signal but instead of maintaining a map
> the s.m.Signal instance is kept in the wrapper of the s.m.SignalHandler.
>> 
>> Looking at j.i.m.Signal., I can see that you explicitly check 
>> for the 'SIG' prefix and prepend it if not there, but toString() also 
>> prepends it. Will getName also be impacted by this ( it may now have 
>> the name prepended with 'SIG', where it previously was not ). 
> Yes, there was a bug there; reverting to backward compatible behavior.
> HotSpot recognizes different forms of signal names on Windows and Posix.  [1]
> To be compatible with previous versions, the "SIG" prefix allowed by the 
> HotSpot change
> (only on Posix) is not supported and throws IllegalArgumentException.
> 
> Thanks, Roger
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8149748
> 
>> 
>>> hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-9149750/ 
>> 
>> Looks fine. 
>> 
>> -Chris. 
> 

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Alan Bateman]

On 15/02/2016 16:30, Steve Drach wrote:

Thank you Alan.  I’ll address the issues you bring up before integration.
Thanks. Are you planning to update the webrev too as it would be nice to 
see the final javadoc?


-Alan

RFR [9] 8150163: JarFileSystem support for MRJARs should use the JDK specific Version API

[Chris Hegarty]

To support MRJARs, currently JarFileSystem uses sun.misc.Version to retrieve the
major version. It should be updated to use the new JDK specific Version API. 

http://cr.openjdk.java.net/~chegar/8150163/
https://bugs.openjdk.java.net/browse/JDK-8150163

-Chris.

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Vladimir Ivanov]

Reviewed.

Best regards,
Vladimir Ivanov

On 2/18/16 2:18 PM, shilpi.rast...@oracle.com wrote:

Thank You Vladimir!

I have done the changes. Please review the updated patch-

http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/

Regards,
Shilpi

On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:

Shilpi,

_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF:
  LF_HIDDEN_SIG
  LF_COMPILED_SIG
  FORCEINLINE_SIG
  DONTINLINE_SIG

Otherwise, looks fine.

Best regards,
Vladimir Ivanov

On 2/17/16 5:47 PM, shilpi rastogi wrote:

Hi All,

Please review fix for the following bug-

https://bugs.openjdk.java.net/browse/JDK-8144931
http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/


Thanks,
Shilpi

RE: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE

[Langer, Christoph]

Hi Joe,

here is the next version:
http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/

Hopefully the final one :-)

> Yes, it's good to have some cleanups. For the generic initialization,
> it's preferable to have no redundant type arguments, for example,
> instead of:
> 
> protected Stack fEntityStack = new Stack();
> 
> do:
> protected Stack fEntityStack = new Stack<>();
>
> Some of the "cleanup" in the webrev were reversing <> with added type
> argument(s), that would be unnecessary.

I think I did that on all the places that I touched now.

> For the obsolete Vector, it would be good to replace it with ArrayList.

There are so many Vector instances in this code that it's out of scope for me 
now to touch them all...

> Copyright year "2016" in XSAttributeChecker needs to be "2016," or else
> the script would complain.

For that one I thought that it would look like "2015,2016" if the initial 
copyright was done in 2015. But I changed it as you suggested.

> In XSDHandler, note that there's another call to get SECURITY_MANAGER
> towards the end of the reset method (at 3572 in your new version) that
> may be removed. You may also consolidate most of those
> setFeature/Property statements in one try-catch block if you want.

OK, I did some further refacturing here. However, I've left the try/catch 
blocks as I think it's the intention to silently catch some exceptions but then 
try to go on with the next property.

> For the test, the @bug tag is still desirable in the general comment
> section. For now, it's @bug 8149915. As we add more test to the class in
> the future, we'd then append more bug ids, @bug 8149915, xxx.

Done. I also left the @bug in the comments for the test method.

> > I'm also wondering why some files have a copyright header like this, e.g.
> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11D
> TDScannerImpl.java:
> > /*
> >   * reserved comment block
> >   * DO NOT REMOVE OR ALTER!
> >   */
> >
> > Shouldn't there be the standard copyright header or is there some reason
> behind it?
> 
> The block is used by the licensing script to swap with license
> information required by licensees. For code that came from Xerces, we
> should keep the block as is with the Apache License. But when we make
> changes, we're required to put in the copyright header. Since the
> changes you made are cleanup / removing unused fields, it's okay to keep
> the block as is.

OK :)

Thanks again for reviewing.

All the Best
Christoph

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Paul Sandoz]

> On 18 Feb 2016, at 12:18, shilpi.rast...@oracle.com wrote:
> 
> Thank You Vladimir!
> 
> I have done the changes. Please review the updated patch-
> 
> http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
> 

Looking good. There are also a few other cases in InvokerBytecodeGenerator you 
can update:

 679 
mv.visitAnnotation("Ljava/lang/invoke/InjectedProfile;", true);


1328 // Suppress this method in backtraces displayed to the user.
1329 mv.visitAnnotation("Ljava/lang/invoke/LambdaForm$Hidden;", true);
1330
1331 // Don't inline the interpreter entry.
1332 mv.visitAnnotation("Ljdk/internal/vm/annotation/DontInline;", 
true);


1387 // Suppress this method in backtraces displayed to the user.
1388 mv.visitAnnotation("Ljava/lang/invoke/LambdaForm$Hidden;", true);
1389
1390 // Force inlining of this invoker method.
1391 mv.visitAnnotation("Ljdk/internal/vm/annotation/ForceInline;", 
true);
1392

That might be all the rest but i have not double checked.

As a follow on task can you check other classes in j.l.i where this technique 
might be applied? if so we can log another issue for that e.g search using the 
regex \"L[\w\/]+;\”.

Paul.

> Regards,
> Shilpi
> 
> On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
>> Shilpi,
>> 
>> _CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to LF:
>>  LF_HIDDEN_SIG
>>  LF_COMPILED_SIG
>>  FORCEINLINE_SIG
>>  DONTINLINE_SIG
>> 
>> Otherwise, looks fine.
>> 
>> Best regards,
>> Vladimir Ivanov
>> 
>> On 2/17/16 5:47 PM, shilpi rastogi wrote:
>>> Hi All,
>>> 
>>> Please review fix for the following bug-
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8144931
>>> http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
>>> 
>>> 
>>> Thanks,
>>> Shilpi
> 

RE: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Uwe Schindler]

Hi,

just a stupid question from somebody outside the OpenJDK developers:
You are already using ASM to generate the class files. Why not also use the 
Type class in ASM to generate the signatures of a class constant?:

Instead of:

static final String  LF_HIDDEN_SIG = 
className("Ljava/lang/invoke/LambdaForm$Hidden;");

Use the following to define the constant:

import jdk.internal.org.objectweb.asm.Type;
import java.lang.invoke.LambdaForm.Hidden;
static final String  LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);

This is compile-time checked, because of the .class notation.

Thanks,
Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of shilpi.rast...@oracle.com
> Sent: Thursday, February 18, 2016 12:18 PM
> To: Vladimir Ivanov ; core-libs-
> d...@openjdk.java.net
> Subject: Re: RFR 8144931: Assert class signatures are correct and refer to 
> valid
> classes
> 
> Thank You Vladimir!
> 
> I have done the changes. Please review the updated patch-
> 
> http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
> 
> Regards,
> Shilpi
> 
> On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
> > Shilpi,
> >
> > _CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to
> LF:
> >   LF_HIDDEN_SIG
> >   LF_COMPILED_SIG
> >   FORCEINLINE_SIG
> >   DONTINLINE_SIG
> >
> > Otherwise, looks fine.
> >
> > Best regards,
> > Vladimir Ivanov
> >
> > On 2/17/16 5:47 PM, shilpi rastogi wrote:
> >> Hi All,
> >>
> >> Please review fix for the following bug-
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8144931
> >> http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
> >>
> >>
> >> Thanks,
> >> Shilpi

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Paul Sandoz]

Hi Uwe

Not a stupid question.

We took the conservative approach to preserve the existing costs (avoid linkage 
and string generation).

Paul.

> On 18 Feb 2016, at 14:54, Uwe Schindler  wrote:
> 
> Hi,
> 
> just a stupid question from somebody outside the OpenJDK developers:
> You are already using ASM to generate the class files. Why not also use the 
> Type class in ASM to generate the signatures of a class constant?:
> 
> Instead of:
> 
> static final String  LF_HIDDEN_SIG = 
> className("Ljava/lang/invoke/LambdaForm$Hidden;");
> 
> Use the following to define the constant:
> 
> import jdk.internal.org.objectweb.asm.Type;
> import java.lang.invoke.LambdaForm.Hidden;
> static final String  LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);
> 
> This is compile-time checked, because of the .class notation.
> 
> Thanks,
> Uwe
> 
> -
> Uwe Schindler
> uschind...@apache.org
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> http://lucene.apache.org/
> 
>> -Original Message-
>> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
>> Behalf Of shilpi.rast...@oracle.com
>> Sent: Thursday, February 18, 2016 12:18 PM
>> To: Vladimir Ivanov ; core-libs-
>> d...@openjdk.java.net
>> Subject: Re: RFR 8144931: Assert class signatures are correct and refer to 
>> valid
>> classes
>> 
>> Thank You Vladimir!
>> 
>> I have done the changes. Please review the updated patch-
>> 
>> http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
>> 
>> Regards,
>> Shilpi
>> 
>> On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
>>> Shilpi,
>>> 
>>> _CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM to
>> LF:
>>>  LF_HIDDEN_SIG
>>>  LF_COMPILED_SIG
>>>  FORCEINLINE_SIG
>>>  DONTINLINE_SIG
>>> 
>>> Otherwise, looks fine.
>>> 
>>> Best regards,
>>> Vladimir Ivanov
>>> 
>>> On 2/17/16 5:47 PM, shilpi rastogi wrote:
 Hi All,
 
 Please review fix for the following bug-
 
 https://bugs.openjdk.java.net/browse/JDK-8144931
 http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
 
 
 Thanks,
 Shilpi
> 

RE: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Uwe Schindler]

Hi,

Thanks! I was expecting something like this. You just want to check the class 
name in an assertion - that’s fine.

Kind regards,
Uwe

P.S.: You could improve the assertion with the Type class, so it would also 
fail on otherwise broken descriptor strings (missing "L" or missing ";"):

static boolean checkClassName(String cn) {
   Type tp = Type.getType(cn);
   // additional sanity so only valid "L;" descriptors work
   if (tp.getSort() != Type.OBJECT) {
 return false;
   }
   try {
   Class c = Class.forName(tp.getClassName(), false, null);
   return true;
   } catch (ClassNotFoundException e) {
   return false;
   }
   }

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/


> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Paul Sandoz
> Sent: Thursday, February 18, 2016 3:05 PM
> Cc: Java Core Libs 
> Subject: Re: RFR 8144931: Assert class signatures are correct and refer to 
> valid
> classes
> 
> Hi Uwe
> 
> Not a stupid question.
> 
> We took the conservative approach to preserve the existing costs (avoid
> linkage and string generation).
> 
> Paul.
> 
> > On 18 Feb 2016, at 14:54, Uwe Schindler  wrote:
> >
> > Hi,
> >
> > just a stupid question from somebody outside the OpenJDK developers:
> > You are already using ASM to generate the class files. Why not also use the
> Type class in ASM to generate the signatures of a class constant?:
> >
> > Instead of:
> >
> > static final String  LF_HIDDEN_SIG =
> className("Ljava/lang/invoke/LambdaForm$Hidden;");
> >
> > Use the following to define the constant:
> >
> > import jdk.internal.org.objectweb.asm.Type;
> > import java.lang.invoke.LambdaForm.Hidden;
> > static final String  LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);
> >
> > This is compile-time checked, because of the .class notation.
> >
> > Thanks,
> > Uwe
> >
> > -
> > Uwe Schindler
> > uschind...@apache.org
> > ASF Member, Apache Lucene PMC / Committer
> > Bremen, Germany
> > http://lucene.apache.org/
> >
> >> -Original Message-
> >> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> >> Behalf Of shilpi.rast...@oracle.com
> >> Sent: Thursday, February 18, 2016 12:18 PM
> >> To: Vladimir Ivanov ; core-libs-
> >> d...@openjdk.java.net
> >> Subject: Re: RFR 8144931: Assert class signatures are correct and refer to
> valid
> >> classes
> >>
> >> Thank You Vladimir!
> >>
> >> I have done the changes. Please review the updated patch-
> >>
> >> http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/
> >>
> >> Regards,
> >> Shilpi
> >>
> >> On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:
> >>> Shilpi,
> >>>
> >>> _CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM
> to
> >> LF:
> >>>  LF_HIDDEN_SIG
> >>>  LF_COMPILED_SIG
> >>>  FORCEINLINE_SIG
> >>>  DONTINLINE_SIG
> >>>
> >>> Otherwise, looks fine.
> >>>
> >>> Best regards,
> >>> Vladimir Ivanov
> >>>
> >>> On 2/17/16 5:47 PM, shilpi rastogi wrote:
>  Hi All,
> 
>  Please review fix for the following bug-
> 
>  https://bugs.openjdk.java.net/browse/JDK-8144931
>  http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/
> 
> 
>  Thanks,
>  Shilpi
> >

Re: RFR [9] 8150163: JarFileSystem support for MRJARs should use the JDK specific Version API

[Alan Bateman]

On 18/02/2016 12:56, Chris Hegarty wrote:

To support MRJARs, currently JarFileSystem uses sun.misc.Version to retrieve the
major version. It should be updated to use the new JDK specific Version API.

http://cr.openjdk.java.net/~chegar/8150163/
https://bugs.openjdk.java.net/browse/JDK-8150163

This looks okay for now but I assume it may have to change again soon 
once a home for the new Version API is found.


-Alan.

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Vladimir Ivanov]

P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise 
broken descriptor strings (missing "L" or missing ";"):

I like how it shapes out with Type. Thanks, Uwe!

Best regards,
Vladimir Ivanov



 static boolean checkClassName(String cn) {
Type tp = Type.getType(cn);
// additional sanity so only valid "L;" descriptors work
if (tp.getSort() != Type.OBJECT) {
  return false;
}
try {
Class c = Class.forName(tp.getClassName(), false, null);
return true;
} catch (ClassNotFoundException e) {
return false;
}
}

-
Uwe Schindler
uschind...@apache.org
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/



-Original Message-
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
Behalf Of Paul Sandoz
Sent: Thursday, February 18, 2016 3:05 PM
Cc: Java Core Libs 
Subject: Re: RFR 8144931: Assert class signatures are correct and refer to valid
classes

Hi Uwe

Not a stupid question.

We took the conservative approach to preserve the existing costs (avoid
linkage and string generation).

Paul.


On 18 Feb 2016, at 14:54, Uwe Schindler  wrote:

Hi,

just a stupid question from somebody outside the OpenJDK developers:
You are already using ASM to generate the class files. Why not also use the

Type class in ASM to generate the signatures of a class constant?:


Instead of:

static final String  LF_HIDDEN_SIG =

className("Ljava/lang/invoke/LambdaForm$Hidden;");


Use the following to define the constant:

import jdk.internal.org.objectweb.asm.Type;
import java.lang.invoke.LambdaForm.Hidden;
static final String  LF_HIDDEN_SIG = Type.getDescriptor(Hidden.class);

This is compile-time checked, because of the .class notation.

Thanks,
Uwe

-
Uwe Schindler
uschind...@apache.org
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/


-Original Message-
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
Behalf Of shilpi.rast...@oracle.com
Sent: Thursday, February 18, 2016 12:18 PM
To: Vladimir Ivanov ; core-libs-
d...@openjdk.java.net
Subject: Re: RFR 8144931: Assert class signatures are correct and refer to

valid

classes

Thank You Vladimir!

I have done the changes. Please review the updated patch-

http://cr.openjdk.java.net/~srastogi/8144931/webrev.02/

Regards,
Shilpi

On 2/18/2016 1:58 PM, Vladimir Ivanov wrote:

Shilpi,

_CLASS suffix looks redundant and you can abbreviate LAMBDA_FORM

to

LF:

  LF_HIDDEN_SIG
  LF_COMPILED_SIG
  FORCEINLINE_SIG
  DONTINLINE_SIG

Otherwise, looks fine.

Best regards,
Vladimir Ivanov

On 2/17/16 5:47 PM, shilpi rastogi wrote:

Hi All,

Please review fix for the following bug-

https://bugs.openjdk.java.net/browse/JDK-8144931
http://cr.openjdk.java.net/~srastogi/8144931/webrev.01/


Thanks,
Shilpi

Re: JNI VERSION CHANGE: RFR: 8145098: JNI GetVersion should return JNI_VERSION_9

[Rachel Protacio]

Thanks for the reviews, David and Alan!
Rachel

On 2/18/2016 2:48 AM, Alan Bateman wrote:



On 17/02/2016 21:21, Rachel Protacio wrote:

Hello, everyone,

We are moving forward with "JNI_VERSION_9". If it later turns out 
that it should be "9_0", we will file a separate bug, but the plain 9 
is most likely. There is currently a compatibility request in the 
approval process.


Here is the updated code, which builds properly and still prints the 
correct version from my own private test.
hotspot repo webrev: 
http://cr.openjdk.java.net/~rprotacio/JNI_hotspot.01/

jdk repo webrev: http://cr.openjdk.java.net/~rprotacio/JNI_jdk.01/

This looks good to me.

Iris - I assume you'll add a section to JEP 223 for this, it seems 
like the right place to capture this.


-Alan

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[Paul Sandoz]

> On 18 Feb 2016, at 15:55, Vladimir Ivanov  
> wrote:
> 
>> P.S.: You could improve the assertion with the Type class, so it would also 
>> fail on otherwise broken descriptor strings (missing "L" or missing ";"):
> I like how it shapes out with Type. Thanks, Uwe!
> 

Me too!

Paul.

> Best regards,
> Vladimir Ivanov
> 
>> 
>> static boolean checkClassName(String cn) {
>>Type tp = Type.getType(cn);
>>// additional sanity so only valid "L;" descriptors work
>>if (tp.getSort() != Type.OBJECT) {
>>  return false;
>>}
>>try {
>>Class c = Class.forName(tp.getClassName(), false, null);
>>return true;
>>} catch (ClassNotFoundException e) {
>>return false;
>>}
>>}
>> 
>> -
>> Uwe Schindler
>> uschind...@apache.org
>> ASF Member, Apache Lucene PMC / Committer
>> Bremen, Germany
>> http://lucene.apache.org/

RFR [9] 8150162: Move sun.misc.Version to a truly internal package

[Chris Hegarty]

sun.misc.Version is the core libraries part of a private interface with the
JVM to query and set specific JVM version and capabilities, as well as
being responsible for setting the system properties for  "java.version”,
"java.runtime.version", and "java.runtime.name" ( which are generated
during the build ).

It is not a "Critical API", as defined by JEP 260, so should be moved
out of sun.misc and placed into a more appropriate package where it
can be encapsulated.

Version is only used by j.l.System during initialization, so I’ve taken the 
liberty of moving it into java.lang as package-private. It could, however,
be placed in jdk.internal.misc, if it is more generally useful. That said, 
most other usages should be able to use the JDK specific Version API.

http://cr.openjdk.java.net/~chegar/8150162/
https://bugs.openjdk.java.net/browse/JDK-8150162

-Chris.

P.S. 8150163 & 8150168 are in progress to replace unnecessary usages
of sun.misc.Version with the JDK specific Version.

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Steve Drach]

>> Thank you Alan.  I’ll address the issues you bring up before integration.
> Thanks. Are you planning to update the webrev too as it would be nice to see 
> the final javadoc?

http://cr.openjdk.java.net/~sdrach/8132734/webrev.07/index.html 

Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE

[huizhe wang]

On 2/18/2016 5:15 AM, Langer, Christoph wrote:

Hi Joe,

here is the next version:
http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/


Looks good!  Thanks.

Copyright year in XSAttributeChecker.java is still incorrect. I meant to 
say the year "2016" needs to have a comma, so in this case, it needs to 
be "2015, 2016, " rather than "2015, 2016".


Best,
Joe



Hopefully the final one :-)


Yes, it's good to have some cleanups. For the generic initialization,
it's preferable to have no redundant type arguments, for example,
instead of:

protected Stack fEntityStack = new Stack();

do:
protected Stack fEntityStack = new Stack<>();

Some of the "cleanup" in the webrev were reversing <> with added type
argument(s), that would be unnecessary.

I think I did that on all the places that I touched now.


For the obsolete Vector, it would be good to replace it with ArrayList.

There are so many Vector instances in this code that it's out of scope for me 
now to touch them all...


Copyright year "2016" in XSAttributeChecker needs to be "2016," or else
the script would complain.

For that one I thought that it would look like "2015,2016" if the initial 
copyright was done in 2015. But I changed it as you suggested.


In XSDHandler, note that there's another call to get SECURITY_MANAGER
towards the end of the reset method (at 3572 in your new version) that
may be removed. You may also consolidate most of those
setFeature/Property statements in one try-catch block if you want.

OK, I did some further refacturing here. However, I've left the try/catch 
blocks as I think it's the intention to silently catch some exceptions but then 
try to go on with the next property.


For the test, the @bug tag is still desirable in the general comment
section. For now, it's @bug 8149915. As we add more test to the class in
the future, we'd then append more bug ids, @bug 8149915, xxx.

Done. I also left the @bug in the comments for the test method.


I'm also wondering why some files have a copyright header like this, e.g.

src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11D
TDScannerImpl.java:

/*
   * reserved comment block
   * DO NOT REMOVE OR ALTER!
   */

Shouldn't there be the standard copyright header or is there some reason

behind it?

The block is used by the licensing script to swap with license
information required by licensees. For code that came from Xerces, we
should keep the block as is with the Apache License. But when we make
changes, we're required to put in the copyright header. Since the
changes you made are cleanup / removing unused fields, it's okay to keep
the block as is.

OK :)

Thanks again for reviewing.

All the Best
Christoph

RE: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature for xsd schema with annotation causes NPE

[Langer, Christoph]

Hi Joe,

I fixed the copyright in 
http://cr.openjdk.java.net/~clanger/webrevs/8149915.3/. So I think I'm done. 
Can you review and push the change then?

Thanks
Christoph


-Original Message-
From: huizhe wang [mailto:huizhe.w...@oracle.com] 
Sent: Donnerstag, 18. Februar 2016 19:19
To: Langer, Christoph 
Cc: core-libs-dev@openjdk.java.net
Subject: Re: JDK 9 RFR of JDK-8149915: enabling validate-annotations feature 
for xsd schema with annotation causes NPE


On 2/18/2016 5:15 AM, Langer, Christoph wrote:
> Hi Joe,
>
> here is the next version:
> http://cr.openjdk.java.net/~clanger/webrevs/8149915.2/

Looks good!  Thanks.

Copyright year in XSAttributeChecker.java is still incorrect. I meant to 
say the year "2016" needs to have a comma, so in this case, it needs to 
be "2015, 2016, " rather than "2015, 2016".

Best,
Joe

>
> Hopefully the final one :-)
>
>> Yes, it's good to have some cleanups. For the generic initialization,
>> it's preferable to have no redundant type arguments, for example,
>> instead of:
>>
>> protected Stack fEntityStack = new Stack();
>>
>> do:
>> protected Stack fEntityStack = new Stack<>();
>>
>> Some of the "cleanup" in the webrev were reversing <> with added type
>> argument(s), that would be unnecessary.
> I think I did that on all the places that I touched now.
>
>> For the obsolete Vector, it would be good to replace it with ArrayList.
> There are so many Vector instances in this code that it's out of scope for me 
> now to touch them all...
>
>> Copyright year "2016" in XSAttributeChecker needs to be "2016," or else
>> the script would complain.
> For that one I thought that it would look like "2015,2016" if the initial 
> copyright was done in 2015. But I changed it as you suggested.
>
>> In XSDHandler, note that there's another call to get SECURITY_MANAGER
>> towards the end of the reset method (at 3572 in your new version) that
>> may be removed. You may also consolidate most of those
>> setFeature/Property statements in one try-catch block if you want.
> OK, I did some further refacturing here. However, I've left the try/catch 
> blocks as I think it's the intention to silently catch some exceptions but 
> then try to go on with the next property.
>
>> For the test, the @bug tag is still desirable in the general comment
>> section. For now, it's @bug 8149915. As we add more test to the class in
>> the future, we'd then append more bug ids, @bug 8149915, xxx.
> Done. I also left the @bug in the comments for the test method.
>
>>> I'm also wondering why some files have a copyright header like this, e.g.
>> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XML11D
>> TDScannerImpl.java:
>>> /*
>>>* reserved comment block
>>>* DO NOT REMOVE OR ALTER!
>>>*/
>>>
>>> Shouldn't there be the standard copyright header or is there some reason
>> behind it?
>>
>> The block is used by the licensing script to swap with license
>> information required by licensees. For code that came from Xerces, we
>> should keep the block as is with the Apache License. But when we make
>> changes, we're required to put in the copyright header. Since the
>> changes you made are cleanup / removing unused fields, it's okay to keep
>> the block as is.
> OK :)
>
> Thanks again for reviewing.
>
> All the Best
> Christoph
>
>

Re: RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more

[Roger Riggs]

Hi Peter,

Nice to see this improvement.

I would support adding the drainQueue method to java.lang.ref.Cleaner; 
it provides
a mechanism to share the cleanup load (as in the case of Direct buffers) 
that may be useful

to other use cases.  It is preferable to hacking in to the CleanerImpl.

Making CleanerImpl implement Runnable is ok as long as CleanerImpl is 
limited to *only*
be the implementation of Cleaner and is not expected to be used directly 
even internal to the base module.


The conversions of lambdas to classes is necessary, unfortunately.

The names of the Cleaner threads could use the threadId instead of 
introducing a counter,
but probably not a big difference either way.  The numbers are not going 
to be significant.


Roger



On 2/17/2016 4:06 AM, Peter Levart wrote:

Hi Kim,

Thanks for looking into this. Answers inline...

On 02/17/2016 01:20 AM, Kim Barrett wrote:
On Feb 16, 2016, at 11:15 AM, Peter Levart  
wrote:


Hello everybody,

Thanks for looking into this and for all your comments. Now that it 
seems we have a consensus that replacing internal Cleaner with 
public Cleaner is not a wrong idea, I created an issue for it [1] so 
that we may officially review the implementation. I also created an 
updated webrev [2] which fixes some comments in usages that were not 
correct any more after the change. I also took the liberty to modify 
the CleanerImpl.InnocuousThreadFactory to give names to threads in 
accordance to the style used in some other thread factories 
(Timer-0, Timer-1, ..., Cleaner-0, Cleaner-1, ..., etc.). This gives 
the thread of internal Cleaner instance, which is now constructed as 
part of the boot-up sequence, a stable name: "Cleaner-0".


If you feel that internal Cleaner instance should have a Thread with 
MAX_PRIORITY, I can incorporate that too.


[1] https://bugs.openjdk.java.net/browse/JDK-8149925
[2] 
http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.03/


I re-ran the java/lang/ref and java/nio tests and all pass except 
two ignored tests:

I think some of the existing uses of sun.misc.Cleaner were really just
a matter of convenience and the previous lack of
java.lang.ref.Cleaner.  An example of this that I have personal
knowledge of the history for is the CallSite cleanup in
MethodHandleNatives.  It's good to see those converted.

As for the others, if nobody wants to defend the special handling by
the reference processing thread, I'm certainly happy to see it
removed. However, I recall Roger saying there were existing tests that
failed when certain uses of sun.misc.Cleaner were replaced with
java.lang.ref.Cleaner. I don't remember the details, but maybe Roger
does.


If the failing test was the following:

java/nio/Buffer/DirectBufferAllocTest.java

...then it has been taken care of (see Bits.java). All java/lang/ref 
and java/nio tests pass on my PC. Are there any internal Oracle tests 
that fail?




-- 


src/java.base/share/classes/java/lang/ref/Reference.java

After removal of the special handling of removing Cleaner objects from
the pending list, do we still need to catch OOMEs from the pending
list?  If not sure, feel free to defer that in another RFE for cleanup.


As you have already noticed it is still possible for a lock.wait() to 
throw OOME because of not being able to allocate InterruptedException.


The other place where OOME could still be thrown (and has not been 
fixed yet) is from sun.misc.Cleaner.clean() special handling. I even 
constructed a reproducer for it (see the last comment in JDK-8066859). 
So removing special handling of sun.misc.Cleaner would finally put the 
JDK-8066859 to the rest.




-- 


src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java

I don't understand why CleanerImpl needs to be changed to public in
order to provide access to the new drainQueue. Wouldn't it be better
to add Cleaner.drainQueue?


An interesting idea. But I don't know if such functionality is 
generally useful enough to commit to it in a public API.


I have another idea where java.lang.ref.Cleaner would use an Executor 
instead of ThreadFactory. The default would be an instance of a 
single-threaded executor per Cleaner instance, but if user passed in a 
ForkJoinPool, he could use it's method ForkJoinPool.awaitQuiescence() 
to help the executor's thread(s) do the cleaning.




Some of the other changes here don't seem related to the problem at
hand. Are these proposed miscellaneous cleanups? I'm specifically
looking at the CleanerCleanable class. And I'm not sure why the
thread's constructor argument is being changed, requiring CleanerImpl
to implement Runnable.


One thing that this change unfortunately requires is to get rid of 
lambdas and method references in the implementation and dependencies 
of java.land.ref.Cleaner API, because it gets used early in t

RE: RFR [9] 8150163: JarFileSystem support for MRJARs should use the JDK specific Version API

[Iris Clark]

Hi, Chris.

I think this change looks fine.

I've added a note to JDK-8144062 (module for jdk.Version), indicating that 
parts of this changeset may need to be updated depending on where jdk.Version 
lands.

Thanks,
iris

-Original Message-
From: Chris Hegarty 
Sent: Thursday, February 18, 2016 4:56 AM
To: core-libs-dev; Steve Drach
Subject: RFR [9] 8150163: JarFileSystem support for MRJARs should use the JDK 
specific Version API

To support MRJARs, currently JarFileSystem uses sun.misc.Version to retrieve 
the major version. It should be updated to use the new JDK specific Version 
API. 

http://cr.openjdk.java.net/~chegar/8150163/
https://bugs.openjdk.java.net/browse/JDK-8150163

-Chris.

JDK 9 RFR of 8150206: Demote LFMultiThreadCachingTest.java to tier 2 until resolution of JDK-8150014

[joe darcy]

Hello,

Right now, we are seeing some cross-platform intermittent failures of

java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java

as noted in bug JDK-8150014. Until that test instability is resolved, 
the test should be demoted from tier 1 to tier 2.


Please see the patch below.

(Circumstantial evidence points to the changes in JDK-8145485 being 
involved.)


-Joe

diff -r 3973fe856db2 test/TEST.groups
--- a/test/TEST.groupsWed Feb 17 12:47:35 2016 -0800
+++ b/test/TEST.groupsThu Feb 18 16:44:40 2016 -0800
@@ -29,6 +29,7 @@
 :jdk_lang \
 -java/lang/ProcessHandle/TreeTest.java \
 -java/util/zip/TestLocalTime.java \
+-java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java \
 :jdk_util \
 -java/util/WeakHashMap/GCDuringIteration.java \
 -java/util/concurrent/ThreadPoolExecutor/ConfigChanges.java \
@@ -39,6 +40,7 @@
 :jdk_math

 tier2 = \
+java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java \
 java/lang/ProcessHandle/TreeTest.java \
 java/util/zip/TestLocalTime.java \
 java/util/WeakHashMap/GCDuringIteration.java \

RE: JNI VERSION CHANGE: RFR: 8145098: JNI GetVersion should return JNI_VERSION_9

[Iris Clark]

Hi, Alan and Rachel.

Here's a proposal for the new text for JEP 223:

---

[ insert between existing sections "@since..." and "Mercurial..."]

JNI Version

The [JNI Specification][JNISpec] defines a constant representing the JNI 
version number.  The constant will drop the initial "1" and truncate trailing 
zeros.  Thus, for JDK 9, the constant will be 'JNI_VERSION_9'.

[JNISpec]: 
http://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#version_information

---

What bugid should be used for this CCC request?  What is the procedure for 
updating the JNI Spec itself?

Thanks,
iris

-Original Message-
From: Alan Bateman 
Sent: Wednesday, February 17, 2016 11:49 PM
To: Rachel Protacio; hotspot-runtime-...@openjdk.java.net; 
core-libs-dev@openjdk.java.net; Iris Clark
Subject: Re: JNI VERSION CHANGE: RFR: 8145098: JNI GetVersion should return 
JNI_VERSION_9



On 17/02/2016 21:21, Rachel Protacio wrote:
> Hello, everyone,
>
> We are moving forward with "JNI_VERSION_9". If it later turns out that 
> it should be "9_0", we will file a separate bug, but the plain 9 is 
> most likely. There is currently a compatibility request in the 
> approval process.
>
> Here is the updated code, which builds properly and still prints the 
> correct version from my own private test.
> hotspot repo webrev: 
> http://cr.openjdk.java.net/~rprotacio/JNI_hotspot.01/
> jdk repo webrev: http://cr.openjdk.java.net/~rprotacio/JNI_jdk.01/
This looks good to me.

Iris - I assume you'll add a section to JEP 223 for this, it seems like the 
right place to capture this.

-Alan

Re: RFR 8144931: Assert class signatures are correct and refer to valid classes

[shilpi.rast...@oracle.com]

Hi All,

Please see the updated webrev
http://cr.openjdk.java.net/~srastogi/8144931/webrev.03/

Regards,
Shilpi

On 2/18/2016 8:33 PM, Paul Sandoz wrote:

On 18 Feb 2016, at 15:55, Vladimir Ivanov  wrote:


P.S.: You could improve the assertion with the Type class, so it would also fail on otherwise 
broken descriptor strings (missing "L" or missing ";"):

I like how it shapes out with Type. Thanks, Uwe!


Me too!

Paul.


Best regards,
Vladimir Ivanov


 static boolean checkClassName(String cn) {
Type tp = Type.getType(cn);
// additional sanity so only valid "L;" descriptors work
if (tp.getSort() != Type.OBJECT) {
  return false;
}
try {
Class c = Class.forName(tp.getClassName(), false, null);
return true;
} catch (ClassNotFoundException e) {
return false;
}
}

-
Uwe Schindler
uschind...@apache.org
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

Re: JDK 9 RFR of 8150206: Demote LFMultiThreadCachingTest.java to tier 2 until resolution of JDK-8150014

[Martin Buchholz]

It's easy enough to get the test to pass by delambdafication.

But we may not want to fix the test because it's not clear whether
it's a jdk lambda bug or not, and we don't sweep bugs under the rug.

Maybe instead of a "tier2" bucket we really want a "test found a bug
that's not yet fixed" bucket?

On Thu, Feb 18, 2016 at 4:48 PM, joe darcy  wrote:
> Hello,
>
> Right now, we are seeing some cross-platform intermittent failures of
>
> java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java
>
> as noted in bug JDK-8150014. Until that test instability is resolved, the
> test should be demoted from tier 1 to tier 2.
>
> Please see the patch below.
>
> (Circumstantial evidence points to the changes in JDK-8145485 being
> involved.)
>
> -Joe
>
> diff -r 3973fe856db2 test/TEST.groups
> --- a/test/TEST.groupsWed Feb 17 12:47:35 2016 -0800
> +++ b/test/TEST.groupsThu Feb 18 16:44:40 2016 -0800
> @@ -29,6 +29,7 @@
>  :jdk_lang \
>  -java/lang/ProcessHandle/TreeTest.java \
>  -java/util/zip/TestLocalTime.java \
> +-java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java \
>  :jdk_util \
>  -java/util/WeakHashMap/GCDuringIteration.java \
>  -java/util/concurrent/ThreadPoolExecutor/ConfigChanges.java \
> @@ -39,6 +40,7 @@
>  :jdk_math
>
>  tier2 = \
> +java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java \
>  java/lang/ProcessHandle/TreeTest.java \
>  java/util/zip/TestLocalTime.java \
>  java/util/WeakHashMap/GCDuringIteration.java \
>

Re: JDK 9 RFR of 8150206: Demote LFMultiThreadCachingTest.java to tier 2 until resolution of JDK-8150014

[joe darcy]

Hi Martin,

On 2/18/2016 10:18 PM, Martin Buchholz wrote:

It's easy enough to get the test to pass by delambdafication.

But we may not want to fix the test because it's not clear whether
it's a jdk lambda bug or not, and we don't sweep bugs under the rug.

Maybe instead of a "tier2" bucket we really want a "test found a bug
that's not yet fixed" bucket?


Well, we have a bug for the test failure in JDK-8150014 and now this bug 
to demote the test from tier 1 to tier 2. The planned snapshot for the 
next promoted build is next Monday. I don't think we should knowingly 
let an intermittent tier 1 test failure escape to a promoted build.


Alternatives to achieve that goal include:

1) Demoting the test to tier 2 temporarily (If the cause of the problem 
is known, this is less helpful than if more data needed to be gathered.)


2) Putting the test on the problem list so it doesn't get run at all.

3) Changing the test so that is passes (delambdafication as you've 
suggested).


4) Changing / reverting the other libs code which triggered this failure.

The closest approach to "test found a bug that's not yet fixed" is 
putting the test on the problem list, option 2). I'm happy to do either 
1) or 2) under JDK-8150206 as an investigation into 3) or 4) proceeds 
over a longer period of time. I just don't think we should leave the 
test in its current flaky state for the next promotion.


Thanks,

-Joe



On Thu, Feb 18, 2016 at 4:48 PM, joe darcy  wrote:

Hello,

Right now, we are seeing some cross-platform intermittent failures of

 java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java

as noted in bug JDK-8150014. Until that test instability is resolved, the
test should be demoted from tier 1 to tier 2.

Please see the patch below.

(Circumstantial evidence points to the changes in JDK-8145485 being
involved.)

-Joe

diff -r 3973fe856db2 test/TEST.groups
--- a/test/TEST.groupsWed Feb 17 12:47:35 2016 -0800
+++ b/test/TEST.groupsThu Feb 18 16:44:40 2016 -0800
@@ -29,6 +29,7 @@
  :jdk_lang \
  -java/lang/ProcessHandle/TreeTest.java \
  -java/util/zip/TestLocalTime.java \
+-java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java \
  :jdk_util \
  -java/util/WeakHashMap/GCDuringIteration.java \
  -java/util/concurrent/ThreadPoolExecutor/ConfigChanges.java \
@@ -39,6 +40,7 @@
  :jdk_math

  tier2 = \
+java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java \
  java/lang/ProcessHandle/TreeTest.java \
  java/util/zip/TestLocalTime.java \
  java/util/WeakHashMap/GCDuringIteration.java \

Re: RFR 9: 8149750 Decouple sun.misc.Signal from the base module

[David Holmes]

On 18/02/2016 11:54 AM, David Holmes wrote:

On 18/02/2016 1:49 AM, Roger Riggs wrote:

Hi David,

On 2/17/2016 3:33 AM, David Holmes wrote:

Hi Roger,

On 17/02/2016 7:37 AM, Roger Riggs wrote:

Hi David,

Webrev updated in place:
http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/


Thanks for those tweaks.

I'm still perplexed by the test. We established that for USR2, PIPE
and XFSZ the handler is not actually invoked - yet you are testing
that case. ??

I'll have to dig a bit more.
When that test is run, the SignalHandler does get called with the
expected signal.
The test succeeds on Linux with jdk 8 and jdk 9.


In my local testing (with the original java.util.Signal changes) raise()
throws IllegalStateException for USR2, PIPE and XFSZ (even though
register succeeded).

If I can get the time I'll try applying these exact changes and testing
them.


And everything worked as you said. I'm now extremely perplexed.

David
-


Thanks,
David


Roger



David


On 2/14/2016 11:55 PM, David Holmes wrote:

Hi Roger,

A few mostly minor comments ...

On 13/02/2016 3:47 AM, Roger Riggs wrote:

Please review moving the functionality of sun.misc.Signal to
jdk.internal.misc and
creating a wrapper sun.misc.Signal for existing external uses.

+++ This time including the hotspot changes to update the target of
the
upcalls.

A new replacement API will be considered separately.

The update includes a test that passes with or without the changes.
(Except for an NPE instead of SEGV if null is passed).

Webrev:
   jdk: http://cr.openjdk.java.net/~rriggs/webrev-signal-9149750/


I take it we don't care about reorder files any more?

There does not seem to be any particular guidance about how/when to use
them on Solaris.
So it seemed as reasonable to remove the entries as to update them.


---

src/java.base/share/classes/sun/misc/Signal.java

The @since should not be changed to 9. You only changed the
implementation not the API. Conversely the renamed class should
arguably be @since 9 and not @since 1.2. But it is wrong to say
sun.misc.Signal is @since 9.

The new sun.misc.Signal is a wrapper around the original implementation
now in jdk.internal.misc.
I had modified the mercurial commands to rename sun.misc.Signal to
jdk.internal.misc to preserve the history
of the implementation.

I will correct sun.misc.Signal to retain the original @since and use
@since 9 for the jdk.internal.misc


The *Handler.of method name reads strangely to me - "for" would be
better but I presume that is not allowed.

The xx.of ()  pattern has been used recently.


---

src/java.base/share/classes/jdk/internal/misc/Signal.java

Not sure I understand the role of NativeSignalHandler any more - given
it can't actually be used ??

They are used to retain the previous handler and can be used to restore
that handler.
The ability to invoke the handler directly was removed as a
simplification.


---

test/sun/misc/SunMiscSignalTest.java

Based on our email discussion this test can not be testing that the
handler actually gets invoked - as we know it does not in some cases.
I have doubts that sun.misc.Signal ever intended to support all the
signals you are testing.

There were no tests for sun.misc.Signal.  I created the tests based on
the current implementations.
They can be updated to reflect current support for handle, raise, and
whether a signal handler is called
and of course -Xrs.  It should allow detection of unintended changes in
behavior.




hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-9149750/


Hotspot changes are fine.


Thanks, Roger


Thanks,
David
-


Issue:
https://bugs.openjdk.java.net/browse/JDK-8149750

Thanks, Roger

JEP 260: https://bugs.openjdk.java.net/browse/JDK-8132928