Re: JDBC bug: Incorrect number of conflicts is reported by CachedRowSetWriter.writeData
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 redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty
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
hg: jdk8/tl/jdk: 8004184: security tests leave JSSEServer running
Changeset: ead651efb271 Author:xuelei Date: 2012-12-03 06:00 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ead651efb271 8004184: security tests leave JSSEServer running Summary: Use othervm mode to release resources, and correct the system properties issues in JSSE Reviewed-by: chegar ! test/sun/security/pkcs11/sslecc/ClientJSSEServerJSSE.java
Re: RFR: 8003596 CheckLockLocationTest-Windows-fix
Thanks. Could you please push the change. Jim On 12/01/2012 10:44 AM, Alan Bateman wrote: On 30/11/2012 23:19, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8003596-CheckLockLocationTest-Windows-fix/ http://cr.openjdk.java.net/%7Ejgish/Bug8003596-CheckLockLocationTest-Windows-fix/ Summary: fixes test when running on Windows so that test that requires setWritable is not run, because Windows does not support setWritable. Thanks, Jim Looks okay to me although if (!isWindows()) to if (!ON_WINDOWS) might be neater. An alternative way to do this would be just to handle check the return from setWritable rather than failing. It is possible to change the deny adding entries to directories with the new file system API but it's probably not worth using it here. -Alan. -- 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
CFR: javax.xml.parsers: Using ServiceLoader to load JAXP parser factories (7169894: JAXP Plugability Layer: using service loader)
Hi, This is a first webrev in a series that addresses a change intended for JDK 8: 7169894: JAXP Plugability Layer: using service loader I've been building up on Joe's work and Paul Alan comments from an earlier discussion thread [1] This here addresses changes in the javax.xml.parsers package for the SAXParserFactory and DocumentBuilderFactory - and more specifically their no-argument newInstance() method. This change replaces the custom code that was reading the META-INF/services/ resources by a simple call to ServiceLoader. http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.parsers/webrev.00/ In addition - since it is foreseen that the default implementation for the parsers might come as a separate Module in the future, we cannot make the assumption that the first service provider encountered by the ServiceLoader will be the third-party/custom provider to use. As a consequence, since we must allow for a third-party/custom provider to override the default implementation - we will skip over any provider whose class name has the default implementation class name, and return it only if no other provider is found. This is a small spec change intended for JDK8 which is reflected in the API Documentation of the two factory classes. The impact is that in the very rare configuration where you have on the classpath two providers: A.jar:B.jar, where A.jar has a services/* config that points at the default factory implementation, and B.jar points at a custom implementation, then the new XxxxFactory will return the custom implementation pointed at by B.jar, whereas it used to return the default implementation pointed at by A.jar. best regards, -- daniel references: [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-June/010595.html
AnnotationType AnnotationSupport + repeating annotations
Hi David and Alan, The following is in part connected with the new code for repeating annotations, and in part with the proposed patch for resolving synchronization issues with annotations as well as improvements regarding space and multi-threaded contention. Although connected, the patch proposed here can be taken entirely by itself. Let me try to explain what this patch does. This patch removes the synchronized keyword from the static method AnnotationType.getInstance(Class). By itself this synchronization does not present any big performance issue since the method is always called on slow-path (when parsing the annotations, when lazily constructing a Map of inherited annotations, when de-serializing annotations). The reason this method is synchronized on a single monitor is because it: - lazily constructs an instance of AnnotationType and exposes it to other threads via a Class.annotationType field - in the middle of the AnnotationType constructor it exposes a half-constructed instance via a Class.annotationType field to current thread so that recursive calls don't result in infinite recursion. As current parsing/resolving logic is coded, other threads must not see the half-constructed AnnotationType instance and current thread must see it. This is achieved by single re-entrant lock because only single lock guarantees the absence of dead-locks (as can be seen from bugs this lock in combination with initAnnotationsIfNecessary() is a cause for dead-locks, but that's another story). Now because there is a single lock, the method must not be called on heavily executed code paths or this will present a synchronization bottleneck. One such heavily executed code path is in the new sun.reflect.annotation.AnnotationSupport class that contains the logic for repeating annotations. In this class the AnnotationType for a particular annotation class is not obtained via this synchronized method, but incorrectly via direct unsynchronized read of Class.annotationType field. The code in AnnotationSupport can therefore dereference a half-constructed AnnotationType instance before it's constructor, executing in another thread, is finished and before final fields in object are frozen. Class.[get,set]AnnotationType should only be called from within the synchronized AnnotationType.getInstance method, but that apparently is to contended to be practical. I solved the problem by: - making AnnotationType an immutable object (all fields final) - exposing the instance to other threads via an unsynchronized write to Class.annotationType field only after constructor is finished and final fields are frozen - exposing the instance to current thread for recursive calls in the middle of the constructor via a special: private static final ClassValueThreadLocalAnnotationType IN_CONSTRUCTION = ... field. It's true, this does present an overhead in storage, since every Class instance for annotation type will have a ClassValue.ClassValueMap installed, but it is hoped that the number of different annotation classes is not big compared to the number of all classes. A ThreadLocal referenced by ClassValue is only set for the in-flight recursive call and cleared afterwards. Because with such non-blocking access to AnnotationType, AnnotationType.getInstance() can be used in AnnotationSupport properly to quickly access the AnnotationType instances. The access to AnnotationType with this patch is so quick that I added 2 fields to it (container, containee) where the container and/or containee for a particular annotation type are cached and used in AnnotationSupport to resolve repeating annotations. This is much faster than invoking Class.getDirectDeclaredAnnotation() which is a HashMap.get, followed by reflective invocation of the value method on that annotation. The patch is here: http://dl.dropbox.com/u/101777488/jdk8-tl/AnnotationTypeSupport/webrev.01/index.html The benchmarks that show improvement are the same benchmarks used in my related proposed patch (Class.getAnnotations() bottleneck): https://raw.github.com/plevart/jdk8-tl/JEP-149/test/src/test/ReflectionTest.java The results are combined results using plain JDK8 code with repeating annotation and then one patch applied at a time and finally both patches combined: https://raw.github.com/plevart/jdk8-tl/JEP-149/test/benchmark_results_i7-2600K.txt Regards, Peter P.S. Maybe this is not the best approach. Another approach would be to construct a special non-recursive variant of annotation parsing logic that would be used only for select meta-annotations - just enough to construct an AnnotationType with all it's data. The proposed patch is trying to keep the semantics of original logic, which is not entirely correct. The patch is not trying to solve the logic in-correctness. Here's a contrived example that exposes the in-correctness: Let's have the following two annotations:
