Re: RFR: 8162563: Fix double checked locking in System.console()

2016-07-27 Thread David Holmes

On 27/07/2016 11:39 PM, Martin Buchholz wrote:



On Tue, Jul 26, 2016 at 11:13 PM, David Holmes mailto:david.hol...@oracle.com>> wrote:

On 27/07/2016 3:45 PM, Martin Buchholz wrote:

Hi David,

I didn't even look at Console.java!  I just fixed the double-checked
locking in System.java.

Looking now at Console.cons, that is NOT safely statically
initialized:

SharedSecrets.setJavaIOAccess(new JavaIOAccess() {
public Console console() {
if (istty()) {
if (cons == null)
cons = new Console();
return cons;
}
return null;
}

(but it's currently safe because only System.java accesses it)


The above code is in a static initialization block.


Sure, but the code isn't run (yet), just made available to be run later
via JavaIOAccess when System.console() is called.


Ah! Right - my mistake, sorry.

David


Re: JDK 9 RFR of JDK-8162666: Mark ValidationWarningsTest.java as intermittently failing

2016-07-27 Thread Lance Andersen
+1

> On Jul 27, 2016, at 6:16 PM, Joseph D. Darcy  wrote:
> 
> The test
> 
> javax/xml/jaxp/unittest/common/ValidationWarningsTest.java
> 
> is known to intermittently fail (JDK-8150145) and should be marked 
> accordingly.
> 
> Please review the patch below with does this (and removes an unnecessary 
> space from the TEST.ROOT file).
> 
> Thanks,
> 
> -Joe
> 
> --- a/test/TEST.ROOTWed Jul 27 18:23:04 2016 +0300
> +++ b/test/TEST.ROOTWed Jul 27 15:07:26 2016 -0700
> @@ -15,7 +15,7 @@
> othervm.dirs=javax/xml/jaxp
> 
> # Group definitions
> -groups=TEST.groups
> +groups=TEST.groups
> 
> # Minimum jtreg version
> requiredVersion=4.2 b02
> diff -r 794a2f7c2bb6 
> test/javax/xml/jaxp/unittest/common/ValidationWarningsTest.java
> --- a/test/javax/xml/jaxp/unittest/common/ValidationWarningsTest.java Wed Jul 
> 27 18:23:04 2016 +0300
> +++ b/test/javax/xml/jaxp/unittest/common/ValidationWarningsTest.java Wed Jul 
> 27 15:07:26 2016 -0700
> @@ -38,8 +38,9 @@
> 
> /*
>  * @test
> + * @bug 8144593
> + * @key intermittent
>  * @modules javax.xml/com.sun.org.apache.xerces.internal.jaxp
> - * @bug 8144593
>  * @summary Check that warnings about unsupported properties from SAX
>  *  parsers are suppressed during the xml validation process.
>  */

 
  

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





JDK 9 RFR of JDK-8162666: Mark ValidationWarningsTest.java as intermittently failing

2016-07-27 Thread Joseph D. Darcy

 The test

javax/xml/jaxp/unittest/common/ValidationWarningsTest.java

is known to intermittently fail (JDK-8150145) and should be marked 
accordingly.


Please review the patch below with does this (and removes an unnecessary 
space from the TEST.ROOT file).


Thanks,

-Joe

--- a/test/TEST.ROOTWed Jul 27 18:23:04 2016 +0300
+++ b/test/TEST.ROOTWed Jul 27 15:07:26 2016 -0700
@@ -15,7 +15,7 @@
 othervm.dirs=javax/xml/jaxp

 # Group definitions
-groups=TEST.groups
+groups=TEST.groups

 # Minimum jtreg version
 requiredVersion=4.2 b02
diff -r 794a2f7c2bb6 
test/javax/xml/jaxp/unittest/common/ValidationWarningsTest.java
--- a/test/javax/xml/jaxp/unittest/common/ValidationWarningsTest.java 
Wed Jul 27 18:23:04 2016 +0300
+++ b/test/javax/xml/jaxp/unittest/common/ValidationWarningsTest.java 
Wed Jul 27 15:07:26 2016 -0700

@@ -38,8 +38,9 @@

 /*
  * @test
+ * @bug 8144593
+ * @key intermittent
  * @modules javax.xml/com.sun.org.apache.xerces.internal.jaxp
- * @bug 8144593
  * @summary Check that warnings about unsupported properties from SAX
  *  parsers are suppressed during the xml validation process.
  */


Re: RFR (JAXP) 8158084: Catalog API: JAXP XML Processor Support

2016-07-27 Thread Joe Wang
Thanks Lance!  Prabu believed that the JCK test should have been fixed 
by a previous patch. I'll send him a build to double-check.


Best,
Joe

On 7/27/16, 1:17 PM, Lance Andersen wrote:

Hi Joe,

This looks OK to me.


You might want to get the JCK test addressed before pushing your changes

Best
Lance
On Jul 25, 2016, at 12:44 AM, huizhe wang > wrote:


Hi,

This is an enhancement to support the Catalog API (javax.xml.catalog) 
throughout the JAXP processors. All of the CatalogFeatures are 
supported by the JAXP factories and processors (e.g. SAXParser), and 
corresponding System properties as specified in the CatalogFeatures. 
An additional javax.xml property "USE_CATALOG" is added to allow 
switching on/off the Catalog support for a processor or all of them 
(through the System properties or jaxp.properties).


The USE_CATALOG feature is on by default for all processors. The only 
thing needed then, to resolve external references with a catalog, is 
setting the path to the catalog on the processor. While the 
USE_CATALOG is on by default, the Catalog support is noninterference. 
As long as no catalog is set, the Catalog is mute. Catalog, even if 
one exists, will not interfere with custom resolvers, it is simply 
ignored if a custom resolver is present.



specdiff: 
http://cr.openjdk.java.net/~joehw/jdk9/8158084/specdiff/overview-summary.html 



webrev: http://cr.openjdk.java.net/~joehw/jdk9/8158084/webrev/ 




Tests: 140ish new test cases; all existing SQE/unit tests pass. JCK 
has a large number of failures due to one Catalog API test that left 
a configuration file behind. I'll discuss that with the JCK team.



Thanks,

Joe







Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 





Re: RFR (JAXP) 8158084: Catalog API: JAXP XML Processor Support

2016-07-27 Thread Lance Andersen
Hi Joe,

This looks OK to me.


You might want to get the JCK test addressed before pushing your changes

Best
Lance
> On Jul 25, 2016, at 12:44 AM, huizhe wang  wrote:
> 
> Hi,
> 
> This is an enhancement to support the Catalog API (javax.xml.catalog) 
> throughout the JAXP processors. All of the CatalogFeatures are supported by 
> the JAXP factories and processors (e.g. SAXParser), and corresponding System 
> properties as specified in the CatalogFeatures. An additional javax.xml 
> property "USE_CATALOG" is added to allow switching on/off the Catalog support 
> for a processor or all of them (through the System properties or 
> jaxp.properties).
> 
> The USE_CATALOG feature is on by default for all processors. The only thing 
> needed then, to resolve external references with a catalog, is setting the 
> path to the catalog on the processor. While the USE_CATALOG is on by default, 
> the Catalog support is noninterference. As long as no catalog is set, the 
> Catalog is mute. Catalog, even if one exists, will not interfere with custom 
> resolvers, it is simply ignored if a custom resolver is present.
> 
> 
> specdiff: 
> http://cr.openjdk.java.net/~joehw/jdk9/8158084/specdiff/overview-summary.html
> 
> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8158084/webrev/
> 
> 
> Tests: 140ish new test cases; all existing SQE/unit tests pass. JCK has a 
> large number of failures due to one Catalog API test that left a 
> configuration file behind. I'll discuss that with the JCK team.
> 
> 
> Thanks,
> 
> Joe
> 
> 
> 

 
  

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





Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-27 Thread Joe Wang



On 7/27/16, 2:27 AM, Frank Yuan wrote:

Hi Daniel

Would you like to have a look at the following changes before I finish all 
rework?
It shows runWithAllPerm, and the handling for user.dir property in 
FilePolicy.java. The code like runWithTmpPermission is still
kept, please ignore them, I will remove them finally.

diff -r 1bfe60e61bad test/javax/xml/jaxp/libs/jaxp/library/FilePolicy.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/javax/xml/jaxp/libs/jaxp/library/FilePolicy.java Wed Jul 27 
02:23:58 2016 -0700
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jaxp.library;
+
+import static jaxp.library.JAXPTestUtilities.getSystemProperty;
+
+import java.io.FilePermission;
+
+import org.testng.ITestContext;
+
+/**
+ * This policy can access local XML files.
+ */
+public class FilePolicy extends BasePolicy {
+
+@Override
+public void onStart(ITestContext arg0) {
+JAXPPolicyManager policyManager = 
JAXPPolicyManager.getJAXPPolicyManager(true);
+String userdir = getSystemProperty("user.dir");
+policyManager.addPermission(new FilePermission(userdir + "/-", "read, 
write, delete"));
+policyManager.addPermission(new FilePermission(System.getProperty("test.src") + 
"/-", "read"));
+policyManager.addPermission(new FilePermission(userdir, "read"));
+}
+}
diff -r 1bfe60e61bad 
test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java  Wed Jul 
27 02:23:58 2016 -0700
@@ -0,0 +1,312 @@
+/*
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jaxp.library;
+
+
+import java.lang.reflect.ReflectPermission;
+import java.security.CodeSource;
+import java.security.Permission;
+import java.security.PermissionCollection;
+import java.security.Permissions;
+import java.security.Policy;
+import java.security.ProtectionDomain;
+import java.security.SecurityPermission;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.PropertyPermission;
+import java.util.StringJoiner;
+
+
+/*
+ * This is a base class that every test class must extend if it needs to be run
+ * with security mode.
+ */
+public class JAXPPolicyManager {
+/*
+ * Backing up policy.
+ */
+private Policy policyBackup;
+
+/*
+ * Backing up security manager.
+ */
+private SecurityManager smBackup;
+
+/*
+ * Current policy.
+ */
+private TestPolicy policy = new TestPolicy();
+
+/*
+ * JAXPPolicyManager singleton.
+ */
+private static JAXPPolicyManager policyManager = null;
+
+/*
+ * Install a SecurityManager along with a default Policy to allow testNG to
+ * run when there is a security manager.
+ */
+private JAXPPolicyManager() {
+// Backing up policy and security manager for restore
+polic

RFR: 8160605: java/util/SplittableRandom/SplittableRandomTest.java failed with timeout

2016-07-27 Thread Martin Buchholz
Hi Paul,

This deletes the testng ports of jsr166 tck random tests.

If you can remember doing any work worth keeping, other than explicitly
testing -Djava.util.secureRandomSeed=true (which we are adding to our tck
tests), let me know.

http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-RandomTests/


Re: RFR: 8162439: Runtime.Version.parse needs fast-path for major versions

2016-07-27 Thread Claes Redestad

On 07/25/2016 08:01 PM, Iris Clark wrote:

Hi, Claes.


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

I think that this change looks good.  We provide a shortcut for the common case 
where only the major version number is of interest without introducing a new 
API.


Thanks! Any reviewer want to give this the go-ahead?



Note that this has a minor side-effect that invocations of the form 
Runtime.Version.parse("notANumber") will now throw NFE instead of IAE.  I don't 
think that this is a problem since NFE extends IAE and the order that the exceptions will 
be checked is intentionally unspecified.


Right, I considered the minor behavior difference and thought it would 
be considered OK and in accordance with the specification, and 
unproblematic since this API is as of yet unreleased.


As an aside: I do think specifying both NFE and IAE to be thrown is 
suspicious when it's not perfectly clear when which one is to be 
expected, since it invites to wrong code with strict dependencies on 
which is currently thrown when, even though it's unspecified, which 
would mean changing the implementation (like in this patch) would have 
some possibility of messing with compatibility. I wonder if we should 
simply specify IAE and drop NFE to allow greater flexibility for future 
implementations?


/Claes



Regards,
iris
(not a Reviewer)

-Original Message-
From: Claes Redestad
Sent: Saturday, July 23, 2016 9:24 AM
To: core-libs-dev@openjdk.java.net core-libs-dev
Subject: RFR: 8162439: Runtime.Version.parse needs fast-path for major versions

Hi,

please review this patch to address a startup regression due to use of
Runtime.Version.parse("8") etc in JarFile, as introduced by JDK-8150680.
This solution introduce a fast-path in case of what appears to be a single 
number is sent to Runtime.Version.parse to avoid initializing
Runtime.VersionBuilder:

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

Passes all existing tests.

Thanks!

/Claes




Re: RFR [9] 8160513: ClassNotFoundException sun.misc.GC when running Tomcat 9 with JDK 9

2016-07-27 Thread Chris Hegarty

On 27/07/16 16:13, Mark Sheppard wrote:

Hi Chris,

change looks good to me.


Thanks for looking at this Mark.


just to clarify, the ClassNotFoundException in this instance is due to
the migration of
sun.misc.GC to sun.rmi.transport.GC.


Correct.


from a more general scope,
the change to the latter is to relieve JreMemoryLeakPreventionListener
of the need to
perform reflective access on this GC class, and  hence mitigates any
potential memory leak JDK-8157570
posed in the Daemon thread of sun.rmi.transport.GC


Correct.  Once this change is in JDK 9 I will contact Tomcat and
propose that they remove the reflective access to sun.misc.GC, in
their JDK 9 trunk.

-Chris.


regards
Mark


On 26/07/2016 16:24, Chris Hegarty wrote:

The GC.Daemon thread has no need of any user defined class loader,
so should set its context class loader to null before starting, so as to
not inadvertently retain a reference to the creating thread’s context
class loader.

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

-Chris.

P.S. I added a detailed comment the JIRA issue, for those wondering
why Tomcat is running into this.




Re: RFR [9] 8160513: ClassNotFoundException sun.misc.GC when running Tomcat 9 with JDK 9

2016-07-27 Thread Mark Sheppard

Hi Chris,

change looks good to me.

just to clarify, the ClassNotFoundException in this instance is due to 
the migration of

sun.misc.GC to sun.rmi.transport.GC.

from a more general scope,
the change to the latter is to relieve JreMemoryLeakPreventionListener 
of the need to
perform reflective access on this GC class, and  hence mitigates any 
potential memory leak JDK-8157570

posed in the Daemon thread of sun.rmi.transport.GC

regards
Mark


On 26/07/2016 16:24, Chris Hegarty wrote:

The GC.Daemon thread has no need of any user defined class loader,
so should set its context class loader to null before starting, so as to
not inadvertently retain a reference to the creating thread’s context
class loader.

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

-Chris.

P.S. I added a detailed comment the JIRA issue, for those wondering
why Tomcat is running into this.




Re: RFR [9] 8160513: ClassNotFoundException sun.misc.GC when running Tomcat 9 with JDK 9

2016-07-27 Thread Daniel Fuchs

On 26/07/16 16:24, Chris Hegarty wrote:

The GC.Daemon thread has no need of any user defined class loader,
so should set its context class loader to null before starting, so as to
not inadvertently retain a reference to the creating thread’s context
class loader.

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


Looks good to me Chris.
Another possibility might be to use InnocuousThread?

best regards,

-- daniel



-Chris.

P.S. I added a detailed comment the JIRA issue, for those wondering
why Tomcat is running into this.





Re: RFR 9: JEP 290: Filter Incoming Serialization Data

2016-07-27 Thread Roger Riggs

Hi Daniel,

On 7/27/2016 9:15 AM, Daniel Fuchs wrote:

Hi Roger,

ObjectInputStream.java:

 179  * If a {@link #setObjectInputFilter(ObjectInputFilter) filter is 
set}


 184  * A {@link 
ObjectInputFilter.Config#setSerialFilter(ObjectInputFilter) 
process-wide filter}


these two lines should be using {@linkplain, not {@link.

right, will fix


 308 private ObjectInputFilter serialFilter;

This field is supposed to be set only once. We can't use final
because we may not know its value right at construction time, so
the setter tries to do its best to ensure that the field is not
changed after serialization has begun.
To improve that and make it more 'final-like' I would make this
field volatile and the setter synchronized.
I don't think this is necessary,  the initial value is set in the 
constructor and is therefore safely published.
OIS is not-thread safe and is used from a single thread.  The owning 
thread would set/replace the filter
before any deserialization occurs and calls to readObject/readUnshared 
would be in that same thread.


Thanks, Roger



best regards,

-- daniel


On 26/07/16 18:57, Roger Riggs wrote:

Hi,

Updated the specdiff and javadoc with SerializablePermission and misc
editorial cleanups.

SpecDiff:
http://cr.openjdk.java.net/~rriggs/filter-diffs/overview-summary.html

Javadoc (subset)
http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputStream.html 



http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputFilter.html 



http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/SerializablePermission.html 




Also, noticed that a filter could not distinguish between a reference to
an array class and
the callback to check the size of a zero length array (size == 0).
Modified the
range of the size to be positive when creating an array and otherwise
negative.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/

Roger


On 7/26/2016 12:34 PM, Roger Riggs wrote:

Hi Chris,

yes, its in the webrev, but I neglected to include it in the javadoc
and specdiff updates.

Thanks, Roger


On 7/26/2016 12:20 PM, Chris Hegarty wrote:

Another final thought that just occurred to me…

java.io.SerializablePermission will need its class-level javadoc
updated to
include the new permission target name.

-Chris.


On 25 Jul 2016, at 19:55, Roger Riggs  wrote:

Hi Chris,

Thanks for the review and comments,

Updates in place:

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/

SpecDiff:
http://cr.openjdk.java.net/~rriggs/filter-diffs/overview-summary.html

Javadoc (subset)
http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputStream.html 




http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputFilter.html 












Re: RFR: 8162563: Fix double checked locking in System.console()

2016-07-27 Thread Martin Buchholz
On Tue, Jul 26, 2016 at 11:13 PM, David Holmes 
wrote:

> On 27/07/2016 3:45 PM, Martin Buchholz wrote:
>
>> Hi David,
>>
>> I didn't even look at Console.java!  I just fixed the double-checked
>> locking in System.java.
>>
>> Looking now at Console.cons, that is NOT safely statically initialized:
>>
>> SharedSecrets.setJavaIOAccess(new JavaIOAccess() {
>> public Console console() {
>> if (istty()) {
>> if (cons == null)
>> cons = new Console();
>> return cons;
>> }
>> return null;
>> }
>>
>> (but it's currently safe because only System.java accesses it)
>>
>
> The above code is in a static initialization block.
>

Sure, but the code isn't run (yet), just made available to be run later via
JavaIOAccess when System.console() is called.


Re: RFR 9: JEP 290: Filter Incoming Serialization Data

2016-07-27 Thread Daniel Fuchs

Hi Roger,

ObjectInputStream.java:

 179  * If a {@link #setObjectInputFilter(ObjectInputFilter) filter is set}

 184  * A {@link 
ObjectInputFilter.Config#setSerialFilter(ObjectInputFilter) process-wide 
filter}


these two lines should be using {@linkplain, not {@link.

 308 private ObjectInputFilter serialFilter;

This field is supposed to be set only once. We can't use final
because we may not know its value right at construction time, so
the setter tries to do its best to ensure that the field is not
changed after serialization has begun.
To improve that and make it more 'final-like' I would make this
field volatile and the setter synchronized.

best regards,

-- daniel


On 26/07/16 18:57, Roger Riggs wrote:

Hi,

Updated the specdiff and javadoc with SerializablePermission and misc
editorial cleanups.

SpecDiff:
http://cr.openjdk.java.net/~rriggs/filter-diffs/overview-summary.html

Javadoc (subset)
http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputStream.html

http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputFilter.html

http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/SerializablePermission.html


Also, noticed that a filter could not distinguish between a reference to
an array class and
the callback to check the size of a zero length array (size == 0).
Modified the
range of the size to be positive when creating an array and otherwise
negative.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/

Roger


On 7/26/2016 12:34 PM, Roger Riggs wrote:

Hi Chris,

yes, its in the webrev, but I neglected to include it in the javadoc
and specdiff updates.

Thanks, Roger


On 7/26/2016 12:20 PM, Chris Hegarty wrote:

Another final thought that just occurred to me…

java.io.SerializablePermission will need its class-level javadoc
updated to
include the new permission target name.

-Chris.


On 25 Jul 2016, at 19:55, Roger Riggs  wrote:

Hi Chris,

Thanks for the review and comments,

Updates in place:

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-serial-filter-jdk9-8155760/

SpecDiff:
http://cr.openjdk.java.net/~rriggs/filter-diffs/overview-summary.html

Javadoc (subset)
http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputStream.html


http://cr.openjdk.java.net/~rriggs/filter-javadoc/java/io/ObjectInputFilter.html








Re: RFR: 6543126: Level.known can leak memory

2016-07-27 Thread Daniel Fuchs

Hi Chris,

On 27/07/16 11:17, Chris Hegarty wrote:

Hi Daniel,

On 25/07/16 19:10, Daniel Fuchs wrote:

Hi,

Please find below a fix for:

6543126: Level.known can leak memory
https://bugs.openjdk.java.net/browse/JDK-6543126

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


Since mirroredLevel is a strong reference to the same given Level,
in the case where the level's class is the JDK's Level.class, then
you do not need either of the reachabilityFence's.


Yes good catch.


With this change there is a race between checking the weak reference
and accessing the mirroredLevel, this is evident in findLevel(String).
I think this is benign, the code is racy, but not susceptible to any
issues resulting from this.


Yes - nothing would prevent the custom level Object (the
reference's target) to be garbaged collected just after the
mirrored level is returned, but we don't care.
So there would be no point in trying to ensure the reference is
not stale just before returning. That's acceptable.



Otherwise, the changes look good to me.


Thanks!



-Chris




Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-27 Thread Daniel Fuchs

Hi Frank,

Please see my comments inline.

On 27/07/16 10:27, Frank Yuan wrote:

Hi Daniel

Would you like to have a look at the following changes before I finish all 
rework?
It shows runWithAllPerm, and the handling for user.dir property in 
FilePolicy.java. The code like runWithTmpPermission is still
kept, please ignore them, I will remove them finally.

diff -r 1bfe60e61bad test/javax/xml/jaxp/libs/jaxp/library/FilePolicy.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/javax/xml/jaxp/libs/jaxp/library/FilePolicy.java Wed Jul 27 
02:23:58 2016 -0700
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jaxp.library;
+
+import static jaxp.library.JAXPTestUtilities.getSystemProperty;
+
+import java.io.FilePermission;
+
+import org.testng.ITestContext;
+
+/**
+ * This policy can access local XML files.
+ */
+public class FilePolicy extends BasePolicy {
+
+@Override
+public void onStart(ITestContext arg0) {
+JAXPPolicyManager policyManager = 
JAXPPolicyManager.getJAXPPolicyManager(true);
+String userdir = getSystemProperty("user.dir");
+policyManager.addPermission(new FilePermission(userdir + "/-", "read, 
write, delete"));


I see that FilePermission allows white spaces but I believe the
canonical form would be "read,write,delete";


+policyManager.addPermission(new FilePermission(System.getProperty("test.src") + 
"/-", "read"));
+policyManager.addPermission(new FilePermission(userdir, "read"));
+}
+}
diff -r 1bfe60e61bad 
test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java  Wed Jul 
27 02:23:58 2016 -0700
@@ -0,0 +1,312 @@


[...]


+private void setDefaultPermissions() {
+//Permissions to set security manager and policy
+addPermission(new SecurityPermission("getPolicy"));
+addPermission(new SecurityPermission("setPolicy"));
+addPermission(new RuntimePermission("setSecurityManager"));
+//Properties that jtreg and TestNG require
+addPermission(new PropertyPermission("testng.show.stack.frames", 
"read"));
+addPermission(new PropertyPermission("test.src", "read"));
+addPermission(new PropertyPermission("test.classes", "read"));
+addPermission(new PropertyPermission("dataproviderthreadcount", 
"read"));
+addPermission(new PropertyPermission("experimental", "read"));
+
+//addPermission(new PropertyPermission("fileStringBuffer", "read"));
+/*
+addPermission(new RuntimePermission("getClassLoader"));
+addPermission(new RuntimePermission("createClassLoader"));
+addPermission(new RuntimePermission("createSecurityManager"));
+addPermission(new RuntimePermission("modifyThread"));
+addPermission(new PropertyPermission("*", "read, write"));
+addPermission(new ReflectPermission("suppressAccessChecks"));
+addPermission(new RuntimePermission("setIO"));
+addPermission(new RuntimePermission("setContextClassLoader"));
+addPermission(new RuntimePermission("accessDeclaredMembers"));*/


Good to see these permissions are now commented: make sure to remove
the commented code in the final version.


+/*
+ * set allowAll in current thread context.
+ */
+void setAllowAll(boolean allow) {
+policy.setAllowAll(allow);
+}


I'd suggest to return the previous value of allowAll here.
This will be more convenient to restore the previous value
in the finally block.
Since setters usually don't return values you might name it e.g.

/**
 * Switches allowAll to the given value and return its
 * previous value in current thread context.
 * 
 * Example of use:
 * {@code
 *final boolean before = switchAllowAll(true);
 *try {
 * // do some privileged operation here
 *} finally {
 * // restore allowAll to

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

2016-07-27 Thread Oleg G. Barbashov


07.07.2016 23:32, Steve Drach пишет:

Hi,

Please review the following:

webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 

issue: https://bugs.openjdk.java.net/browse/JDK-8158295 


This changeset adds a multi-release jar validator to jar tool.  After the jar 
tool builds a multi-release jar, the potential resultant jar file is passed to 
the validator to assure that the jar file meets the minimal standards of a 
multi-release jar, in particular that versioned classes have the same api as 
base classes they override.  There are other checks, for example warning if two 
classes are identical.  If the jar file is invalid, it is not kept, so —create 
will not produce a jar file and —update will keep the input jar file.

Thanks
Steve


574 private boolean validate(String fname) {
 575 boolean valid = true;
 576 Validator validator = new Validator(this);
 577
 578 try (JarFile jf = new JarFile(fname)) {
 579 AtomicBoolean validHolder = new AtomicBoolean(valid);
 580 jf.stream()
 581 .filter(e -> !e.isDirectory())
 582 .filter(e -> !e.getName().equals(MANIFEST_NAME))
 583 .filter(e -> !e.getName().endsWith(MODULE_INFO))
 584 .sorted(entryComparator)
 585 .forEachOrdered(je -> {
 586 boolean b = validator.accept(je, jf);
 587 if (validHolder.get()) validHolder.set(b);
 588 });
 589 valid = validHolder.get();
 590 } catch (IOException e) {
 591 error(formatMsg2("error.validator.jarfile.exception", fname, 
e.getMessage()));
 592 valid = false;
 593 } catch (InvalidJarException e) {
 594 error(formatMsg("error.validator.bad.entry.name", 
e.getMessage()));
 595 valid = false;
 596 }
 597 return valid;
 598 }

(IMHO) Using of AtomicBoolean and forEachOrdered() for stateful validator here looks 
forced. It may be avoided by using regular iterator with "for" loop:

Stream sorted = jf.stream()
.filter(e -> !e.isDirectory())
.filter(e -> !e.getName().equals(MANIFEST_NAME))
.filter(e -> !e.getName().endsWith(MODULE_INFO))
.sorted(entryComparator);
for (Iterator iter = sorted.iterator(); iter.hasNext();) {
if (!validator.accept(iter.next(), jf)) {
valid = false;
}
}

or even better by storing "valid" state inside the Validator after turning it to 
regular Consumer:

try (JarFile jf = new JarFile(fname)) {
Validator validator = new Validator(this, jf);
jf.stream()
.filter(e -> !e.isDirectory())
.filter(e -> !e.getName().equals(MANIFEST_NAME))
.filter(e -> !e.getName().endsWith(MODULE_INFO))
.sorted(entryComparator)
.forEachOrdered(validator);
return validator.isValidated();
} catch (IOException e) {
error(formatMsg2("error.validator.jarfile.exception", fname, 
e.getMessage()));
return false;
} catch (NumberFormatException e) {
error(formatMsg("error.validator.bad.entry.name", e.getMessage()));
return false;
}

Moreover what is the reason to have an external loop over the jar's entries? 
Even if we will use several different filter sets in future it would be better 
to use it as optional parameter for Validator.



RFR 8162458 Buffer view implementations use incorrect offset for Unsafe access

2016-07-27 Thread Paul Sandoz
Hi,

I made an embarrassing mistake in the fix for

  https://bugs.openjdk.java.net/browse/JDK-8151163
  All Buffer implementations should leverage Unsafe unaligned accessors

The offset calculation for Unsafe access was incorrect, it’s easy to get 
confused because for heap buffers the offset is relative to the array, and for 
direct buffers the address (which can update for slices/duplicates). 
Disturbingly all existing tests were passing both for core and hotspot when i 
pushed to hs.

As a penance i wrote a combinatorial test for buffer views to navigate the 
twisty maze of heap/direct, aligned/unaligned, little/big endian for accessing 
binary data and views from the source buffer.

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8162458-byte-buffer-view-offset-access-incorrect/webrev/

(This may be a duplicate of [1]).

Test has been verified to fail with the existing code. Focused JPRT runs pass, 
but i will kick off core/hotspot runs later on today.

I will push to hs since that is where JDK-8151163 and it has not been 
integrated into jdk9/dev.

Paul.

[1] https://bugs.openjdk.java.net/browse/JDK-8159257
unsafe.cpp: assert(byte_offset < p_size) failed: Unsafe access: offset 32767 > 
object's size 16

For the test runtime/Unsafe/RangeCheck.java I can reproduce a crash in jdk9/dev 
which does not have JDK-8151163, and i can reproduce on jdk9/hs with this fix 
for JDK-8162458.


Re: RFR: 6543126: Level.known can leak memory

2016-07-27 Thread Chris Hegarty

Hi Daniel,

On 25/07/16 19:10, Daniel Fuchs wrote:

Hi,

Please find below a fix for:

6543126: Level.known can leak memory
https://bugs.openjdk.java.net/browse/JDK-6543126

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


Since mirroredLevel is a strong reference to the same given Level,
in the case where the level's class is the JDK's Level.class, then
you do not need either of the reachabilityFence's.

With this change there is a race between checking the weak reference
and accessing the mirroredLevel, this is evident in findLevel(String).
I think this is benign, the code is racy, but not susceptible to any
issues resulting from this.

Otherwise, the changes look good to me.

-Chris


RE: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

2016-07-27 Thread Frank Yuan
Hi Daniel

Would you like to have a look at the following changes before I finish all 
rework? 
It shows runWithAllPerm, and the handling for user.dir property in 
FilePolicy.java. The code like runWithTmpPermission is still
kept, please ignore them, I will remove them finally.

diff -r 1bfe60e61bad test/javax/xml/jaxp/libs/jaxp/library/FilePolicy.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/javax/xml/jaxp/libs/jaxp/library/FilePolicy.java Wed Jul 27 
02:23:58 2016 -0700
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jaxp.library;
+
+import static jaxp.library.JAXPTestUtilities.getSystemProperty;
+
+import java.io.FilePermission;
+
+import org.testng.ITestContext;
+
+/**
+ * This policy can access local XML files.
+ */
+public class FilePolicy extends BasePolicy {
+
+@Override
+public void onStart(ITestContext arg0) {
+JAXPPolicyManager policyManager = 
JAXPPolicyManager.getJAXPPolicyManager(true);
+String userdir = getSystemProperty("user.dir");
+policyManager.addPermission(new FilePermission(userdir + "/-", "read, 
write, delete"));
+policyManager.addPermission(new 
FilePermission(System.getProperty("test.src") + "/-", "read"));
+policyManager.addPermission(new FilePermission(userdir, "read"));
+}
+}
diff -r 1bfe60e61bad 
test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java  Wed Jul 
27 02:23:58 2016 -0700
@@ -0,0 +1,312 @@
+/*
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jaxp.library;
+
+
+import java.lang.reflect.ReflectPermission;
+import java.security.CodeSource;
+import java.security.Permission;
+import java.security.PermissionCollection;
+import java.security.Permissions;
+import java.security.Policy;
+import java.security.ProtectionDomain;
+import java.security.SecurityPermission;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.PropertyPermission;
+import java.util.StringJoiner;
+
+
+/*
+ * This is a base class that every test class must extend if it needs to be run
+ * with security mode.
+ */
+public class JAXPPolicyManager {
+/*
+ * Backing up policy.
+ */
+private Policy policyBackup;
+
+/*
+ * Backing up security manager.
+ */
+private SecurityManager smBackup;
+
+/*
+ * Current policy.
+ */
+private TestPolicy policy = new TestPolicy();
+
+/*
+ * JAXPPolicyManager singleton.
+ */
+private static JAXPPolicyManager policyManager = null;
+
+/*
+ * Install a SecurityManager along with a default Policy to allow testNG to
+ * run when there is a security manager.
+ */
+private JAXPPolicyManager() {
+// Backing up policy and security manager for restore
+policyBackup = Policy.getPolicy();
+smB