Re: Simple 1-line fix for 8027943 to fix serial version of com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImp
+1 On Nov 7, 2013, at 4:59 PM, Mandy Chung wrote: This reverts com.sun.corba.se.spi.orbutil.proxy.CompositeInvocationHandlerImpl back to the previous serial version. diff --git a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java --- a/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java +++ b/src/share/classes/com/sun/corba/se/spi/orbutil/proxy/CompositeInvocationHandlerImpl.java @@ -89,4 +89,6 @@ sm.checkPermission(perm); } } + +private static final long serialVersionUID = 4571178305984833743L; } Webrev: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027943/webrev.00/ Mandy 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 doc only typo in java.io.DataInput
+1 On Nov 7, 2013, at 5:29 PM, roger riggs wrote: Please review this straightforward typo correction: Webrev: http://cr.openjdk.java.net/~rriggs/webrev-doc-readlong-8024458/ Thanks, Roger 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: JDK RFR to clean-up lint warnings in reflection implementation
looks fine Joe
On Nov 8, 2013, at 2:40 PM, Joe Darcy wrote:
Hello,
Please review the simple patch below which addresses a handful of raw types
lint warning in the core reflection implementation code.
(If memory serves, this code dates back from a time during the development of
JDK 5 when wildcards could not be used with arrays; before the release
shipped, that combination was allowed.)
Thanks,
-Joe
diff -r 46982ca895b4
src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java
---
a/src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java
Fri Nov 08 18:54:29 2013 +
+++
b/src/share/classes/sun/reflect/generics/reflectiveObjects/ParameterizedTypeImpl.java
Fri Nov 08 11:37:54 2013 -0800
@@ -52,7 +52,7 @@
}
private void validateConstructorArguments() {
-TypeVariable/*?*/[] formals = rawType.getTypeParameters();
+TypeVariable?[] formals = rawType.getTypeParameters();
// check correct arity of actual type args
if (formals.length != actualTypeArguments.length){
throw new MalformedParameterizedTypeException();
diff -r 46982ca895b4
src/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java
---
a/src/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java
Fri Nov 08 18:54:29 2013 +
+++
b/src/share/classes/sun/reflect/generics/repository/GenericDeclRepository.java
Fri Nov 08 11:37:54 2013 -0800
@@ -42,7 +42,7 @@
public abstract class GenericDeclRepositoryS extends Signature
extends AbstractRepositoryS {
-private TypeVariable[] typeParams; // caches the formal type parameters
+private TypeVariable?[] typeParams; // caches the formal type
parameters
protected GenericDeclRepository(String rawSig, GenericsFactory f) {
super(rawSig, f);
@@ -64,7 +64,7 @@
* Return the formal type parameters of this generic declaration.
* @return the formal type parameters of this generic declaration
*/
-public TypeVariable/*?*/[] getTypeParameters(){
+public TypeVariable?[] getTypeParameters(){
if (typeParams == null) { // lazily initialize type parameters
// first, extract type parameter subtree(s) from AST
FormalTypeParameter[] ftps = getTree().getFormalTypeParameters();
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: JDK 8 RFR: more core libs raw warnings cleanup
Looks Ok Joe
On Nov 12, 2013, at 4:28 AM, Joe Darcy wrote:
Hello,
Please review the patch below which would remove another batch of raw type
javac lint warnings from the core libraries.
No signatures of public or protected methods in the Java SE specification
have been modified. Regression tests in affected areas pass.
Thanks,
-Joe
diff -r 9fcb07df1c92 src/share/classes/java/io/ObjectOutputStream.java
--- a/src/share/classes/java/io/ObjectOutputStream.javaSat Nov 09
04:21:28 2013 +0400
+++ b/src/share/classes/java/io/ObjectOutputStream.javaTue Nov 12
00:51:15 2013 -0800
@@ -1248,7 +1248,7 @@
handles.assign(unshared ? null : desc);
Class? cl = desc.forClass();
-Class[] ifaces = cl.getInterfaces();
+Class?[] ifaces = cl.getInterfaces();
bout.writeInt(ifaces.length);
for (int i = 0; i ifaces.length; i++) {
bout.writeUTF(ifaces[i].getName());
diff -r 9fcb07df1c92 src/share/classes/java/io/ObjectStreamClass.java
--- a/src/share/classes/java/io/ObjectStreamClass.javaSat Nov 09 04:21:28
2013 +0400
+++ b/src/share/classes/java/io/ObjectStreamClass.javaTue Nov 12 00:51:15
2013 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2011, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1996, 2013, 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
@@ -1746,7 +1746,7 @@
dout.writeUTF(()V);
}
-Constructor[] cons = cl.getDeclaredConstructors();
+Constructor?[] cons = cl.getDeclaredConstructors();
MemberSignature[] consSigs = new MemberSignature[cons.length];
for (int i = 0; i cons.length; i++) {
consSigs[i] = new MemberSignature(cons[i]);
diff -r 9fcb07df1c92 src/share/classes/java/lang/reflect/Proxy.java
--- a/src/share/classes/java/lang/reflect/Proxy.javaSat Nov 09 04:21:28
2013 +0400
+++ b/src/share/classes/java/lang/reflect/Proxy.javaTue Nov 12 00:51:15
2013 -0800
@@ -494,9 +494,10 @@
private final int hash;
private final WeakReferenceClass?[] refs;
+@SuppressWarnings({rawtypes, unchecked})
KeyX(Class?[] interfaces) {
hash = Arrays.hashCode(interfaces);
-refs = new WeakReference[interfaces.length];
+refs = (WeakReferenceClass?[])new
WeakReference[interfaces.length];
for (int i = 0; i interfaces.length; i++) {
refs[i] = new WeakReference(interfaces[i]);
}
diff -r 9fcb07df1c92 src/share/classes/java/nio/file/TempFileHelper.java
--- a/src/share/classes/java/nio/file/TempFileHelper.javaSat Nov 09
04:21:28 2013 +0400
+++ b/src/share/classes/java/nio/file/TempFileHelper.javaTue Nov 12
00:51:15 2013 -0800
@@ -81,7 +81,7 @@
String prefix,
String suffix,
boolean createDirectory,
- FileAttribute[] attrs)
+ FileAttribute?[] attrs)
throws IOException
{
if (prefix == null)
@@ -155,7 +155,7 @@
static Path createTempFile(Path dir,
String prefix,
String suffix,
- FileAttribute[] attrs)
+ FileAttribute?[] attrs)
throws IOException
{
return create(dir, prefix, suffix, false, attrs);
@@ -167,7 +167,7 @@
*/
static Path createTempDirectory(Path dir,
String prefix,
-FileAttribute[] attrs)
+FileAttribute?[] attrs)
throws IOException
{
return create(dir, prefix, null, true, attrs);
diff -r 9fcb07df1c92 src/share/classes/java/util/IdentityHashMap.java
--- a/src/share/classes/java/util/IdentityHashMap.javaSat Nov 09 04:21:28
2013 +0400
+++ b/src/share/classes/java/util/IdentityHashMap.javaTue Nov 12 00:51:15
2013 -0800
@@ -1243,7 +1243,7 @@
if (ti = size) {
throw new ConcurrentModificationException();
}
-a[ti++] = (T) new
AbstractMap.SimpleEntry(unmaskNull(key), tab[si + 1]);
+a[ti++] = (T) new
AbstractMap.SimpleEntry(unmaskNull(key), tab[si + 1]);
}
}
// fewer elements than expected or concurrent modification from
other thread detected
diff -r 9fcb07df1c92 src/share/classes/java/util/logging/Logger.java
--- a/src/share/classes/java/util/logging/Logger.javaSat Nov 09 04:21:28
2013 +0400
+++ b/src/share/classes/java/util/logging/Logger.javaTue Nov 12
Re: RFR(2): 7174936: several String methods claim to always create new String
The wording changes seem fine to me. Thanks for the specdiff as it made it much easier to review On Nov 12, 2013, at 11:43 AM, Stuart Marks wrote: Hi all, Here's an updated version of the String spec change. Changes from the previous version address comments made by Brent Christian and David Holmes. Specifically: - Specify copyValueOf() overloads as equivalent to corresponding valueOf() overloads. - Remove extranous changes to subSequence() method - Clarify wording of concat() method doc. Bug report: https://bugs.openjdk.java.net/browse/JDK-7174936 Webrev: http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.1/ Specdiff: http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.1/overview-summary.html Thanks! s'marks On 11/7/13 2:31 AM, Stuart Marks wrote: Hi all, Please review the following spec changes to java.lang.String. In several places the specs mention returning new strings. This is overspecified; it's sufficient to return a string that satisfies the required properties. In some cases the current implementation doesn't create a new string -- for example, substring(0) returns 'this' -- which is strictly a violation of the spec. The solution is to relax the spec requirement to create new strings. Also, this change cleans up the specs for the copyValueOf() overloads to make them identical to the corresponding valueOf() overloads. Previously, they were gratuitously different. I think that some time in the dim, distant past, probably before JDK 1.0, they had different semantics, but now they're identical. The specs should say they're identical. This change is spec only, no code changes. Bug report: https://bugs.openjdk.java.net/browse/jdk-7174936 Webrev: http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.0/ Specdiff: http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.0/overview-summary.html Thanks! s'marks 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: Review request for 8028234: Remove unused methods in sun.misc.JavaAWTAccess
+1 On Nov 12, 2013, at 2:29 PM, Mandy Chung wrote: This is a simple code deletion in sun.misc.JavaAWTAccess and its implementation class: Webrev: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8028234/webrev.00/ This patch removes the methods from sun.misc.JavaAWTAccess that are no longer used. The only dependency remaining from core-libs to AppContext is the ability to obtain an opaque unique object to represent an applet context ifit is running in an applet environment. thanks Mandy 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): 8028111 : XML readers share the same entity expansion counter
looks fine joe On Nov 13, 2013, at 3:02 PM, huizhe wang wrote: Hi, The issue is that the limits applied to each processing process rather than each file processing. This applies to not only StAX as reported, but also other parsers and validators. The fix is to add reset to XMLSecurityManager and call it upon each file processing. XSLT Transform is verified fixed as the underlying parsers are fixed. webrev: http://cr.openjdk.java.net/~joehw/jdk8/8028111/webrev/ 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: JDK-8028631 - Improve the test coverage to the pathname handling on unix-like platforms
The changes seem OK. I did not run the tests though On Nov 19, 2013, at 2:08 PM, Dan Xu wrote: Hi All, We have java/io/pathNames/GeneralWin32.java testcase to do the general exhaustive test of pathname handling on windows. I am adding a new test GeneralSolaris.java to test the pathname handling in unix-like platforms. In the changes, I also make sure the test can run successfully in concurrency mode. Please help review it. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8028631 Webrev: http://cr.openjdk.java.net/~dxu/8028631/webrev/ -Dan 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: 8028638: java/rmi/activation/Activatable/checkRegisterInLog/CheckRegisterInLog.java fails
looks fine On Nov 19, 2013, at 5:24 PM, Stuart Marks wrote: Hi all, Please review this small fix for an intermittent timeout. Nothing seems to be going wrong except that if the machine running the test is exceptionally slow, spurious timeouts will occur. The solution is to raise the timeout in the RMI test infrastructure. Bug: https://bugs.openjdk.java.net/browse/JDK-8028638 Diff: (appended below) Thanks, s'marks # HG changeset patch # User smarks # Date 1384899795 28800 # Node ID ebbfb1b45a4e6b37d339942568a662268dcb18fe # Parent 67d742c759717ca17518aaadb17725cac85c5897 8028638: java/rmi/activation/Activatable/checkRegisterInLog/CheckRegisterInLog.java fails Reviewed-by: XXX diff -r 67d742c75971 -r ebbfb1b45a4e test/java/rmi/testlibrary/RMID.java --- a/test/java/rmi/testlibrary/RMID.java Tue Nov 19 20:10:58 2013 +0100 +++ b/test/java/rmi/testlibrary/RMID.java Tue Nov 19 14:23:15 2013 -0800 @@ -74,6 +74,10 @@ // + // -Djava.security.debug=all ; +// Set execTimeout to 60 sec (default is 30 sec) +// to avoid spurious timeouts on slow machines. +options += -Dsun.rmi.activation.execTimeout=6; + return options; } 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) 8028822 : Error in the documentation for newFactory method of the javax.xml.stream factories
looks OK Joe On Nov 26, 2013, at 5:27 PM, huizhe wang wrote: On 11/26/2013 1:59 PM, roger riggs wrote: Hi, I looked at that twice also.java.time had a similar situation. To get to the TCCL you need to call ServiceLoader.load(type). The FactoryFinder:348 findServiceProvider method should test if cl == null and call the original ServiceLoader function. Yes. I forgot that part. load(service, loader) treats null as the system class loader was one of the topics in the discussion of the spec. Joe Roger On 11/26/2013 4:37 PM, Alan Bateman wrote: On 26/11/2013 18:38, huizhe wang wrote: Hi all, Here's revised webrev, as Alan suggested, including the implementation. http://cr.openjdk.java.net/~joehw/jdk8/jaxp16MR/8028822/webrev/ So is this handling the null case correctly? It is spec'ed to use the TCCL but it looks like it's going through to ServiceLoader.load where null means the system class loader. Maybe this needs to wait for tests as that it really the only way to check this. -Alan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request 8029417: Minor javadoc cleanup for JDBC 4.2
Hi all This is a review request for some minor javadoc clarifications for JDBC 4.2 based on some feedback that I received. The webrev can be found http://cr.openjdk.java.net/~lancea/8029417/webrev.00 Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
looks OK
On Dec 3, 2013, at 8:40 PM, Stuart Marks wrote:
Hi all,
Please review the following small javadoc change. The StringJoiner doc for a
couple methods uses i.e. in the first sentence, which screws up the javadoc
logic that pulls the first sentence into the Method Summary. This is an
editorial change to fix this up; there is no actual specification change.
Thanks!
s'marks
# HG changeset patch
# User smarks
# Date 1386121046 28800
# Node ID 0bc91b9f6afd9b4dceea31ecd1036e367b365690
# Parent 75165f6c1c505ff43f7fd235a95b2e7955413b78
8029489: StringJoiner spec for setEmptyValue() and length() is malformatted
Reviewed-by: XXX
diff -r 75165f6c1c50 -r 0bc91b9f6afd
src/share/classes/java/util/StringJoiner.java
--- a/src/share/classes/java/util/StringJoiner.java Tue Dec 03 17:25:28
2013 -0800
+++ b/src/share/classes/java/util/StringJoiner.java Tue Dec 03 17:37:26
2013 -0800
@@ -131,7 +131,7 @@
/**
* Sets the sequence of characters to be used when determining the string
* representation of this {@code StringJoiner} and no elements have been
- * added yet, i.e. when it is empty. A copy of the {@code emptyValue}
+ * added yet, that is, when it is empty. A copy of the {@code
emptyValue}
* parameter is made for this purpose. Note that once an add method has
been
* called, the {@code StringJoiner} is no longer considered empty, even if
* the element(s) added correspond to the empty {@code String}.
@@ -228,8 +228,8 @@
}
/**
- * The length of the {@code StringJoiner} value, i.e. the length of
- * {@code String} representation of the {@code StringJoiner}. Note that
if
+ * Returns the length of the {@code String} representation
+ * of this {@code StringJoiner}. Note that if
* no add methods have been called, then the length of the {@code String}
* representation (either {@code prefix + suffix} or {@code emptyValue})
* will be returned. The value should be equivalent to
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Re: RFR: JDK-8028712 : Tidy warnings cleanup for java.sql package
Hi Serge This looks OK. For --- old/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.587885460 + +++ new/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.435885464 + Please remove the following Package Specification • Specification of the JDBC 4.0 API Related Documentation • Getting Started--overviews of the major interfaces • Chapters on the JDBC API--from the online version of The Java Tutorial Continued • JDBCTMAPI Tutorial and Reference, Third Edition-- a complete reference and tutorial for the JDBC 3.0 API The above links keep breaking now that we are off of java.sun.com And I agree with roger, please use br Alan, yes I can look to clean up some of the formatting crud for Java SE 9 once we have access to the workspace Thank you for doing this Serge. Best Lance On Dec 5, 2013, at 10:31 AM, Alan Bateman wrote: On 05/12/2013 17:25, Serge wrote: Hi all, please review the fix http://cr.openjdk.java.net/~yan/8028712/webrev.02/ http://cr.openjdk.java.net/%7Eyan/8028712/webrev.02/ for https://bugs.openjdk.java.net/browse/JDK-8028712 This patch cleanup tidy warnings for generated html documentation, and do not affect the appearance of the documentation. The removal of the p tags seems okay. The only thing that I'm not sure about the addition of br/ to the package docs (is that needed?). Lance - while scanning this patch then it appears that some of the formatting in the javadoc comments is all over the place (java.sql.Connection is one example). It might be good to clean that up once the jdk9 project is open for business. -Alan 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) 8029895 : XMLOutputFactory.newFactory(String, ClassLoader) - incorrect specification
looks fine joe On Dec 11, 2013, at 4:10 PM, huizhe wang wrote: On 12/11/2013 12:21 PM, Alan Bateman wrote: On 11/12/2013 19:52, huizhe wang wrote: Hi, This is a quick documentation change to fix an error in javax.xml.stream.XMLOutputFactory: http://cr.openjdk.java.net/~joehw/jdk8/jaxp16MR/8029895/webrev/ Thanks, Joe It looks okay although I think it could be reduced to something like The original method was incorrectly defined to return XMLInputFactory. Updated to use your statement: http://cr.openjdk.java.net/~joehw/jdk8/jaxp16MR/8029895/webrev/ -Joe -Alan. 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) 8029955 : AIOB in XMLEntityScanner.scanLiteral upon parsing literals with 100 LF chars
Joe, I thought this looked OK also On Dec 17, 2013, at 12:26 PM, huizhe wang wrote: On 12/17/2013 4:10 AM, Daniel Fuchs wrote: Hi Joe, The fix looks good - though I wonder at whether incrementing whiteSpaceLookup by a fix amount wouldn't be better than doubling its length. Both would be okay. The case, as demonstrated in the report, that there are hundreds of LFs in an attribute, is very rare. Best, Joe best regards, -- daniel On 12/16/13 8:31 PM, huizhe wang wrote: Hi, This is a quick fix for a whitespace buffer that was not adjusted properly in one of the two cases. The buffer, whiteSpaceLookup, is filled in two cases and adjusted properly the 2nd time. The code is moved into a method storeWhiteSpace so that it's shared for the 1st case as well. Note at line 1175, there is no need to save character 0x20 since all whitespace characters will later be replaced with character 0x20. webrevs: http://cr.openjdk.java.net/~joehw/jdk8/8029955/webrev/ 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: JDK-8028712 : Tidy warnings cleanup for java.sql package
Hi Serge, On Dec 20, 2013, at 9:16 AM, Serge wrote: Hi all. Please review a second fix http://cr.openjdk.java.net/~yan/8028712/webrev.03/ for https://bugs.openjdk.java.net/browse/JDK-8028712 I deleted part of java/sql/package.html, I did not see the section below deleted, perhaps the change did not get pushed to where you generated the webrev from? and replaced br/ to br for compliance with html 3.2 On 12/05/2013 10:39 PM, Lance Andersen - Oracle wrote: Hi Serge This looks OK. For --- old/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.587885460 + +++ new/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.435885464 + Please remove the following Package Specification . Specification of the JDBC 4.0 API Related Documentation . Getting Started--overviews of the major interfaces . Chapters on the JDBC API--from the online version of The Java Tutorial Continued . JDBCTMAPI Tutorial and Reference, Third Edition-- a complete reference and tutorial for the JDBC 3.0 API The above links keep breaking now that we are off of java.sun.com And I agree with roger, please use br Alan, yes I can look to clean up some of the formatting crud for Java SE 9 once we have access to the workspace Thank you for doing this Serge. Best Lance On Dec 5, 2013, at 10:31 AM, Alan Bateman wrote: On 05/12/2013 17:25, Serge wrote: Hi all, please review the fix http://cr.openjdk.java.net/~yan/8028712/webrev.02/ http://cr.openjdk.java.net/%7Eyan/8028712/webrev.02/ for https://bugs.openjdk.java.net/browse/JDK-8028712 This patch cleanup tidy warnings for generated html documentation, and do not affect the appearance of the documentation. The removal of the p tags seems okay. The only thing that I'm not sure about the addition of br/ to the package docs (is that needed?). Lance - while scanning this patch then it appears that some of the formatting in the javadoc comments is all over the place (java.sql.Connection is one example). It might be good to clean that up once the jdk9 project is open for business. -Alan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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 java.time.Duration spec correction (8031103)
looks fine Roger On Jan 6, 2014, at 2:09 PM, roger riggs wrote: Please review this minor specification correction to the java.time.Duration.toDays() and toHours() methods. Only the javadoc is corrected, no code or tests are affected. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-duration-javadoc-8031103/ Thanks, Roger JDK-8031103 https://bugs.openjdk.java.net/browse/JDK-8031103 java.time.Duration has wrong Javadoc Comments in toDays() and toHours() 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: JDK 9 RFR of JDK-8027063 SecurityManger.getClassContext returns a raw type
+1 On Jan 6, 2014, at 3:53 PM, Joe Darcy wrote: Hello, Please review the simple change to fix JDK-8027063 SecurityManger.getClassContext returns a raw type, which changes a signature of a protected method in SecurityManger to remove a use of raw types in the core libraries: --- a/src/share/classes/java/lang/SecurityManager.javaMon Jan 06 11:48:32 2014 -0800 +++ b/src/share/classes/java/lang/SecurityManager.javaMon Jan 06 12:51:52 2014 -0800 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1995, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1995, 2014, 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 @@ -307,7 +307,7 @@ * * @return the execution stack. */ -protected native Class[] getClassContext(); +protected native Class?[] getClassContext(); /** * Returns the class loader of the most recently executing method from A clean build succeeds and the SecurityManager tests pass after this change. 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: JDK 9 RFR of JDK-8031210 Remove serial warning from java.lang.Enum
+1
On Jan 6, 2014, at 4:41 PM, Joe Darcy wrote:
Hello,
Please review the patch below to add a @SuppressWarning(serial) to
java.lang.Enum to resolve a lint warning in the core libraries.
Thanks,
-Joe
--- a/src/share/classes/java/lang/Enum.javaMon Jan 06 11:48:32 2014 -0800
+++ b/src/share/classes/java/lang/Enum.javaMon Jan 06 13:37:06 2014 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2003, 2014, 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
@@ -52,6 +52,8 @@
* @see java.util.EnumMap
* @since 1.5
*/
+@SuppressWarnings(serial) // No serialVersionUID needed due to
+// special-casing of enum types.
public abstract class EnumE extends EnumE
implements ComparableE, Serializable {
/**
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: JDK-8028726 - (prefs) Check src/solaris/native/java/util/FileSystemPreferences.c for JNI pending exceptions
Dan, Looks OK, but line 914 which you did not change, notice the comments not sure if that is common in this code but seemed a bit off to me: 914 //// If at first, you don't succeed... On Jan 6, 2014, at 5:29 PM, Dan Xu wrote: Hi All, Please review the simple fix for JNI pending exceptions in FileSystemPreferences.c. Thanks! Bug: https://bugs.openjdk.java.net/browse/JDK-8028726 Webrev: http://cr.openjdk.java.net/~dxu/8028726/webrev/ -Dan 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: JDK 9 RFR of raw type lint fix to java.lang.management
+1 On Jan 7, 2014, at 3:30 PM, Joe Darcy wrote: Hello, Please review another minor lint fix of a raw type issues in the core libraries: diff -r 2647b91dbc2a src/share/classes/java/lang/management/ManagementFactory.java --- a/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 07 09:58:16 2014 -0800 +++ b/src/share/classes/java/lang/management/ManagementFactory.java Tue Jan 07 12:29:22 2014 -0800 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2014, 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 @@ -612,7 +612,7 @@ is not an instance of + mxbeanInterface); } -final Class[] interfaces; +final Class?[] interfaces; // check if the registered MBean is a notification emitter boolean emitter = connection.isInstanceOf(objName, NOTIF_EMITTER); 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: JDK 9 RFR of JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}
looks good Joe
On Jan 7, 2014, at 6:58 PM, Joe Darcy wrote:
Hello,
Please review the fix below to address
JDK-8031369: Fix raw types warnings in sun.misc.{Cache, SoftCache}
by a quick-and-dirty generification and deprecation of some very old classes
http://cr.openjdk.java.net/~darcy/8031369.0/
Corresponding patch below.
In the fullness of time, these classes should probably be removed from the
platform, but for the moment I'm more concerned with eliminating the several
dozen lint warnings in this code.
Thanks,
-Joe
--- old/src/share/classes/sun/misc/Cache.java2014-01-07
15:51:32.0 -0800
+++ new/src/share/classes/sun/misc/Cache.java2014-01-07
15:51:32.0 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1995, 1996, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1995, 2014, 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
@@ -74,8 +74,9 @@
* @see java.lang.Object#equals
* @see sun.misc.Ref
*/
+@Deprecated
public
-class Cache extends Dictionary {
+class Cache extends DictionaryObject, Object {
/**
* The hash table data.
*/
@@ -163,7 +164,7 @@
* @see Cache#elements
* @see Enumeration
*/
-public synchronized Enumeration keys() {
+public synchronized EnumerationObject keys() {
return new CacheEnumerator(table, true);
}
@@ -173,7 +174,7 @@
* @see Cache#keys
* @see Enumeration
*/
-public synchronized Enumeration elements() {
+public synchronized EnumerationObject elements() {
return new CacheEnumerator(table, false);
}
@@ -305,7 +306,7 @@
* A Cache enumerator class. This class should remain opaque
* to the client. It will use the Enumeration interface.
*/
-class CacheEnumerator implements Enumeration {
+class CacheEnumerator implements EnumerationObject {
boolean keys;
int index;
CacheEntry table[];
--- old/src/share/classes/sun/misc/SoftCache.java2014-01-07
15:51:33.0 -0800
+++ new/src/share/classes/sun/misc/SoftCache.java2014-01-07
15:51:33.0 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2006, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1998, 2014, 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
@@ -101,8 +101,8 @@
* @see java.lang.ref.SoftReference
*/
-
-public class SoftCache extends AbstractMap implements Map {
+@Deprecated
+public class SoftCache extends AbstractMapObject, Object implements
MapObject, Object {
/* The basic idea of this implementation is to maintain an internal
HashMap
that maps keys to soft references whose referents are the keys' values;
@@ -115,18 +115,18 @@
*/
-static private class ValueCell extends SoftReference {
+static private class ValueCell extends SoftReferenceObject {
static private Object INVALID_KEY = new Object();
static private int dropped = 0;
private Object key;
-private ValueCell(Object key, Object value, ReferenceQueue queue) {
+private ValueCell(Object key, Object value, ReferenceQueueObject
queue) {
super(value, queue);
this.key = key;
}
private static ValueCell create(Object key, Object value,
-ReferenceQueue queue)
+ ReferenceQueueObject queue)
{
if (value == null) return null;
return new ValueCell(key, value, queue);
@@ -154,10 +154,10 @@
/* Hash table mapping keys to ValueCells */
-private Map hash;
+private MapObject, Object hash;
/* Reference queue for cleared ValueCells */
-private ReferenceQueue queue = new ReferenceQueue();
+private ReferenceQueueObject queue = new ReferenceQueue();
/* Process any ValueCells that have been cleared and enqueued by the
@@ -189,7 +189,7 @@
* factor is less than zero
*/
public SoftCache(int initialCapacity, float loadFactor) {
-hash = new HashMap(initialCapacity, loadFactor);
+hash = new HashMap(initialCapacity, loadFactor);
}
/**
@@ -202,7 +202,7 @@
* or equal to zero
*/
public SoftCache(int initialCapacity) {
-hash = new HashMap(initialCapacity);
+hash = new HashMap(initialCapacity);
}
/**
@@ -210,7 +210,7 @@
* capacity and the default load factor.
*/
public SoftCache() {
-hash = new HashMap();
+hash = new HashMap();
}
@@ -348,13 +348,13 @@
/* Internal class for
Re: RFR JDK 9: 8032502: java.time add @param tags to readObject
looks fine Roger as am sure this will make the doclint warnings less On Jan 22, 2014, at 4:26 PM, roger riggs wrote: Please review this javadoc improvement to add @param tags to readObject Webrev: http://cr.openjdk.java.net/~rriggs/webrev-time-param-8032502/ Thanks, Roger 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): 8032392 : Spec: javax.xml.stream.XMLEventFactory/XMLOutputFactory/XMLInputFactory.newFactory(String, ClassLoader) referring to ServiceLoader.load(Class, ClassLoader)
+1 On Jan 24, 2014, at 3:31 PM, huizhe wang wrote: Hi, Please review a javadoc change to javax.xml.stream factories. This change makes it clear that the two args ServiceLoader#load method is used when the specified classLoader is not null. http://cr.openjdk.java.net/~joehw/jdk9/8032392/webrev/ 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 8032221 Typo in java.util.date
looks fine. getting rid of tt and code, is something I guess we should look to do throughout all of our code? On Jan 31, 2014, at 1:33 PM, roger riggs wrote: Please review a typo and javadoc cleanup for java.util.Date webrev: http://cr.openjdk.java.net/~rriggs/webrev-date-typo-8032221/ Thanks, Roger 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: Review request for JDK-8030010: cleanup native code warnings
Hi Mandy This looks OK to me On Feb 12, 2014, at 12:46 PM, Mandy Chung wrote: This patch cleans up a few trivial native warnings (mainly remove local unreferenced variable) https://bugs.openjdk.java.net/browse/JDK-8030010 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8030010/webrev.00/ This patch was contributed by Francis Andre [1] and I fixed a couple other trivial ones. Mandy [1] http://mail.openjdk.java.net/pipermail/build-dev/2013-December/011332.html 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 [4682009] Typo in javadocs in javax/naming
Looks fine. assume the code/code will be addressed as part of a full sweep of javax/naming On Feb 14, 2014, at 2:48 PM, Ivan Gerasimov wrote: Hello! May I please have a review of the fix? It's not meant to be a proof reading, I only fixed some obvious typos. Some of them were reported at the time of java 1.4, so it is probably the time to have them fixed. BUG: https://bugs.openjdk.java.net/browse/JDK-4682009 WEBREV: http://cr.openjdk.java.net/~igerasim/4682009/0/webrev/ Sincerely yours, Ivan 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 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec
Looks Ok. Kind of surprised the tck tests have no assertion details in the tests. Minor nit would have been nice to have even a minor comment for the new method DateTimeFormatterBuilder though that seems to be the norm in some scenarios for the smaller methods. On Feb 28, 2014, at 4:48 PM, roger riggs wrote: Please review this bug fix from Steve Colebourne for java.time parsing of fixed width adjacent values. The patch includes new tests. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/ Thanks, Roger 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): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote: nitpicking, (1) shouldn't the variable at #468 to be updated to lch instead of uch as well? I would agree given you are now calling Character.toLowerCase (2) StringBuilder can be used to replace the StringBuffer in toString(). Agree, but I think this could be considered optional for this putback... -Sherman On 03/05/2014 12:18 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David 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): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On Mar 5, 2014, at 5:10 PM, huizhe wang wrote: On 3/5/2014 12:46 PM, Lance Andersen - Oracle wrote: On Mar 5, 2014, at 3:37 PM, Xueming Shen wrote: nitpicking, (1) shouldn't the variable at #468 to be updated to lch instead of uch as well? I would agree given you are now calling Character.toLowerCase Also, at line 463 and 464, it looks like 'uppers' meant to be 'lowers' defined at 462. I think we should add tests with this change it looks like we really need to test this if your suggestion above is correct. (2) StringBuilder can be used to replace the StringBuffer in toString(). Agree, but I think this could be considered optional for this putback... Yes, feel free to replace StringBuffer with StringBuilder, Vector to ArrayList and etc. -Joe -Sherman On 03/05/2014 12:18 PM, David Li wrote: Hi, This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577. Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/ No new tests. There were none added in Xerces. Existing tests: JAXP SQE and unit tests passed. Thanks, David Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
I think this OK.
The comments with the o--o did not do much for me though and found them a bit
confusing but perhaps I need more coffee this morning ?
Also, not sure we need the @author tag but I think its usage varies in the
workspace
Best
Lance
On Mar 19, 2014, at 7:10 PM, David Li wrote:
Hi,
This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For
details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577.
Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/
Existing tests: JAXP SQE and unit tests passed.
Test cases added for typo fix in RangeToken.intersectRanges. Code also
updated to fix a bug where regular expression intersection returns incorrect
value when first range ends later than second range. Example below. Test
cases have been added to cover any scenarios that the code changes affect.
new RegularExpression((?[b-d][a-r])); - returns [b-d] (Correct)
new RegularExpression((?[a-r][b-d])); - returns [b-de-r] (Incorrect)
Thanks,
David
P.S. Notes on bug fixes.
1) Line 404 removal of while loop.
This fixes a new bug where incorrect results are given when first range ends
later than second range. In the old code we got
(?[a-r][b-d]) - returns [b-de-r]
By removing the while loop, we get [b-d].
This while loop looks like a copy-paste error from subtractRanges. In
subtractRanges we need to keep the leftover portion from the first range, but
this does not apply to intersection.
2) Line 388, addition of src2 += 2;
This code change affects anything of the form (?[a-r][b-eg-j]). The code
execution is diagrammed below.
oo (src1)
o--o o--o (src2)
For the first match we get
oo (src1)
o--o (src2)
Next we want to run src2+=2 to get the second pair of endpoints (since the
first two endpoints are already used). Notice how src1begin has been updated
to this.ranges[src1] = src2end+1, which is directly from the code.
o--o (src1)
o--o (src2)
The src2+=2 statement was left out of the old code, and is added in this
webrev. If we leave out the src2+=2 at line 388, on the next iteration of
the large while loop we will reach case } else if (src2end src1begin) {
which also executes src2+=2. This means the correct final result is
generated, but on a later loop. We want to add the new code because it's
better to have all associated variable updated in the sameloop. In addition,
all the other conditions have similar src1 or src2 updates.
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 : 7162902 / 6893617 Umbrella port of a number of corba bug fixes from JDK 6 to jdk7u/8
Hi Sean, I think this looks good. ship it :-) Best Lance On Jun 25, 2012, at 4:26 PM, Sean Coffey wrote: Hi, I'm looking for a code review around the following corba changes. It turns out that we've a few bug fixes in corba area for jdk6 that were never forward ported to jdk7 or 8. The port is pretty much identical to what was used in JDK6. Some formatting and diamond operator changes introduced but that's about it. There were a few bug fixes to follow up on regressions around the initial fix but this umbrella port should cover all issues. The initial bugs fixes were fixed under the jets category in the bug tool and that category entered read only mode over a year ago. Hence the requirement for a new bug ID. The main bug fix is 6725987 which is an issue where references to the ORB were being kept after ORB.destroy was called. http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7162902 An accompanying CNCtx fix in JDK repo is also made to correct an issue where the java.naming.corba.orb system property wasn't used correctly. (CR 6893617) http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6893617 Corba testsuites run and no issues seen. I've also run some JCK testing and saw no issues. http://cr.openjdk.java.net/~coffeys/webrev.7162902.jdk8/ (corba) http://cr.openjdk.java.net/~coffeys/webrev.6893617.jdk8/ (jdk) The corba codebase code formatting seems to vary quite a bit. I've reformatted in a small number of areas but the whole corba repo probably warrants it's own clean up project at a later date. regards, Sean. 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: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Deven, Thanks for the email and the proposed patch. I will look at this later today or tomorrow. I actually have made these changes in my workspace for JDK 8 but will compare your changes to mine. Best Lance On Jul 2, 2012, at 5:04 AM, Deven You wrote: Hi All, Could anyone notice this problem? Thanks a lot! On 06/25/2012 04:18 PM, Deven You wrote: Hi All, First of all, if the jdbc problem has a better mailing list to post please tell me. I find that javax.sql.rowset.serial.SerialBlob is not fully implemented in OpenJDK 8. Methods public InputStream getBinaryStream(long pos,long length) throws SQLException public void free() throws SQLException only throw UnsupportedOperationException. I have made a patch[1] to implement these 2 methods. Could anyone take a look to review it. BTW: I think the spec for SerialBlob is not very clear like it doesn't mention if all method rather than free() need throw any exception after free() is invoked. However that behavior seems reasonable. [1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev Thanks a lot. -- Best Regards, Deven 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: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Deven, We had a holiday here on Weds, so I am still catching up. First, thank you for taking the time to propose a patch. Here are a few comments based on the changes I have made in my workspace and comparing to your proposed changes - The same issue applies to SerialClob - instead of duplicating all of the code to check if free has been called in each method, I created a private method which does the validation - I do not see a reason to make the flag isFree transient, if the object has been freed, it has been freed - free() needs to call blob.free if the blob object is not null prior to nulling out the object - Your 2nd if statement in getBinaryStream() duplicates part of the check in the 1st if statement - All of the public methods need to have their javadocs indicate that if called after free has been called, a SerialException will be thrown. additional calls to free is a no-op - The test case is is a good start.I would change this a bit though: + create separate test classes for free and getbinarystream + each individual test be a method + If you catch the expected Exception, print that the test has passed + Each test needs a comment as to what it is trying to validate (just a simple comment is fine) + return 0 if the test passed, 1 if it fails and then print the number of failed tests at the end after running through all of them As I need to change the javadocs, I have create a ccc request which I will do as part of the JDBC 4.2 work and will put the change back at that time. If you would like to change your test and then create similar tests for SerialClob I will add those as part of the put-back. Again, thank you for your input and time. Best Lance On Jul 5, 2012, at 2:26 AM, Deven You wrote: Hi Lance, Did you review the patch and compare it to yours. I just have some more words for the patch. 1. I think the current spec for free() is not clear, how about add below comments: After free has been called, any attempt to invoke a method other than free will result in a SerialException being thrown. 2. getBinaryStream(long pos,long length) add a javadoc: * @throws SerialException if this SerialBlob already be freed. add throws SerialException from this method Any suggestions? Thanks a lot! On 07/02/2012 06:25 PM, Lance Andersen - Oracle wrote: Hi Deven, Thanks for the email and the proposed patch. I will look at this later today or tomorrow. I actually have made these changes in my workspace for JDK 8 but will compare your changes to mine. Best Lance On Jul 2, 2012, at 5:04 AM, Deven You wrote: Hi All, Could anyone notice this problem? Thanks a lot! On 06/25/2012 04:18 PM, Deven You wrote: Hi All, First of all, if the jdbc problem has a better mailing list to post please tell me. I find that javax.sql.rowset.serial.SerialBlob is not fully implemented in OpenJDK 8. Methods public InputStream getBinaryStream(long pos,long length) throws SQLException public void free() throws SQLException only throw UnsupportedOperationException. I have made a patch[1] to implement these 2 methods. Could anyone take a look to review it. BTW: I think the spec for SerialBlob is not very clear like it doesn't mention if all method rather than free() need throw any exception after free() is invoked. However that behavior seems reasonable. [1] http://cr.openjdk.java.net/~littlee/OJDK-576/webrev Thanks a lot. -- Best Regards, Deven Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com -- Best Regards, Deven 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 [7u8]: 7166896: DocumentBuilder.parse(String uri) is not IPv6 enabled. It throws MalformedURLException
+1 Best Lance On Jul 12, 2012, at 5:42 AM, Paul Sandoz wrote: Hi Joe, On Jul 11, 2012, at 7:59 AM, Joe Wang wrote: Hi Paul, This is now for 7u8, so I started a new thread. As we discussed, I've removed the hack of using escapeNonUSAscii. In this regard, we're now in sync with the source code in Xerces. Here's the webrev: http://cr.openjdk.java.net/~joehw/7u8/7166896/webrev/ Looks OK to me. Note i am not an official reviewer, yet! so please don't commit cause i thought it looked OK ;-) Paul. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();
Looking for a reviewer for the following change:
- add a call to Thread.currentThread().getContextClassLoader() to
DriverManager.getDriver()
- Remove the synchronized block for the same call in getConnection()
Thank you.
Best
Lance
localhost:sql lanceandersen$ hg diff DriverManager.java
diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00
2012 -0400
@@ -264,6 +264,10 @@
// be null.
ClassLoader callerCL = DriverManager.getCallerClassLoader();
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
+}
+
// Walk through the loaded registeredDrivers attempting to locate
someone
// who understands the given URL.
for (DriverInfo aDriver : registeredDrivers) {
@@ -563,11 +567,8 @@
* classloader, so that the JDBC driver class outside rt.jar
* can be loaded from here.
*/
-synchronized(DriverManager.class) {
- // synchronize loading of the correct classloader.
- if(callerCL == null) {
- callerCL = Thread.currentThread().getContextClassLoader();
- }
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
}
if(url == null) {
localhost:sql lanceandersen$
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: Review request 7190657 Modify getDriver() to call Thread.currentThread().getContextClassLoader();
For completeness, they could be added and I guess I can do it as part of this
particular change.
I would probably move the call to DriverManager.getCallerClassLoader() to
avoid the duplication of the if block.
Best
Lance
On Aug 10, 2012, at 5:02 PM, David Schlosnagle wrote:
Lance,
There are a couple of other uses of
DriverManager.getCallerClassLoader() that do no fall back to
Thread.currentThread().getContextClassLoader() if the caller class
loader is null (bootstrap loader). Should these be updated as well?
From
http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/sql/DriverManager.java:
Line 334 in deregisterDriver(Driver driver)
Line 365 in getDrivers()
Thanks,
Dave
On Fri, Aug 10, 2012 at 1:19 PM, Lance Andersen - Oracle
lance.ander...@oracle.com wrote:
Looking for a reviewer for the following change:
- add a call to Thread.currentThread().getContextClassLoader() to
DriverManager.getDriver()
- Remove the synchronized block for the same call in getConnection()
Thank you.
Best
Lance
localhost:sql lanceandersen$ hg diff DriverManager.java
diff -r 629f357fc17b src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Fri Aug 10 09:17:14
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Fri Aug 10 13:05:00
2012 -0400
@@ -264,6 +264,10 @@
// be null.
ClassLoader callerCL = DriverManager.getCallerClassLoader();
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
+}
+
// Walk through the loaded registeredDrivers attempting to locate
someone
// who understands the given URL.
for (DriverInfo aDriver : registeredDrivers) {
@@ -563,11 +567,8 @@
* classloader, so that the JDBC driver class outside rt.jar
* can be loaded from here.
*/
-synchronized(DriverManager.class) {
- // synchronize loading of the correct classloader.
- if(callerCL == null) {
- callerCL = Thread.currentThread().getContextClassLoader();
- }
+if(callerCL == null) {
+callerCL = Thread.currentThread().getContextClassLoader();
}
if(url == null) {
localhost:sql lanceandersen$
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
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: 7056731: Race condition in CORBA code causes re-use of ABORTed connections
Hi Sean, This looks good. Best Lance On Aug 14, 2012, at 12:38 PM, Seán Coffey wrote: I'm looking to forward port this corba fix from 6u34 to jdk8 (and eventually port to 7u) d.macdon...@auckland.ac.nz originally reported this issue and I'll be marking the fix as contributed by him. He's already signed the OCA. There's a good description in bug report. We have a race like condition if the corba server is shut down while there are still workers/client threads in a waiting queue waiting for server. The issue arises when the purgeCalls method marks a socket state as ABORTed or CLOSE_RECVD but fails to remove that socket from the socket connectionCache. Fix is to wrap the removal of the socket in a finally block. The CorbaResponseWaitingRoomImpl changes are more focused on ensuring that the Map holding these clients is better synchronized. webrev : http://cr.openjdk.java.net/~coffeys/webrev.7056731.jdk8/ http://cr.openjdk.java.net/%7Ecoffeys/webrev.7056731.jdk8/ bug report : http://bugs.sun.com/view_bug.do?bug_id=7056731 Regards, Sean. 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 [7u8] (JAXP): 7191547 : XMLEventFactory.newFactory(String factoryId, ClassLoader loader) does not work as expected
Hi Joe, Looks fine. Best Lance On Aug 17, 2012, at 12:07 AM, Joe Wang wrote: In the patch for 6756677, we fixed errors in StAX's input and output factories where factoryId such as 'javax.xml.stream.XMLInputFactory was taken as factory class e.g. com.sun.xml.internal.stream.XMLInputFactoryImpl. Unfortunately, we missed XMLEventFactory in that patch. So this is a patch for XMLEventFactory similar to that of XMLInputFactory and XMLOutputFactory. The change itself is to search an implementation class using the provided factory id instead of trying to create a instance directly. The webrev is here: http://cr.openjdk.java.net/~joehw/7u8/7191547/webrev/ All tests pass except two XMLEventFactory tests that assumed the factoryId to mean factory class. They will be updated once this patch is approved. 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: Review Request: 7193683: Cleanup Warning in java.sql package
This looks fine. Dan, I will commit this for you Thursday Best Lance On Aug 29, 2012, at 1:29 AM, Dan Xu wrote: I made a simple fix to clean up build warnings in java.sql package. The change can be reviewed at http://cr.openjdk.java.net/~dxu/7193683/webrev.01/. Thanks! -Dan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
Hi all, Looking for a reviewer for the removal of PropertyChangeSupport from JDBCRowSetImpl that was originally going to be used by the EOL Rave product. As it is no longer needed the code has been removed. The SQE and RowSet TCK tests all continue to run without regression. The webrev can be found at http://cr.openjdk.java.net/~lancea/7192302/webrev.00 Best Lance 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: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01 I know there is more clean-up that can be done to remove other Rave added code (such as the removal of set/getPreparedStatement/Connection/ResultSet), I want to keep the focus to just removing PropertyChangeSupport. SQE and RowSet TCKs continue to pass with these changes. Best Lance On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote: On 05/09/2012 22:04, Lance Andersen - Oracle wrote: Hi all, Looking for a reviewer for the removal of PropertyChangeSupport from JDBCRowSetImpl that was originally going to be used by the EOL Rave product. As it is no longer needed the code has been removed. The SQE and RowSet TCK tests all continue to run without regression. The webrev can be found at http://cr.openjdk.java.net/~lancea/7192302/webrev.00 Thanks Lance, it's good to remove this dependency. In both commit and rollback then it looks to me that the setting of oldVal can be removed. Otherwise looks good to me. -Alan 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: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
Thank you for the comments Alan On Sep 6, 2012, at 9:00 AM, Alan Bateman wrote: On 06/09/2012 13:40, Lance Andersen - Oracle wrote: Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01 I know there is more clean-up that can be done to remove other Rave added code (such as the removal of set/getPreparedStatement/Connection/ResultSet), I want to keep the focus to just removing PropertyChangeSupport. SQE and RowSet TCKs continue to pass with these changes. Your previous mail uses the word nuke, I was thinking the same thing :-) The latest webrev looks okay except that in one of the constructors you have removed a call to ensure that the connection is established, I'm not sure about the significance of that. This is not needed here and given I have already tested with this removed, I figured I would OK to keep this as part of the change I also see there are a couple of residual references to Rave that can probably be pulled, these seem to be related to the PropertyChangeListener support. I left those in as a reminder to go back as part of the rest of the Rave clean-up. I would prefer to leave them for now and when I make another pass for Rave, I will get rid of them One method that looks like it could be removed too is setConcurrency but I agree that keeping focused on just removing the beans dependency is right for now. I had thought about that but I have to think about this one a bit more as the getConcurrency() is leveraging the value returned from the active ResultSet which is why I did not remove this at this time. In some cases, I must confess the original authors have left me scratching my head on some of this code but I want to be surgical in removing or addressing changes so that it would be easier to find any un-intended gotchas. Best Lance -Alan. 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: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
On Sep 6, 2012, at 9:31 AM, Alan Bateman wrote: On 06/09/2012 14:09, Lance Andersen - Oracle wrote: : The latest webrev looks okay except that in one of the constructors you have removed a call to ensure that the connection is established, I'm not sure about the significance of that. This is not needed here and given I have already tested with this removed, I figured I would OK to keep this as part of the change Okay, I'll have to trust you on this one but I am a little bit concerned that it could cause a NPE, say someone creates a JdbcRowSet and invokes a method such as comment, rollback or getAutoCommit without doing an explicit connect. connect() is typically called to get a connection and it will validate that there is a non-null Connection. I have unit tests which leverage that constructor and run clean with that change. I agree that commit(), rollback, etc should call checkState() and that is another fix I need to do but the potential for the NPE is there depending on which constructor was used and if you did something silly like making a rollback without doing any work. I can revert that change if you like but I have not seen any failures under what I would term normal use. I left those in as a reminder to go back as part of the rest of the Rave clean-up. I would prefer to leave them for now and when I make another pass for Rave, I will get rid of them Okay. One method that looks like it could be removed too is setConcurrency but I agree that keeping focused on just removing the beans dependency is right for now. I had thought about that but I have to think about this one a bit more as the getConcurrency() is leveraging the value returned from the active ResultSet which is why I did not remove this at this time. Okay although it looks to be just an optimization introduced as part of adding listener events I am not sure I agree with the code either, just want to spend a bit more time looking at the code flow and see what tests if any we have here I'm fine with leaving it as it is. thank you -Alan 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: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
Hi Mandy, Thank you also for the feedback along with Alan. I will push these back shortly and also create a CR. Best Lance On Sep 6, 2012, at 11:04 AM, Mandy Chung wrote: Lance, On 9/6/2012 5:40 AM, Lance Andersen - Oracle wrote: Here is the updated webrev http://cr.openjdk.java.net/~lancea/7192302/webrev.01 It's good to see this change that jdbc rowset doesn't depend on java.beans. This looks okay to me as you have explained why you remove the call to connect() in one of the constructors. Alan and you discussed if setConcurrency could be removed in this fix. setType() is another one that could be removed. I'm fine with leaving these cleanup in another fix and have this fix to focus on removing the dependency of PropertyChangeSupport. It'd be good to file a CR as a follow-up of this work. Thanks Mandy I know there is more clean-up that can be done to remove other Rave added code (such as the removal of set/getPreparedStatement/Connection/ResultSet), I want to keep the focus to just removing PropertyChangeSupport. SQE and RowSet TCKs continue to pass with these changes. Best Lance On Sep 5, 2012, at 5:17 PM, Alan Bateman wrote: On 05/09/2012 22:04, Lance Andersen - Oracle wrote: Hi all, Looking for a reviewer for the removal of PropertyChangeSupport from JDBCRowSetImpl that was originally going to be used by the EOL Rave product. As it is no longer needed the code has been removed. The SQE and RowSet TCK tests all continue to run without regression. The webrev can be found at http://cr.openjdk.java.net/~lancea/7192302/webrev.00 Thanks Lance, it's good to remove this dependency. In both commit and rollback then it looks to me that the setting of oldVal can be removed. Otherwise looks good to me. -Alan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
Hi Alan, The connect method is used by the RI not the RowSet spec. It was made protected for Rave. Best Lance On Sep 6, 2012, at 1:21 PM, Alan Bateman wrote: Lance, I see you've just pushed this but one thing I didn't spot initially is that in the second webrev you changed the protected connect method to be private. Is this a supported API and could this cause problems for JDBC drivers that extend this? -Alan 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: Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans
The recommended way to access a rowset is via the factory now.For whatever reason the original authors chose not to provide a factory. The connect() method has always been a method to be used internally by JdbcRowSetRIImpl and Rave. There is no reason for a user of the API to ever have to call this method. I can certainly update the javadocs for JdbcRowSet to use the factory but all of the examples as well as the tutorial always leveraged using the defined API. Best Lance On Sep 6, 2012, at 1:54 PM, Mandy Chung wrote: As the comment noted, the connect method was made protected for Rave requirement but it's not defined in JdbcRowSet and BaseRowSet. It's a method specific to the RI and changing it to private is okay if they get an JdbcRowSet instance by calling the rowset factory. Alan's question prompts me to look if anyone references JdbcRowSetImpl directly - I find that the javax.sql.rowset.JdbcRowSet spec has the example to instantiate JdbcRowSetImpl. Is it still a supported or recommended way? If so, there is a potential compatibility risk; otherwise, it'd be good to update the spec to remove reference to JdbcRowSetImpl. Mandy On 9/6/2012 10:21 AM, Alan Bateman wrote: Lance, I see you've just pushed this but one thing I didn't spot initially is that in the second webrev you changed the protected connect method to be private. Is this a supported API and could this cause problems for JDBC drivers that extend this? -Alan 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: Review Request for 7195933: There is incorrect link to Info-ZIP Application Note 970311 in doc page of Package java.util.zip
looks OK Dan and I was able to access the URL and download the zip Best Lance On Sep 18, 2012, at 1:29 AM, Dan Xu wrote: Hi, This is the change to correct a java doc link in java.util.zip package page. The webrev can be reviewed at http://cr.openjdk.java.net/~dxu/7195933/webrev/. Thanks! -Dan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request for 7197395
Hi, This is a review request for adding @Deprecated to the remaining JDBC methods to suppress compiler warnings. The CCC was approved as this annotation results in the signatures changing (though they do not effect the execution of any applications). The webrev is http://cr.openjdk.java.net/~lancea/7197395/webrev.00/ Best Lance 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: JAXP RFR: 8000172 : 2 SAX features does not work properly
Look OK Joe On Oct 9, 2012, at 12:31 AM, Joe Wang wrote: This is an issue found while I was working with SQE to expand test coverage. For a non-validating parser, when load-external-dtd is false, entity references are skipped. However, the skippedEntity() event was not reported. The problem was due to missing infoset augmentations when document handler's startGeneralEntity/endGeneralEntity were called. The fix therefore is simply add the arguments. webrev: http://cr.openjdk.java.net/~joehw/jdk8/8000172/webrev/ Please review. 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 : 7196086 : update copyright years for files in corba repository (JDK 8)
looks fine Sean Best Lance On Oct 9, 2012, at 2:38 PM, Seán Coffey wrote: steve.si...@oracle.com has contributed the following patch which I'd like to push to jdk8 TL. It's the correction of copyright years in corba repo. Need a reviewer. webrev : http://cr.openjdk.java.net/~coffeys/webrev.7196086.jdk8/ http://cr.openjdk.java.net/%7Ecoffeys/webrev.7196086.jdk8/ regards, Sean. 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: 7192274: Deprecate LogManager addPropertyChangeListener and removePropertyChangeListe
looks fine Alan and in line with the other work we have done Best Lance On Oct 10, 2012, at 7:19 AM, Alan Bateman wrote: JEP 162 [1] captures a number of things that we can do in preparation for future modularization of the platform. One of these items is deprecating the Java SE APIs that are problematic for our modularization efforts. Thankfully the list is very short as this is deprecation is in anticipation of really removing the APIs when a module system comes along. The patch proposed here is the first installation to deprecate the LogManager addPropertyChangeListener and removePropertyChangeListener methods. These methods are problematic because of the API dependency on java.beans.PropertyChangeListener (java.beans is toxic because of types in that package that are tied to Applet, AWT, and Swing). When we eventually get to remove these methods then we expect the impact will not be too significant. Mandy Chung has done a static analysis over 20,000 projects to locate usages of these methods and found only 3 so we have some useful evidence to demonstrate that they aren't used very often. For now we are just proposing to deprecate the methods and the proposed patch is here: http://cr.openjdk.java.net/~alanb/7192274/webrev/jdk.patch Thanks, Alan. [1] http://openjdk.java.net/jeps/162 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review needed for 8000687
Need a reviewer for a simple typo in the DriverManager javadoc
new-host-2:sql lanceandersen$ hg diff
diff -r 036c55976cef src/share/classes/java/sql/DriverManager.java
--- a/src/share/classes/java/sql/DriverManager.java Tue Oct 09 08:58:27
2012 -0400
+++ b/src/share/classes/java/sql/DriverManager.java Wed Oct 10 10:59:00
2012 -0400
@@ -412,7 +412,7 @@
* method throws a codejava.lang.SecurityException/code.
*
* @param out the new logging/tracing PrintStream; to disable, set to
codenull/code
- * @deprecated Use {@code setLogWriter)
+ * @deprecated Use {@code setLogWriter}
* @throws SecurityException if a security manager exists and its
*codecheckPermission/code method denies setting the log stream
*
@@ -439,7 +439,7 @@
* and all drivers.
*
* @return the logging/tracing PrintStream; if disabled, is
codenull/code
- * @deprecated Use {@code getLogWriter)
+ * @deprecated Use {@code getLogWriter}
* @see #setLogStream
*/
@Deprecated
new-host-2:sql lanceandersen$
Best
Lance
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Review Request for 8000712
Hi, Looking for a reviewer for the removal of the following non-used fields in SyncFactory private static String default_provider private static Level rsLevel private static Object logSync private static java.io.PrintWriter logWriter Best Lance new-host-2:spi lanceandersen$ hg diff SyncFactory.java diff -r 3c4be36de073 src/share/classes/javax/sql/rowset/spi/SyncFactory.java --- a/src/share/classes/javax/sql/rowset/spi/SyncFactory.java Wed Oct 10 11:15:27 2012 -0400 +++ b/src/share/classes/javax/sql/rowset/spi/SyncFactory.java Wed Oct 10 16:57:46 2012 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2012, 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 @@ -229,11 +229,7 @@ * The standard resource file name. */ private static String ROWSET_PROPERTIES = rowset.properties; -/** - * The RI Optimistic Provider. - */ -private static String default_provider = -com.sun.rowset.providers.RIOptimisticProvider; + /** * Permission required to invoke setJNDIContext and setLogger */ @@ -248,24 +244,13 @@ * The codeLogger/code object to be used by the codeSyncFactory/code. */ private static volatile Logger rsLogger; -/** - * - */ -private static Level rsLevel; + /** * The registry of available codeSyncProvider/code implementations. * See section 2.0 of the class comment for codeSyncFactory/code for an * explanation of how a provider can be added to this registry. */ private static HashtableString, SyncProvider implementations; -/** - * Internal sync object used to maintain the SPI as a singleton - */ -private static Object logSync = new Object(); -/** - * Internal PrintWriter field for logging facility - */ -private static java.io.PrintWriter logWriter = null; /** * Adds the the given synchronization provider to the factory register. Guidelines new-host-2:spi lanceandersen$ 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: 8000362: (pack200) Deprecate Packer/Unpacker addPropertyChangeLister and removePropertyChangeListener methods
looks fine alan On Oct 11, 2012, at 9:40 AM, Alan Bateman wrote: This is a follow-on from yesterday's mail on deprecating the LogManager's add/removePropertyChangeListener methods. The other 4 problematic methods identified in JEP 162 [1] are the same name methods on Pack200.Packer and Pack200.Unpacker. The webrev to deprecate these methods is here: http://cr.openjdk.java.net/~alanb/8000362/webrev/ As with the LogManager methods, Mandy Chung searched 2 projects. In this case we didn't find any usages of these methods and this gives us confidence that these methods are rarely used. We don't propose to provide a replacement for these methods but instead just include wording to suggest that applications poll the value of the progress property if they really need to provide some progress indication. Kumar is the pack200 guru here and I know he's busy/away at the moment so I plan to hold onto this patch until I at least hear from him. -Alan. [1] http://openjdk.java.net/jeps/162 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Reviewer needed for 8000763: use XXX.valueOf methods instead of constructors
Hi,
Need a review for changing to use the XXX.valueOf methods from constructors.
Thank you
Best
Lance
new-host-2:rowset lanceandersen$ hg status -m
M src/share/classes/com/sun/rowset/CachedRowSetImpl.java
M src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
M src/share/classes/javax/sql/rowset/BaseRowSet.java
M src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java
new-host-2:rowset lanceandersen$ hg diff
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/CachedRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11
12:43:57 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -1747,7 +1747,7 @@
// convert to a Double and compare to zero
try {
Double d = new Double(value.toString());
-if (d.compareTo(new Double((double)0)) == 0) {
+if (d.compareTo(Double.valueOf(0)) == 0) {
return false;
} else {
return true;
@@ -4432,7 +4432,7 @@
// make sure the cursor is on a valid row
checkCursor();
-Object obj = convertNumeric(new Float(x),
+Object obj = convertNumeric(Float.valueOf(x),
java.sql.Types.REAL,
RowSetMD.getColumnType(columnIndex));
@@ -4467,7 +4467,7 @@
checkIndex(columnIndex);
// make sure the cursor is on a valid row
checkCursor();
-Object obj = convertNumeric(new Double(x),
+Object obj = convertNumeric(Double.valueOf(x),
java.sql.Types.DOUBLE,
RowSetMD.getColumnType(columnIndex));
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java Thu Oct 11
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java Thu Oct 11
12:43:57 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -839,7 +839,7 @@
if(onInsertRow) {
if(p != null) {
-bool = p.evaluate(new Float(x) , columnIndex);
+bool = p.evaluate(Float.valueOf(x) , columnIndex);
if(!bool) {
throw new
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
@@ -906,7 +906,7 @@
if(onInsertRow) {
if(p != null) {
-bool = p.evaluate(new Double(x) , columnIndex);
+bool = p.evaluate(Double.valueOf(x) , columnIndex);
if(!bool) {
throw new
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
diff -r c2be39b27e1c src/share/classes/javax/sql/rowset/BaseRowSet.java
--- a/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11
11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11
12:43:57 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -1850,7 +1850,7 @@
if(params == null){
throw new SQLException(Set initParams() before setFloat);
}
-params.put(Integer.valueOf(parameterIndex - 1), new Float(x));
+params.put(Integer.valueOf(parameterIndex - 1), Float.valueOf(x));
}
/**
@@ -1882,7 +1882,7 @@
if(params == null){
throw new SQLException(Set initParams() before setDouble);
}
-params.put(Integer.valueOf(parameterIndex - 1), new Double(x));
+params.put(Integer.valueOf(parameterIndex - 1), Double.valueOf(x));
}
/**
diff -r c2be39b27e1c
src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java
--- a/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java Thu Oct
11 11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java Thu Oct
11 12:43:57 2012 -0400
@@ -215,7 +215,7 @@
*/
@SuppressWarnings(unchecked)
public void writeFloat(float x) throws SQLException {
-attribs.add(new Float(x));
+attribs.add(Float.valueOf(x));
}
/**
@@ -230,7 +230,7 @@
*/
@SuppressWarnings(unchecked)
Resend: Reviewer needed for 8000763: use XXX.valueOf methods instead of constructors
Hi,
Revised CachedRowSetImpl as I noticed Findbugs missed a scenario where you
should use the XXX.valueOf methods from constructors.
Thank you
Best
Lance
new-host-2:rowset lanceandersen$ hg status -m
M src/share/classes/com/sun/rowset/CachedRowSetImpl.java
M src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
M src/share/classes/javax/sql/rowset/BaseRowSet.java
M src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java
new-host-2:rowset lanceandersen$ hg diff
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/CachedRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/CachedRowSetImpl.javaThu Oct 11
13:17:26 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -1746,8 +1746,8 @@
// convert to a Double and compare to zero
try {
-Double d = new Double(value.toString());
-if (d.compareTo(new Double((double)0)) == 0) {
+Double d = Double.valueOf(value.toString());
+if (d.compareTo(Double.valueOf(0)) == 0) {
return false;
} else {
return true;
@@ -4432,7 +4432,7 @@
// make sure the cursor is on a valid row
checkCursor();
-Object obj = convertNumeric(new Float(x),
+Object obj = convertNumeric(Float.valueOf(x),
java.sql.Types.REAL,
RowSetMD.getColumnType(columnIndex));
@@ -4467,7 +4467,7 @@
checkIndex(columnIndex);
// make sure the cursor is on a valid row
checkCursor();
-Object obj = convertNumeric(new Double(x),
+Object obj = convertNumeric(Double.valueOf(x),
java.sql.Types.DOUBLE,
RowSetMD.getColumnType(columnIndex));
diff -r c2be39b27e1c src/share/classes/com/sun/rowset/FilteredRowSetImpl.java
--- a/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java Thu Oct 11
11:47:05 2012 +0100
+++ b/src/share/classes/com/sun/rowset/FilteredRowSetImpl.java Thu Oct 11
13:17:26 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -839,7 +839,7 @@
if(onInsertRow) {
if(p != null) {
-bool = p.evaluate(new Float(x) , columnIndex);
+bool = p.evaluate(Float.valueOf(x) , columnIndex);
if(!bool) {
throw new
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
@@ -906,7 +906,7 @@
if(onInsertRow) {
if(p != null) {
-bool = p.evaluate(new Double(x) , columnIndex);
+bool = p.evaluate(Double.valueOf(x) , columnIndex);
if(!bool) {
throw new
SQLException(resBundle.handleGetObject(filteredrowsetimpl.notallowed).toString());
diff -r c2be39b27e1c src/share/classes/javax/sql/rowset/BaseRowSet.java
--- a/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11
11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/BaseRowSet.javaThu Oct 11
13:17:26 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, 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
@@ -1850,7 +1850,7 @@
if(params == null){
throw new SQLException(Set initParams() before setFloat);
}
-params.put(Integer.valueOf(parameterIndex - 1), new Float(x));
+params.put(Integer.valueOf(parameterIndex - 1), Float.valueOf(x));
}
/**
@@ -1882,7 +1882,7 @@
if(params == null){
throw new SQLException(Set initParams() before setDouble);
}
-params.put(Integer.valueOf(parameterIndex - 1), new Double(x));
+params.put(Integer.valueOf(parameterIndex - 1), Double.valueOf(x));
}
/**
diff -r c2be39b27e1c
src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java
--- a/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java Thu Oct
11 11:47:05 2012 +0100
+++ b/src/share/classes/javax/sql/rowset/serial/SQLOutputImpl.java Thu Oct
11 13:17:26 2012 -0400
@@ -215,7 +215,7 @@
*/
@SuppressWarnings(unchecked)
public void writeFloat(float x) throws SQLException {
-attribs.add(new Float(x));
+
Re: Review Request: JDK-8001565, (fs) Typo Path.endsWith(String) javadoc
This fix is fine Dan Best Lance On Oct 25, 2012, at 5:45 PM, Dan Xu wrote: Hi, Please help review the javadoc typo fix at, http://cr.openjdk.java.net/~dxu/8001565/webrev/. Thanks! -Dan 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: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: Hi Lance, I am very busy with other work so I can't work with the SerialBlob/SerialClob item for long time. I am very happy to refine the current test case and create new tests for SerialClob. I have create a new webre[1] for this task, please review it. [1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/ PS: If the isFree is not transient, I want to know how we add this field to the javadoc serialized form? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request for 8001536
Hi, This is a request for review of http://cr.openjdk.java.net/~lancea/8001536/webrev.00/. This adds read/writeObject as well as clone methods to SerialXLob classes. All SQE tests passed, 1 failure in the RowSet JCK/TCK tests due to a bug in the test that the TCK team is aware of and will address. JDBC Unit tests all pass . Best Lance 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: Review request for 8001536
Hi Ulf, The bug is described below, it is just adding the read/writeObject and clone methods. Best Lance On Oct 30, 2012, at 2:18 PM, Ulf Zibis wrote: Hi all, please add the bug summary to the subject line. Bug 8001536 is not publicly visible :-( -Ulf Am 30.10.2012 17:25, schrieb Lance Andersen - Oracle: Hi, This is a request for review of http://cr.openjdk.java.net/~lancea/8001536/webrev.00/. This adds read/writeObject as well as clone methods to SerialXLob classes. All SQE tests passed, 1 failure in the RowSet JCK/TCK tests due to a bug in the test that the TCK team is aware of and will address. JDBC Unit tests all pass . Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: Review request for 8001536
Hi Remi, Thank you for the feedback On Oct 30, 2012, at 2:05 PM, Remi Forax wrote: On 10/30/2012 05:25 PM, Lance Andersen - Oracle wrote: Hi, This is a request for review of http://cr.openjdk.java.net/~lancea/8001536/webrev.00/. This adds read/writeObject as well as clone methods to SerialXLob classes. All SQE tests passed, 1 failure in the RowSet JCK/TCK tests due to a bug in the test that the TCK team is aware of and will address. JDBC Unit tests all pass . Hi Lance. In SerialBlob and in SerialClob test (obj == null) is not necessary in equals, null instanceof X is always false. OK, thanks for the suggestion, I will make this change in hashCode, Objects.hash() allocate an array to pass arguments to Arrays.hashCode() and box primitive values to Object. while this method is really convenient to use, each calls will allocate an array and box the two values, the overhead seems to high here. This code should be equivalent: return ((31 +Arrays.hashCode(buf)) * 31 +len) * 31 + origLen; I can simplify hashCode to the what you have above, I liked the convenience method which is why I was using it. But happy to change it in clone, sb should not be initialized to null I think it is OK as I have it as HashMap does it similar to what I have done vs ArrayList which follows your suggestion. Do we have a preferred practice or is this just a style choice? and the catch should be: throw new InternalError(e), Given I am providing clone(), I did not see a reason to provide InternalError(). I have no strong preference but some java classes do and others do not (HashMap for example), so what is our preferred standard? this is the standard code you can see in clone. in readObject, the test (buf.length != len) can be done before decoding the blob. True, I can move it up. in writeObject, you set blob twice, which is weird, my bad, I forgot to remove that. also I think that if blob is not Serializable, the code should throw an exception, so you should not use instanceof and let s.writeFields() to throw NotSerializable exception. This is intentional. A Blob or Clob will not be serializable as its properties are unique to the database and it is created from an active Connection object. In the event someone actually tried to serialize this, I do not want it to fail just because someone passed in a LOB to instantiate this beast (note these methods should never have been created this way but this is way before my time). In the unlikely event someone created their own wrapped Blob/Clob (which I cannot see why anyone would do), I am allowing for both for backwards compatibility. Best Lance cheers, Rémi Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: Review request for 8001536
Point taken Ulf, thank you for the feedback and will follow your suggestion going forward , which I typically do, but I think I am still feeling the effects of being offline due to hurricane sandy :-( Best Lance On Oct 30, 2012, at 2:50 PM, Ulf Zibis wrote: Thanks Lance. But having the subject of the request in clear text in the list view of the email client would be a great help. -Ulf Am 30.10.2012 19:28, schrieb Lance Andersen - Oracle: Hi Ulf, The bug is described below, it is just adding the read/writeObject and clone methods. Best Lance On Oct 30, 2012, at 2:18 PM, Ulf Zibis wrote: Hi all, please add the bug summary to the subject line. Bug 8001536 is not publicly visible :-( -Ulf Am 30.10.2012 17:25, schrieb Lance Andersen - Oracle: Hi, This is a request for review of http://cr.openjdk.java.net/~lancea/8001536/webrev.00/. This adds read/writeObject as well as clone methods to SerialXLob classes. All SQE tests passed, 1 failure in the RowSet JCK/TCK tests due to a bug in the test that the TCK team is aware of and will address. JDBC Unit tests all pass . Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: Review request for 8001536 updated
Here is revised webrev taking into account Remi's suggestions http://cr.openjdk.java.net/~lancea/8001536/webrev.01/ Best, Lance On Oct 30, 2012, at 2:05 PM, Remi Forax wrote: On 10/30/2012 05:25 PM, Lance Andersen - Oracle wrote: Hi, This is a request for review of http://cr.openjdk.java.net/~lancea/8001536/webrev.00/. This adds read/writeObject as well as clone methods to SerialXLob classes. All SQE tests passed, 1 failure in the RowSet JCK/TCK tests due to a bug in the test that the TCK team is aware of and will address. JDBC Unit tests all pass . Hi Lance. In SerialBlob and in SerialClob test (obj == null) is not necessary in equals, null instanceof X is always false. in hashCode, Objects.hash() allocate an array to pass arguments to Arrays.hashCode() and box primitive values to Object. while this method is really convenient to use, each calls will allocate an array and box the two values, the overhead seems to high here. This code should be equivalent: return ((31 +Arrays.hashCode(buf)) * 31 +len) * 31 + origLen; in clone, sb should not be initialized to null and the catch should be: throw new InternalError(e), this is the standard code you can see in clone. in readObject, the test (buf.length != len) can be done before decoding the blob. in writeObject, you set blob twice, which is weird, also I think that if blob is not Serializable, the code should throw an exception, so you should not use instanceof and let s.writeFields() to throw NotSerializable exception. cheers, Rémi Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: 8002120: ProblemList.txt updates (11/2012)
looks fine On Nov 1, 2012, at 5:12 PM, Alan Bateman wrote: I need a reviewer to remove 5 tests from the exclude list. 4 of the tests were excluded temporarily during the perm gen removal work. The other one was a compiler2 bug that is long fixed. While I was there I updated TEST.ROOT to add the top-level directories for the client area to othervm.dirs. This ensures that the tests for those areas is run in jtreg othervm mode even if samevm or agentvm mode is specified to jtreg. If the tests in those areas are updated so that each test doesn't require its own VM then they can be removed from the othervm.dirs list. The patch is trivial and inlined below. -Alan diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -136,12 +136,6 @@ java/lang/management/MemoryMXBean/ResetP # 7196801 java/lang/management/MemoryMXBean/LowMemoryTest2.shgeneric-all - -# Exclude until the fix for 7195557 propagates widely. -java/lang/management/MemoryMXBean/CollectionUsageThresholdParallelGC.sh generic-all -java/lang/management/MemoryMXBean/CollectionUsageThresholdSerialGC.sh generic-all -java/lang/management/MemoryMXBean/MemoryTest.java generic-all -java/lang/management/MemoryMXBean/MemoryTestAllGC.sh generic-all # Exclude until hotspot/jdk repos are sync'd w.r.t. JAVA_MAX_SUPPORTED_VERSION # Needed when hotspot fix 7054345 is present. Remove when the JDK source is @@ -334,9 +328,6 @@ sun/security/krb5/auto/MaxRetries.java # jdk_text -# 7196199 -java/text/Bidi/Bug6665028.java generic-all - # jdk_tools diff --git a/test/TEST.ROOT b/test/TEST.ROOT --- a/test/TEST.ROOT +++ b/test/TEST.ROOT @@ -6,7 +6,7 @@ keys=2d dnd i18n keys=2d dnd i18n # Tests that must run in othervm mode -othervm.dirs=java/rmi sun/rmi javax/management +othervm.dirs=java/awt java/beans java/rmi javax/accessibility javax/imageio javax/sound javax/print javax/management com/sun/awt sun/awt sun/java2d sun/pisces sun/rmi # Tests that cannot run concurrently exclusiveAccess.dirs=java/rmi/Naming sun/management/jmxremote sun/tools/jstatd Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request 8002212 - adding read/writeObject to additional SerialXXX classes
This is similar to 8001536, just additional classes. This adds read/writeObject, equals, clone methods to additional SerialXXX classes SQE, JCK and JDBC Unit tests all pass. The webrev can be viewed at http://cr.openjdk.java.net/~lancea/8002212/webrev.00 Best Lance 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: Review request 8002212 - adding read/writeObject to additional SerialXXX classes
Hi Remi,
Thank you for the feedback
On Nov 2, 2012, at 7:42 PM, Remi Forax wrote:
On 11/02/2012 11:57 PM, Lance Andersen - Oracle wrote:
This is similar to 8001536, just additional classes.
This adds read/writeObject, equals, clone methods to additional SerialXXX
classes
SQE, JCK and JDBC Unit tests all pass.
The webrev can be viewed at
http://cr.openjdk.java.net/~lancea/8002212/webrev.00
Hi Lance,
in SerialArray.equals(), I prefer
return baseType == sa.baseType
baseTypeName.equals(sa.baseTypeName))
Arrays.equals(elements, sa.elements);
instead of
if(baseType == sa.baseType baseTypeName.equals(sa.baseTypeName)) {
return Arrays.equals(elements, sa.elements);
}
That is fine, I can make the change.
In SerialDataLink, do you really need readObject/writeObject given
that you call the default implementations.
I thought about that but had decided to add them for consistency
in SerialJavaObject, in equals, you should declare a local variable like in
SerialDataLink.equals,
even if the local varialble is used once, it's more readable.
OK
Also like in SerialArray.equals, you can do a return directly instead of
if(...) return true.
in clone(), you can use the diamond syntax,
sjo.chain = new Vector(chain);
Yeah, long story, but I forgot to reset to diamond syntax (will tell you over a
beer sometime :-) )
in setWarning(), you can use the diamond syntax as the original source does.
and in readObject, you can use the diamond syntax too.
OK
In readObject, you forget to throw an exception if there are some static
fields
in getClass().getFields() as the constructor does
(this code can be moved in a private static method).
I thought about that, but figured since it was already through that check when
the object was created, I did not think it required repeating in the readObject
method
Also, you should add a comment that because you call getClass() on obj,
there is an implicit null check.
Would it be better to put an null check in explicitly?
This can be fixed as a separated bug or not but the code of
method SerialJavaObject.getField() is weird, the code checks if fields can be
null,
but fields is never null. Also, cloning the field array is perhaps a better
idea
if the reflection implementation doesn't cache the array of fields.
I will do this in a separate bug, trying to keep things clear on the bug
In SerialRef.equals, again, if(...) return should be transformed into return
...
OK
Thank you again, will send an update webrev over the weekend
Best
Lance
Best
Lance
cheers,
Rémi
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
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: Review request 8002212 - adding read/writeObject to additional SerialXXX classes -- Updated
I revised the webrev, http://cr.openjdk.java.net/~lancea/8002212/webrev.01,
taking into account the vast majority of Remi's suggestions.
I also added SerialStruct to the webrev.
Have a great weekend.
Best
Lance
On Nov 2, 2012, at 7:42 PM, Remi Forax wrote:
On 11/02/2012 11:57 PM, Lance Andersen - Oracle wrote:
This is similar to 8001536, just additional classes.
This adds read/writeObject, equals, clone methods to additional SerialXXX
classes
SQE, JCK and JDBC Unit tests all pass.
The webrev can be viewed at
http://cr.openjdk.java.net/~lancea/8002212/webrev.00
Hi Lance,
in SerialArray.equals(), I prefer
return baseType == sa.baseType
baseTypeName.equals(sa.baseTypeName))
Arrays.equals(elements, sa.elements);
instead of
if(baseType == sa.baseType baseTypeName.equals(sa.baseTypeName)) {
return Arrays.equals(elements, sa.elements);
}
In SerialDataLink, do you really need readObject/writeObject given
that you call the default implementations.
in SerialJavaObject, in equals, you should declare a local variable like in
SerialDataLink.equals,
even if the local varialble is used once, it's more readable.
Also like in SerialArray.equals, you can do a return directly instead of
if(...) return true.
in clone(), you can use the diamond syntax,
sjo.chain = new Vector(chain);
in setWarning(), you can use the diamond syntax as the original source does.
and in readObject, you can use the diamond syntax too.
In readObject, you forget to throw an exception if there are some static
fields
in getClass().getFields() as the constructor does
(this code can be moved in a private static method).
Also, you should add a comment that because you call getClass() on obj,
there is an implicit null check.
This can be fixed as a separated bug or not but the code of
method SerialJavaObject.getField() is weird, the code checks if fields can be
null,
but fields is never null. Also, cloning the field array is perhaps a better
idea
if the reflection implementation doesn't cache the array of fields.
In SerialRef.equals, again, if(...) return should be transformed into return
...
Best
Lance
cheers,
Rémi
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
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: Review request 8002212 - adding read/writeObject to additional SerialXXX classes
On Nov 3, 2012, at 11:14 AM, Remi Forax wrote: On 11/03/2012 01:46 AM, Lance Andersen - Oracle wrote: Hi Remi, [...] In SerialDataLink, do you really need readObject/writeObject given that you call the default implementations. I thought about that but had decided to add them for consistency The serialization implementation needs to create metadata associated with readObject/writeObject, so if we can avoid to implement them, we reduce the runtime memory footprint a little. I would prefer to have a comment saying that default serialization is Ok here. OK, I will just comment the methods out and add a comment as you suggest in SerialJavaObject, in equals, you should declare a local variable like in SerialDataLink.equals, even if the local varialble is used once, it's more readable. OK Also like in SerialArray.equals, you can do a return directly instead of if(...) return true. in clone(), you can use the diamond syntax, sjo.chain = new Vector(chain); Yeah, long story, but I forgot to reset to diamond syntax (will tell you over a beer sometime :-) ) sure, now or you have to visit Paris or I have to go to NY :) Or Boston, where I am based or perhaps a JavaOne ;-) in setWarning(), you can use the diamond syntax as the original source does. and in readObject, you can use the diamond syntax too. OK In readObject, you forget to throw an exception if there are some static fields in getClass().getFields() as the constructor does (this code can be moved in a private static method). I thought about that, but figured since it was already through that check when the object was created, I did not think it required repeating in the readObject method A serialization stream can be forged to encode a SerialJavaObject that references an object that have a static field without creating a SerialJavaObject in memory. True and there are tests in the test suite that basically do that. Anyways, I added that check in the revised webrev that I pushed earlier. Also, you should add a comment that because you call getClass() on obj, there is an implicit null check. Would it be better to put an null check in explicitly? As you prefer :) [...] Thank you again, will send an update webrev over the weekend Best Lance cheers, Rémi Best Lance 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: Review request 8002212 - adding read/writeObject to additional SerialXXX classes -- Updated
On Nov 3, 2012, at 11:34 AM, Remi Forax wrote:
On 11/03/2012 03:11 PM, Lance Andersen - Oracle wrote:
I revised the webrev, http://cr.openjdk.java.net/~lancea/8002212/webrev.01,
taking into account the vast majority of Remi's suggestions.
in SerialJavaObject, hasStaticFields doesn't work, the original code doesn't
work because
it only check for fields that are declared static but not for fields that are
by example public static.
private static boolean hasStaticFields(Field[] fields) {
for (Field field : fields) {
if ( Modifier.isStatic(field.getModifiers())) {
return true;
}
}
return false;
}
This may cause compatibility issue because despite the specification, the
original code
will let objects that have a static field to be serialized.
I cannot make the change above as it breaks too many tests and I would prefer
to go with the less is more scenario. As I think I mentioned before, I do not
think the original authors really thought through these classes and thankfully
they are not used much, if at all.
Also, in readObject, if obj is null, the code should throw an IOException
because
it's not possible to create a SerialJavaObject with null has parameter
(because obj.getClass()
that implictly checks null in the constructor).
I made the change to readObject. I did not put an explicit check in the
constructor but will do that under a separate bug
I also added the comment to SerialDataLink and removed the read/writeObject
http://cr.openjdk.java.net/~lancea/8002212/webrev.02
Best
Lance
All other classes are Ok for me.
I also added SerialStruct to the webrev.
SerialStruct is Ok for me.
Have a great weekend.
Have a nice weekend too.
Best
Lance
cheers,
Rémi
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: [PATCH FOR REVIEW] ResourceManager.getApplicationResources() does not close InputStreams
Is there a reason the patch was not created originally leveraging try-with-resoruces as it seems like the perfect candidate from the webrev? I can create a bug for it, but I think I would prefer to see the patch take advantage of try-with-resoruces Best Lance On Nov 7, 2012, at 3:30 PM, Andrew Hughes wrote: IcedTea bug: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1197 com.sun.naming.internal.ResourceManager.getApplicationResources() does not close the input streams it gets from helper.getResources() and helper.getJavaHomeLibStream(). This patch: http://cr.openjdk.java.net/~andrew/pr1197/webrev.01/ adds finally blocks to close these streams. As noted in the original bug report, this has been tested on a J2EE server (JBoss) without issue. I realise that there's a possibility that try-with-resources could be used here, but the patch as posted is the one that was tested and I don't want to negate that testing by changing it. Ok for tl? If so, can I have a bug ID for this? Thanks, -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: 248BDC07 (https://keys.indymedia.org/) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07 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: [PATCH FOR REVIEW] ResourceManager.getApplicationResources() does not close InputStreams
The bug number is 8003120 Best Lance On Nov 7, 2012, at 3:30 PM, Andrew Hughes wrote: IcedTea bug: http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1197 com.sun.naming.internal.ResourceManager.getApplicationResources() does not close the input streams it gets from helper.getResources() and helper.getJavaHomeLibStream(). This patch: http://cr.openjdk.java.net/~andrew/pr1197/webrev.01/ adds finally blocks to close these streams. As noted in the original bug report, this has been tested on a J2EE server (JBoss) without issue. I realise that there's a possibility that try-with-resources could be used here, but the patch as posted is the one that was tested and I don't want to negate that testing by changing it. Ok for tl? If so, can I have a bug ID for this? Thanks, -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: 248BDC07 (https://keys.indymedia.org/) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07 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: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData
Frank, If you can please post the bug info here, I will take a look at your patch Best Lance On Nov 8, 2012, at 10:01 PM, Frank Ding wrote: Hi guys, We discovered a bug in CachedRowSetWriter.writeData method where incorrect number of conflicts is reported. I searched in Oracle bug database and no similar record was found. So I submitted a new one whose internal review ID is 2376620. A test case with code is illustrated in the bug submission that leverages PostgreSQL server but the issue is platform independent and easy to reproduce. I provided a patch review, available @ http://cr.openjdk.java.net/~dingxmin/2376620/webrev.01/ Is there anybody who is interested in patch and can also review bug 2376620? Your reply is appreciated. Best regards, Frank 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: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData
Hi Frank,
Thank you for the note
If you could in the future, please provide a complete test program to repro
the issue as it would save time with the reviews. Ideally if the issue is not
database specific it would be good to leverage Java DB as it is included within
Oracle JDK
I will look at this sometime this week
Best
Lance
On Nov 12, 2012, at 9:25 PM, Frank Ding wrote:
Hi Lance
Thanks for your quick response. Please find the bug info below.
The problem:
When CachedRowSetImpl.acceptChanges() is called, incorrect number of
conflicts, if any, is reported. The number of conflicts is the actual number
of existing rows in database, which is the size of variable 'status' defined
in CachedRowSetWriter.writeData(). It's not the conflict number that is
supposed to be.
Test case:
The bug can be easily manifested in all SQL server environment. Here take
PostgreSQL for example.
1. The sql script to create a table
CREATE TABLE ressystem.roomdescription
(
roomdescription_id serial NOT NULL,
roomdescription character varying NOT NULL,
CONSTRAINT roomdescription_pkey PRIMARY KEY (roomdescription_id)
)
2. Manually insert 3 rows
(1, Test 1)
(2, Test 2)
(3, Test 3)
3. Create a Java class that connects the established database and then
execute the following code snippet.
String query = select roomdescription_id, roomdescription from
ressystem.roomdescription;
Object[] values = {2, Test2};
rs.setCommand(query);
rs.execute(conn);
rs.moveToInsertRow();
for(int i=0; ivalues.length; i++) {
rs.updateObject(i+1,values[i]);
}
rs.insertRow();
rs.moveToCurrentRow();
rs.acceptChanges(conn);
4. An exception occurs with following output.
javax.sql.rowset.spi.SyncProviderException: 4conflicts while synchronizing
at
com.sun.rowset.internal.CachedRowSetWriter.writeData(CachedRowSetWriter.java:412)
at com.sun.rowset.CachedRowSetImpl.acceptChanges(CachedRowSetImpl.java:880)
5. In fact, there is only one conflicting row but 4 were reported.
Best regards,
Frank
On 11/9/2012 7:41 PM, Lance Andersen - Oracle wrote:
Frank,
If you can please post the bug info here, I will take a look at your patch
Best
Lance
On Nov 8, 2012, at 10:01 PM, Frank Ding wrote:
Hi guys,
We discovered a bug in CachedRowSetWriter.writeData method where incorrect
number of conflicts is reported. I searched in Oracle bug database and no
similar record was found. So I submitted a new one whose internal review
ID is 2376620. A test case with code is illustrated in the bug submission
that leverages PostgreSQL server but the issue is platform independent and
easy to reproduce.
I provided a patch review, available @
http://cr.openjdk.java.net/~dingxmin/2376620/webrev.01/
Is there anybody who is interested in patch and can also review bug
2376620? Your reply is appreciated.
Best regards,
Frank
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
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: 8003380 - Compiler warnings in logging test code
looks Ok. On Nov 14, 2012, at 4:15 PM, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8003380-logging-test-warnings/ http://cr.openjdk.java.net/%7Ejgish/Bug8003380-logging-test-warnings/ These are simple changes to eliminate compiler warnings from java.util.logging test code. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.g...@oracle.com 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: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: Hi Lance, I am very busy with other work so I can't work with the SerialBlob/SerialClob item for long time. I am very happy to refine the current test case and create new tests for SerialClob. I have create a new webre[1] for this task, please review it. [1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/ PS: If the isFree is not transient, I want to know how we add this field to the javadoc serialized form? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Adding field to BatchUpdateException
Hi,
For JDBC 4.2, I am adding methods to allow for larger update counts (request
from JDBC driver vendors) and because of this I have to tweak
BatchUpdateException
The Statement interface has the method
int[] executeBatch()
I am planning to add
long[] executeLargeBatch().
To accomodate this change, I also need to add a new field and the method
getLargeUpdateCount to BatchUpdateException.
I have exchanged emails on this with Alan and he indicated that the changes
seemed reasonable but to send a general email out to see if anything was
missed from the serialization perspective.
I have added JDBC Unit tests to validate that the serialization/deserialization
works between JDBC 4.1 and JDBC 4.2 and they run without a problem.
Best
Lance
new-host-2:sql lanceandersen$ diff BatchUpdateException.java
~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
2c2
* Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights
reserved.
---
* Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights
reserved.
27a28,31
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
83a88
this.longUpdateCounts = (updateCounts == null) ? null :
copyUpdateCount(updateCounts);
192c197
this((cause == null ? null : cause.toString()), null, 0, null, cause);
---
this((cause == null ? null : cause.toString()), null, 0, (int[])null,
cause);
295a301
this.longUpdateCounts = (updateCounts == null) ? null :
copyUpdateCount(updateCounts);
331c337,401
---
/**
* Constructs a codeBatchUpdateException/code object initialized with
* a given codereason/code, codeSQLState/code,
codevendorCode/code
* codecause/code and codeupdateCounts/code.
* p
* This constructor should be used when the returned update count may exceed
* {@link Integer.MAX_VALUE}.
* p
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of codelong/code, with each element
*indicating the update count, codeStatement.SUCCESS_NO_INFO/code or
* codeStatement.EXECUTE_FAILED/code for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* codeStatement.SUCCESS_NO_INFO/code for each SQL command in the batch
* prior to the failure for JDBC drivers that stop processing after a
command
* failure
* @param cause the underlying reason for this codeSQLException/code
* (which is saved for later retrieval by the codegetCause()/code
method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
public BatchUpdateException(String reason, String SQLState, int vendorCode,
long []updateCounts,Throwable cause) {
super(reason, SQLState, vendorCode, cause);
this.longUpdateCounts = (updateCounts == null) ? null :
Arrays.copyOf(updateCounts, updateCounts.length);
this.updateCounts = (longUpdateCounts == null) ? null :
copyUpdateCount(longUpdateCounts);
}
/**
* Retrieves the update count for each update statement in the batch
* update that executed successfully before this exception occurred.
* A driver that implements batch updates may or may not continue to
* process the remaining commands in a batch when one of the commands
* fails to execute properly. If the driver continues processing commands,
* the array returned by this method will have as many elements as
* there are commands in the batch; otherwise, it will contain an
* update count for each command that executed successfully before
* the codeBatchUpdateException/code was thrown.
* p
* This method should be used when the returned update count may exceed
* {@link Integer.MAX_VALUE}.
* p
* @return an array of codelong/code containing the update counts
* for the updates that were executed successfully before this error
* occurred. Or, if the driver continues to process commands after an
* error, one of the following for every command in the batch:
* OL
* LIan update count
* LIcodeStatement.SUCCESS_NO_INFO/code to indicate that the command
* executed successfully but the number of rows affected is unknown
* LIcodeStatement.EXECUTE_FAILED/code to indicate that the command
* failed to execute successfully
* /OL
* @since 1.8
*/
public long[] getLargeUpdateCounts() {
return (longUpdateCounts == null) ? null :
Arrays.copyOf(longUpdateCounts, longUpdateCounts.length);
}
337c407,414
private final int[] updateCounts;
---
private int[] updateCounts;
/**
* The array that describes the outcome of a batch execution.
* @serial
Re: Adding field to BatchUpdateException
Hi Joe,
Thank you for the sanity check.
I had added the following to the top of the javadoc (still playing with the
wording):
As of Java SE 8, the method getLargeUpdateCount has been added to provide
support for update counts that may be exceed Integer.MAX_VALUE and returned by
the method Statement.executeLargeBatch. A JDBC driver implementation is
required to throw BatchUpdateException(String reason, String SQLState, int
vendorCode, long []updateCounts,Throwable cause) if an error occurs during the
the execution of Statement.executeLargeBatch. If Statement.executeLargeBatch is
invoked it is recommended that getLargeUpdateCounts be called instead of
getUpdateCounts in order to avoid a possible overflow of the integer update
count.
Best
Lance
On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:
Hi Lance,
I don't see an obvious problem with the code, but I strongly suggest
documenting the correctness conditions regarding the updateCounts and
longUpdateCounts fields; I think that would ease reviewing the new
constructors and serialization code.
Cheers,
-Joe
On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:
Hi,
For JDBC 4.2, I am adding methods to allow for larger update counts (request
from JDBC driver vendors) and because of this I have to tweak
BatchUpdateException
The Statement interface has the method
int[] executeBatch()
I am planning to add
long[] executeLargeBatch().
To accomodate this change, I also need to add a new field and the method
getLargeUpdateCount to BatchUpdateException.
I have exchanged emails on this with Alan and he indicated that the changes
seemed reasonable but to send a general email out to see if anything was
missed from the serialization perspective.
I have added JDBC Unit tests to validate that the
serialization/deserialization works between JDBC 4.1 and JDBC 4.2 and they
run without a problem.
Best
Lance
new-host-2:sql lanceandersen$ diff BatchUpdateException.java
~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
2c2
* Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights
reserved.
---
* Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights
reserved.
27a28,31
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
83a88
this.longUpdateCounts = (updateCounts == null) ? null :
copyUpdateCount(updateCounts);
192c197
this((cause == null ? null : cause.toString()), null, 0, null,
cause);
---
this((cause == null ? null : cause.toString()), null, 0,
(int[])null, cause);
295a301
this.longUpdateCounts = (updateCounts == null) ? null :
copyUpdateCount(updateCounts);
331c337,401
---
/**
* Constructs a codeBatchUpdateException/code object initialized with
* a given codereason/code, codeSQLState/code,
codevendorCode/code
* codecause/code and codeupdateCounts/code.
* p
* This constructor should be used when the returned update count may
exceed
* {@link Integer.MAX_VALUE}.
* p
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of codelong/code, with each element
*indicating the update count, codeStatement.SUCCESS_NO_INFO/code or
* codeStatement.EXECUTE_FAILED/code for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* codeStatement.SUCCESS_NO_INFO/code for each SQL command in the
batch
* prior to the failure for JDBC drivers that stop processing after a
command
* failure
* @param cause the underlying reason for this codeSQLException/code
* (which is saved for later retrieval by the codegetCause()/code
method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
public BatchUpdateException(String reason, String SQLState, int
vendorCode,
long []updateCounts,Throwable cause) {
super(reason, SQLState, vendorCode, cause);
this.longUpdateCounts = (updateCounts == null) ? null :
Arrays.copyOf(updateCounts, updateCounts.length);
this.updateCounts = (longUpdateCounts == null) ? null :
copyUpdateCount(longUpdateCounts);
}
/**
* Retrieves the update count for each update statement in the batch
* update that executed successfully before this exception occurred.
* A driver that implements batch updates may or may not continue to
* process the remaining commands in a batch when one of the commands
* fails to execute properly. If the driver continues processing commands,
* the array returned by this method will have as many elements as
* there are commands in the batch; otherwise, it will contain
Re: Adding field to BatchUpdateException
Hi Ulf,
Thank you for the suggestion
The current spec is silent on what happens if the update count overflows for
any executeUpdate method. Today the driver vendors just pass in their array of
UpdateCounts (or status) to BatchException (or overflow the return count from
executeUpdate) if an error occurs at some point along the way. I really would
prefer to not change any existing behavior here at all. If application
programmers care about the update count, then they will switch to new xxxLarge
methods.
Best
Lance
On Nov 26, 2012, at 5:10 PM, Ulf Zibis wrote:
Hi Lance,
I would throw an IllegalStateException if invoking e.g. getUpdateCounts on
integer overflow.
-Ulf
Am 26.11.2012 20:44, schrieb Lance Andersen - Oracle:
Hi Joe,
Thank you for the sanity check.
I had added the following to the top of the javadoc (still playing with the
wording):
As of Java SE 8, the method getLargeUpdateCount has been added to provide
support for update counts that may be exceed Integer.MAX_VALUE and returned
by the method Statement.executeLargeBatch. A JDBC driver implementation is
required to throw BatchUpdateException(String reason, String SQLState, int
vendorCode, long []updateCounts,Throwable cause) if an error occurs during
the the execution of Statement.executeLargeBatch. If
Statement.executeLargeBatch is invoked it is recommended that
getLargeUpdateCounts be called instead of getUpdateCounts in order to avoid
a possible overflow of the integer update count.
Best
Lance
On Nov 26, 2012, at 1:51 AM, Joe Darcy wrote:
Hi Lance,
I don't see an obvious problem with the code, but I strongly suggest
documenting the correctness conditions regarding the updateCounts and
longUpdateCounts fields; I think that would ease reviewing the new
constructors and serialization code.
Cheers,
-Joe
On 11/24/2012 2:05 PM, Lance Andersen - Oracle wrote:
Hi,
For JDBC 4.2, I am adding methods to allow for larger update counts
(request from JDBC driver vendors) and because of this I have to tweak
BatchUpdateException
The Statement interface has the method
int[] executeBatch()
I am planning to add
long[] executeLargeBatch().
To accomodate this change, I also need to add a new field and the method
getLargeUpdateCount to BatchUpdateException.
I have exchanged emails on this with Alan and he indicated that the
changes seemed reasonable but to send a general email out to see if
anything was missed from the serialization perspective.
I have added JDBC Unit tests to validate that the
serialization/deserialization works between JDBC 4.1 and JDBC 4.2 and they
run without a problem.
Best
Lance
new-host-2:sql lanceandersen$ diff BatchUpdateException.java
~/NetBeansProjects/JDBC4.2/jdbc4.0/src/java/sql/
2c2
* Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights
reserved.
---
* Copyright (c) 1998, 2012, Oracle and/or its affiliates. All rights
reserved.
27a28,31
import java.io.IOException;
import java.io.InvalidObjectException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
83a88
this.longUpdateCounts = (updateCounts == null) ? null :
copyUpdateCount(updateCounts);
192c197
this((cause == null ? null : cause.toString()), null, 0, null,
cause);
---
this((cause == null ? null : cause.toString()), null, 0,
(int[])null, cause);
295a301
this.longUpdateCounts = (updateCounts == null) ? null :
copyUpdateCount(updateCounts);
331c337,401
---
/**
* Constructs a codeBatchUpdateException/code object initialized
with
* a given codereason/code, codeSQLState/code,
codevendorCode/code
* codecause/code and codeupdateCounts/code.
* p
* This constructor should be used when the returned update count may
exceed
* {@link Integer.MAX_VALUE}.
* p
* @param reason a description of the error
* @param SQLState an XOPEN or SQL:2003 code identifying the exception
* @param vendorCode an exception code used by a particular
* database vendor
* @param updateCounts an array of codelong/code, with each element
*indicating the update count, codeStatement.SUCCESS_NO_INFO/code or
* codeStatement.EXECUTE_FAILED/code for each SQL command in
* the batch for JDBC drivers that continue processing
* after a command failure; an update count or
* codeStatement.SUCCESS_NO_INFO/code for each SQL command in the
batch
* prior to the failure for JDBC drivers that stop processing after a
command
* failure
* @param cause the underlying reason for this codeSQLException/code
* (which is saved for later retrieval by the codegetCause()/code
method);
* may be null indicating the cause is non-existent or unknown.
* @since 1.8
*/
public BatchUpdateException(String reason, String SQLState, int
vendorCode,
long []updateCounts,Throwable cause) {
super
Re: Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces
On Nov 30, 2012, at 7:56 AM, Remi Forax wrote: On 11/30/2012 01:50 PM, Lance Andersen - Oracle wrote: On Nov 30, 2012, at 4:58 AM, Chris Hegarty wrote: On 30/11/2012 02:03, David Holmes wrote: On 30/11/2012 12:44 AM, Chris Hegarty wrote: On 11/29/2012 05:50 AM, David Holmes wrote: ... I don't agree that we need to describe what the default implementation does, for two reasons: 1. Normal methods don't usually specify how they are implemented - it is an implementation detail. The default simply indicates that this method does have an implementation and you should expect that implementation to obey the contract of the method. 2. It is not obvious to me that the JDK's choice for a default implementation has to be _the_ only possible implementation choice. In many/most cases there will be a very obvious choice, but that doesn't mean that all suppliers of OpenJDK classes have to be locked in to that choice. This is certainly interesting, and something I've wondered for a while now. If java.util.Iterator is to ever be fitted with a default implementation of remove ( to throw UnsupportedOperationException ), then it would clearly need to be part of the spec, and not an implementation detail of OpenJDK. Otherwise, what's the point, every developer will still have to implement it because they cannot be guaranteed of it's behavior. I think optional methods are a bit of a special case here because they don't have to work. It's the end user of a class that needs to understand if they can use remove() to actually do a removal. The developer of the class can inherit whatever default implementations Iterator provides, as long as they don't mind what they get. If they do mind ie they need a real remove(), then they will have to implement it themselves and in the process document that fact. The end user has to look at the docs for the concrete class and follow through to determine whether it's iterator().remove() is optional or not. Put another way, a default method is great for adding a new method to types that have not yet been revised to handle the new method. As a developer once you revise your class you should make a conscious implementation choice in my opinion and not rely on the default unless you truly don't care what it does. Sorry David, I've not been following lambda that closely, but (in my opinion) if default methods do not, or cannot, have defined semantics then I really think it is limiting. Maybe Iterator is a bad example, but I will continue with it anyway. In many cases developers of iterator().remove() want it to throw, if this is not defined in Iterator's default remove method then every Iterator subclass will still have to define its own remove that throws. For this particular case at least (if it were to ever happen), I would like to see specification added to remove that defines the default implementation. I had wondered about this as well and had a brief email exchange with Mike. I thought a new javadoc tag might also be something to consider. @implementation ? For JDBC, I am thinking of leveraging default methods to throw a specific exception (maybe IllegalStateException?) if the method must be implemented by the driver vendor an UnsupportedOperationException is better for that. Agree that could be a better choice or a SQLFeatureNotSupportedException for methods which may be optional based on the backend support. Rémi 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: Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces
The problem for an API such as JDBC is that the implementation is going to be specific to the driver and backend so providing a default implementation just won't work. This allows existing drivers to compile as they finish their implementation and complete their migration to the new version of the API. On Nov 30, 2012, at 8:14 AM, Ricky Clarkson wrote: What is the benefit of throwing an IllegalStateException in a default method over not providing any default method so that the compiler and runtime make sure concrete subtypes provide an implementation? On Nov 30, 2012 9:54 AM, Lance Andersen - Oracle lance.ander...@oracle.com wrote: On Nov 30, 2012, at 4:58 AM, Chris Hegarty wrote: On 30/11/2012 02:03, David Holmes wrote: On 30/11/2012 12:44 AM, Chris Hegarty wrote: On 11/29/2012 05:50 AM, David Holmes wrote: ... I don't agree that we need to describe what the default implementation does, for two reasons: 1. Normal methods don't usually specify how they are implemented - it is an implementation detail. The default simply indicates that this method does have an implementation and you should expect that implementation to obey the contract of the method. 2. It is not obvious to me that the JDK's choice for a default implementation has to be _the_ only possible implementation choice. In many/most cases there will be a very obvious choice, but that doesn't mean that all suppliers of OpenJDK classes have to be locked in to that choice. This is certainly interesting, and something I've wondered for a while now. If java.util.Iterator is to ever be fitted with a default implementation of remove ( to throw UnsupportedOperationException ), then it would clearly need to be part of the spec, and not an implementation detail of OpenJDK. Otherwise, what's the point, every developer will still have to implement it because they cannot be guaranteed of it's behavior. I think optional methods are a bit of a special case here because they don't have to work. It's the end user of a class that needs to understand if they can use remove() to actually do a removal. The developer of the class can inherit whatever default implementations Iterator provides, as long as they don't mind what they get. If they do mind ie they need a real remove(), then they will have to implement it themselves and in the process document that fact. The end user has to look at the docs for the concrete class and follow through to determine whether it's iterator().remove() is optional or not. Put another way, a default method is great for adding a new method to types that have not yet been revised to handle the new method. As a developer once you revise your class you should make a conscious implementation choice in my opinion and not rely on the default unless you truly don't care what it does. Sorry David, I've not been following lambda that closely, but (in my opinion) if default methods do not, or cannot, have defined semantics then I really think it is limiting. Maybe Iterator is a bad example, but I will continue with it anyway. In many cases developers of iterator().remove() want it to throw, if this is not defined in Iterator's default remove method then every Iterator subclass will still have to define its own remove that throws. For this particular case at least (if it were to ever happen), I would like to see specification added to remove that defines the default implementation. I had wondered about this as well and had a brief email exchange with Mike. I thought a new javadoc tag might also be something to consider. For JDBC, I am thinking of leveraging default methods to throw a specific exception (maybe IllegalStateException?) if the method must be implemented by the driver vendor or a SQLFeatureNotSupportedException for methods which may be optional based on the backend support. -Chris. But maybe we kid ourselves when we give this illusion of flexibility in implementation. David -Chris. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData
I will get to it sometime within the next week. Have some higher priority
items to address first
Best
Lance
On Dec 3, 2012, at 3:17 AM, Frank Ding wrote:
Hi Lance,
Thanks for your clarification. I created the test case you requested and
attached it in this email. Please review it. By the way, the new Oracle bug
(internal id 2376620) submitted by me several days ago seems not having been
reviewed. Could you also help me on this?
Best regards,
Frank
On 11/30/2012 8:40 PM, Lance Andersen - Oracle wrote:
Hi Frank,
Thank you for the email. No we do not want tests that require database
access in jtreg.
What I was trying to say, albeit not probably as clear as it could have been
is that it would be helpful to provide a complete example and to use Java DB
as the database if it is a generic data access issue as while it is not a
required part of the Java SE specification, we do provide it with the Oracle
JDK so it makes it easier to test against for developers vs finding an
instance of DB2, Sybase or even Oracle to test against.
Best
Lance
On Nov 30, 2012, at 12:26 AM, Frank Ding wrote:
Hi Lance,
Sorry for late response and thanks for your comment. You mean I can write
a jtreg test case that connects to Java DB? I can do that.
Best regards,
Frank
On 11/13/2012 10:13 PM, Lance Andersen - Oracle wrote:
Hi Frank,
Thank you for the note
If you could in the future, please provide a complete test program to
repro the issue as it would save time with the reviews. Ideally if the
issue is not database specific it would be good to leverage Java DB as it
is included within Oracle JDK
I will look at this sometime this week
Best
Lance
On Nov 12, 2012, at 9:25 PM, Frank Ding wrote:
Hi Lance
Thanks for your quick response. Please find the bug info below.
The problem:
When CachedRowSetImpl.acceptChanges() is called, incorrect number of
conflicts, if any, is reported. The number of conflicts is the actual
number of existing rows in database, which is the size of variable
'status' defined in CachedRowSetWriter.writeData(). It's not the
conflict number that is supposed to be.
Test case:
The bug can be easily manifested in all SQL server environment. Here
take PostgreSQL for example.
1. The sql script to create a table
CREATE TABLE ressystem.roomdescription
(
roomdescription_id serial NOT NULL,
roomdescription character varying NOT NULL,
CONSTRAINT roomdescription_pkey PRIMARY KEY (roomdescription_id)
)
2. Manually insert 3 rows
(1, Test 1)
(2, Test 2)
(3, Test 3)
3. Create a Java class that connects the established database and then
execute the following code snippet.
String query = select roomdescription_id, roomdescription from
ressystem.roomdescription;
Object[] values = {2, Test2};
rs.setCommand(query);
rs.execute(conn);
rs.moveToInsertRow();
for(int i=0; ivalues.length; i++) {
rs.updateObject(i+1,values[i]);
}
rs.insertRow();
rs.moveToCurrentRow();
rs.acceptChanges(conn);
4. An exception occurs with following output.
javax.sql.rowset.spi.SyncProviderException: 4conflicts while synchronizing
at
com.sun.rowset.internal.CachedRowSetWriter.writeData(CachedRowSetWriter.java:412)
at
com.sun.rowset.CachedRowSetImpl.acceptChanges(CachedRowSetImpl.java:880)
5. In fact, there is only one conflicting row but 4 were reported.
Best regards,
Frank
On 11/9/2012 7:41 PM, Lance Andersen - Oracle wrote:
Frank,
If you can please post the bug info here, I will take a look at your
patch
Best
Lance
On Nov 8, 2012, at 10:01 PM, Frank Ding wrote:
Hi guys,
We discovered a bug in CachedRowSetWriter.writeData method where
incorrect number of conflicts is reported. I searched in Oracle bug
database and no similar record was found. So I submitted a new one
whose internal review ID is 2376620. A test case with code is
illustrated in the bug submission that leverages PostgreSQL server but
the issue is platform independent and easy to reproduce.
I provided a patch review, available @
http://cr.openjdk.java.net/~dingxmin/2376620/webrev.01/
Is there anybody who is interested in patch and can also review bug
2376620? Your reply is appreciated.
Best regards,
Frank
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
/*
* Copyright (c) 2012 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
Re: Request for Review : CR#8004015 : [2nd pass] Add interface extends and defaults for basic functional interfaces
I am still wondering if we need some sort of javadoc tag for default implementations so that it will stand out better and allow us to be consistent with how we specify this across Java SE and other APIs that leverage default methods. Has any thought been given to this? Best Lance On Dec 5, 2012, at 12:47 AM, Mike Duigou wrote: Hello all; I have updated the proposed patch. The changes primarily add class and method documentation regarding handling of null for the primitive specializations. http://cr.openjdk.java.net/~mduigou/8004015/1/webrev/ http://cr.openjdk.java.net/~mduigou/8004015/1/specdiff/java/util/function/package-summary.html I've also reformatted the source for the default methods. Mike On Nov 26 2012, at 18:12 , Mike Duigou wrote: Hello all; In the original patch which added the basic lambda functional interfaces, CR#8001634 [1], none of the interfaces extended other interfaces. The reason was primarily that the javac compiler did not, at the time that 8001634 was proposed, support extension methods. The compiler now supports adding of method defaults so this patch improves the functional interfaces by filing in the inheritance hierarchy. Adding the parent interfaces and default methods allows each functional interface to be used in more places. It is especially important for the functional interfaces which support primitive types, IntSupplier, IntFunction, IntUnaryOperator, IntBinaryOperator, etc. We expect that eventually standard implementations of these interfaces will be provided for functions like max, min, sum, etc. By extending the reference oriented functional interfaces such as Function, the primitive implementations can be used with the boxed primitive types along with the primitive types for which they are defined. The patch to add parent interfaces and default methods can be found here: http://cr.openjdk.java.net/~mduigou/8004015/0/webrev/ http://cr.openjdk.java.net/~mduigou/8004015/0/specdiff/java/util/function/package-summary.html Mike [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c2e80176a697 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
signatures that are recorded for default methods
Folks, Will the signatures for interfaces that are recorded by the TCKs for interfaces record the fact that a method includes a default method? or will it just record the method definition? I am assuming it will, but I know there has been discussion that a implementor could choose a different default implementation on one of the recent threads that was up for review. I am still trying to understand what our guidelines are, if any for documenting the behavior of the supplied default methods given the javadocs are part of the specification for many APIs (and in some case the only spec). Best Lance 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: Request for review: JDK-8004337: java/sql tests aren't run in test/Makefile
Looks fine Rob On Dec 6, 2012, at 4:22 PM, Rob McKenna wrote: Hi folks, There's a missing folder in the jdk_other test target: http://cr.openjdk.java.net/~robm/8004337/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8004337/webrev.01/ -Rob 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: Review needed: 8004374 : Fwd: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData
Hi Frank, As I explained in one of my earlier emails, tests that require a database will not be added to jtreg. I have a unit test suite which i use for that but that is not external Best Lance On Dec 10, 2012, at 2:44 AM, Frank Ding wrote: Hi Lance, The code refactory looks good. By the way, the newly added unit test is not jtreg test case? Best regards, Frank On 12/5/2012 4:38 AM, Lance Andersen - Oracle wrote: All, Attached is the patch for: 8004374 based off the issue that Frank reported. for http://cr.openjdk.java.net/~lancea/8004374/webrev.00/ http://cr.openjdk.java.net/%7Elancea/8004374/webrev.00/ The TCK, SQE and the JDBC Unit Tests run clean. I added a new Unit Test to validate the issue. Frank, I did not use your fix as I was able to clean the code up a bit more and get rid of more crud while addressing it. It is similar though. Best Lance http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request 8004357: Implement various methods in SerialBlob/Clob/Array and specify Thread Safety
Need a reviewer for 8004357:Implement various methods in SerialBlob/Clob/Array and specify Thread Safety This defines thread safety adds missing methods to SerialBlob/Clob/Array The CCC request has been reviewed. The changes uncovered a couple of bugs in the JCK which the JCK team is going to address in the JCK and RowSet TCK. JDBC Unit tests and SQE tests all continue to pass. http://cr.openjdk.java.net/~lancea/8004357/webrev.00/ Best Lance 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: 8005051: default methods for Iterator
On Dec 14, 2012, at 7:28 AM, Alan Bateman wrote: On 14/12/2012 01:24, Akhil Arora wrote: As part of the library lambdafication, this patch adds a forEach default method to Iterator, and converts remove() into a default method so that implementations of Iterator no longer have to override remove if they desire the default behavior, which is to throw an UnsupportedOperationException. http://cr.openjdk.java.net/~akhil/8005051.0/webrev/ I looked at the changes to Iterator, a few minor comments: I think it would help to change This default implementation to The default implementation, it makes it more obviously normative (and so testable) and might help for sub-types that override the method but don't inherit the javadoc. I assume the generated javadoc ends as a long paragraph, did you consider putting in p tags to make it a bit easier on the reader, minimally put the description of the default implementation isn't own paragraph. This is why i am a proponent of adding a javadoc tag for default methods so that everyone is consistent in where/how we document the default method. I worry if we do not developers will place this info in various places making it easy to overlook. Should Collection have a lowercase c as it may be an iterator over a collection that is not a java.util.Collection? In forEach then it may be smoother to change ... execution subsequent ... to ... execution then subsequent As per the other thread, if new methods are coming with the public modifier then I think it should be added to the existing methods. -Alan. 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) 8003260 : some fields should be package protected
Hi Joe,
Shouldn't this also be private:
static final char [] xmlDecl = {'','?','x','m','l'};
otherwise it is fine
Best
Lance
On Dec 14, 2012, at 1:33 PM, Joe Wang wrote:
Hi,
This is one of the three [findbug] issues. I've checked with Drew. None of
them are vulnerabilities. Nonetheless, the fields should have been private or
package private.
webrev:
http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
Test:
No new test. Existing regression tests passed.
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) 8003260 : some fields should be package protected
Thanks Joe. maybe a quick comment would help in the code could be useful
Best
Lance
On Dec 14, 2012, at 2:49 PM, Joe Wang wrote:
On 12/14/2012 10:36 AM, Lance Andersen - Oracle wrote:
Hi Joe,
Shouldn't this also be private:
static final char [] xmlDecl = {'','?','x','m','l'};
A subclass in the same package, XMLDocumentScannerImpl, referred to it.
That's why I made it package private.
Joe
otherwise it is fine
Best
Lance
On Dec 14, 2012, at 1:33 PM, Joe Wang wrote:
Hi,
This is one of the three [findbug] issues. I've checked with Drew. None of
them are vulnerabilities. Nonetheless, the fields should have been private
or package private.
webrev:
http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
Test:
No new test. Existing regression tests passed.
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
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) 8003260 : some fields should be package protected
+1
On Dec 14, 2012, at 3:43 PM, Joe Wang wrote:
Thanks. I added a comment. Here's the webrev again:
http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
Best
Joe
On 12/14/2012 11:52 AM, Lance Andersen - Oracle wrote:
Thanks Joe. maybe a quick comment would help in the code could be useful
Best
Lance
On Dec 14, 2012, at 2:49 PM, Joe Wang wrote:
On 12/14/2012 10:36 AM, Lance Andersen - Oracle wrote:
Hi Joe,
Shouldn't this also be private:
static final char [] xmlDecl = {'','?','x','m','l'};
A subclass in the same package, XMLDocumentScannerImpl, referred to it.
That's why I made it package private.
Joe
otherwise it is fine
Best
Lance
On Dec 14, 2012, at 1:33 PM, Joe Wang wrote:
Hi,
This is one of the three [findbug] issues. I've checked with Drew. None
of them are vulnerabilities. Nonetheless, the fields should have been
private or package private.
webrev:
http://cr.openjdk.java.net/~joehw/7u12/8003260/webrev/
Test:
No new test. Existing regression tests passed.
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
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
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: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
Deven, This was pushed a while ago you should have seen the putback on the alias See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a8978a5bb6e Best Lance On Jan 10, 2013, at 12:49 AM, Deven You wrote: Hi Lance, Is there any update for this issue. If you have anything I can do, please let me know. Thanks a lot! On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote: It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: Hi Lance, I am very busy with other work so I can't work with the SerialBlob/SerialClob item for long time. I am very happy to refine the current test case and create new tests for SerialClob. I have create a new webre[1] for this task, please review it. [1] http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/ PS: If the isFree is not transient, I want to know how we add this field to the javadoc serialized form? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: javax.sql.rowset.serial.SerialBlob doesn't support free and getBinaryStream
The JCK and RowSet TCK will have base tests to validate. I have a separate set of unit tests for JDBC which includes more detailed tests than what you had suggested as well as end to end tests. Best Lance On Jan 10, 2013, at 8:24 AM, Deven You wrote: Hi Lance, I am glad to see the patch for implementing these methods are committed. Do you have a plan to add the test cases[1] I created too? Thanks a lot! [1]http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ On 01/10/2013 08:31 PM, Lance Andersen - Oracle wrote: Deven, This was pushed a while ago you should have seen the putback on the alias See http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7a8978a5bb6e Best Lance On Jan 10, 2013, at 12:49 AM, Deven You wrote: Hi Lance, Is there any update for this issue. If you have anything I can do, please let me know. Thanks a lot! On 11/24/2012 12:45 AM, Lance Andersen - Oracle wrote: It is on my list. to update the javadocs I need a ccc which I have not done yet and is needed as part of this change On Nov 23, 2012, at 3:07 AM, Deven You wrote: Hi Lance, Is there any plan for this issue, if any could you update to me? Thanks a lot! On 10/29/2012 06:39 PM, Lance Andersen - Oracle wrote: Hi Deven, I will address the needed updates a bit later. Thank you for your input Best Lance On Oct 29, 2012, at 3:51 AM, Deven You wrote: Hi Alan, The Java Spec does not mention the thread safe for JDBC API. But I see the other code in SerialBlob/SerialClob have not consider it. I think use buff == null to replace isFree is a good idea because it also avoid the problem for the condition buf == null isFree == false so we won't need create a readObject method. Thanks for your suggestion for isFree, I will correct it later. Lance: How about your suggestion? Since you mentioned you will develop the implementation yourself. I use my implementation mainly for the test cases. But you may also take a look my implementation. Thanks a lot! On 09/21/2012 04:37 PM, Alan Bateman wrote: On 21/09/2012 04:21, Deven You wrote: Hi Lance, I am very busy with other work so I can't work with the SerialBlob/SerialClob item for long time. I am very happy to refine the current test case and create new tests for SerialClob. I have create a new webre[1] for this task, please review it. [1]http://cr.openjdk.java.net/~youdwei/OJDK-576/webrev.01/ http://cr.openjdk.java.net/%7Eyoudwei/OJDK-576/webrev.01/ PS: If the isFree is not transient, I want to know how we add this field to the javadoc serialized form? I don't know very much about the rowset API and I can't see anything to specify whether it is meant to be safe for use by concurrent threads. There are clearly lots of issues here and implementing free introduces a lot more, especially with the possibility of an asynchronous free or more than one thread calling free at around the same time. Have you considered buf == null to mean that the resources are freed? That might avoid needing to change the serialized form. Also as these types are serializable it means you have to consider the case where you deserialize to buf == null isFree == false for example. On that point, it looks to me that this code needs a readObject anyway (for several reasons). A small point is that isFree is a odd name for a method that doesn't return a boolean. If the patch goes ahead then I think it needs a better name, ensureNotFree or requireNotFree or something like that. -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com mailto:lance.ander...@oracle.com http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Review request 8005080: JDBC 4.2
The following webrev has the bulk of the JDBC 4.2 changes: http://cr.openjdk.java.net/~lancea/8005080/webrev.00/ There will be additional updates to java.sql.Date/TIme/Timestamp (by Sherman) once JSR 310 is integrated to aide in moving to and from the new date time datatypes. I will also probably have one more set of minor changes once 310 is integrated. The CCC has been approved for these changes the specdiff of the javadocs is still available at http://cr.openjdk.java.net/~lancea/8005080/specdiffs.02/ (note some of the changes highlighted were put back previously) Best Lance 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: javax.xml.validation: Using ServiceLoader to load JAXP schema factories (7169894: JAXP Plugability Layer: using service loader)
On Jan 11, 2013, at 2:05 PM, Joe Wang wrote: On 1/11/2013 8:58 AM, Alan Bateman wrote: On 09/01/2013 14:28, Daniel Fuchs wrote: Hi, Here is a new webrev in the series that addresses using ServiceLoader in JAXP for JDK 8. 7169894: JAXP Plugability Layer: using service loader This changeset addresses modification in the javax.xml.validation package. It is a bit more complex than the changes required for the other packages because the newInstance methods takes an additional schemaLanguage parameter, and therefore we need to loop over the providers in order to find one that supports the language. http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.validation/webrev.00/ Also this particular package did not have any specific configuration error to throw in case of misconfiguration (the xpath package in which the logic is very similar had one for instance), so we're adding a new SchemaFactoryConfigurationError for that purpose. I've taken an initial look at this and I'm wondering about SchemeFactory.newInstance throwing SchemaFactoryConfigurationError. Technically this is an incompatible change but in practical terms it may not be concern as this provider interface may not be used very much. Joe Wang - have you come across SchemaFactory implementations, I'm trying to get a feel for how much this is used, if ever. I don't have any data on how much the service mechanism may be used, Xerces would surely be the one most frequently used. I'm more concerned with the spec change that would require TCK change (the addition of SchemaFactoryConfigurationError related tests). Would that require MR? We probably need to run it with the JCK engineers. If you are changing the exception, then you would need an MR for this and want to update any TCK/JCK tests (or add tests) Joe -Alan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Hi This is a review request for 8006139 which adds missing methods to SQLInput/Output The webrev can be found at http://cr.openjdk.java.net/~lancea/8006139/webrev.00/ best Lance 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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -Alan 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: 8006344: Broken javadoc link in javax.lang.model.element.Element
+1
On Jan 15, 2013, at 12:59 PM, Chris Hegarty wrote:
Minor oversight in the changes from 7193719: Support repeating annotations
in javax.lang.model.
~/repos/jdk8/tl/adder/build/solaris-i586/impsrc/javax/lang/model/element/Element.java:199:
warning - Tag @see: can't find getAnnotation() in
javax.lang.model.element.Element
I only noticed when building javadocs for another issue, thankfully we don't
have many warnings here ;-)
diff -r d54b4a091450 src/share/classes/javax/lang/model/element/Element.java
--- a/src/share/classes/javax/lang/model/element/Element.java Mon Jan 14
14:17:25 2013 -0800
+++ b/src/share/classes/javax/lang/model/element/Element.java Tue Jan 15
17:55:17 2013 +
@@ -188,7 +188,7 @@ public interface Element {
* type if present on this element, else an empty array
*
* @see #getAnnotationMirrors()
- * @see #getAnnotation()
+ * @see #getAnnotation(java.lang.Class)
* @see java.lang.reflect.AnnotatedElement#getAnnotations
* @see EnumConstantNotPresentException
* @see AnnotationTypeMismatchException
-Chris.
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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Thank you Ulf. I deleted the extra line on 579 Best Lance On Jan 15, 2013, at 3:45 PM, Ulf Zibis wrote: Looks great! Little nit: SQLOutputImpl.java line 579 could be dropped. -Ulf Am 15.01.2013 17:48, schrieb Lance Andersen - Oracle: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -Alan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com 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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
On Jan 19, 2013, at 9:43 AM, Alan Bateman wrote: On 15/01/2013 16:48, Lance Andersen - Oracle wrote: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. It would be good to get the ClassCastException specified when you get time. Yes will do once the ccc is approved on my list I looked at the latest webrev and it seems okay to me. SQLInputImpl.getNextAttribute would be bit better if it only incremented idx after retrieving an attribute but that is existing code. Minor intent issue in SQLOutputImpl. writeNClob. fixed the spacing. going to leave idx alone for now as but will look at this for the future Best Lance -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
