Thanks Seán!
I added an example of stacktrace to the description.
With kind regards,
Ivan
On 30.06.2016 17:31, Seán Coffey wrote:
Looks fine to me Ivan.
If you have a stacktrace of an example ConcurrentModificationException
issue, please paste it into the bug report so that it's documented for
others to find.
Regards,
Sean.
On 24/06/2016 15:56, Ivan Gerasimov wrote:
Hi Aleksey!
I've double-checked the Pool class and found no other code that has
to be additionally synchronized.
Please note that the method expungeStaleConnections() is thread-safe
and can be called from outside the synchronized blocks.
The method Pool.getConnections(Object) accesses the `map` field, but
it is only called from synchronized blocks, so it's fine.
Please let me know, if I'm missing something obvious.
With kind regards,
Ivan
On 21.06.2016 19:25, Ivan Gerasimov wrote:
On 21.06.2016 18:43, Aleksey Shipilev wrote:
On 06/21/2016 06:14 PM, Ivan Gerasimov wrote:
Hello!
The Pool has a member `map`, which is accessed from different
threads,
thus the access is synchronized.
However, in some code paths (mostly in debug printing) it is accessed
without proper synchronization, which results in intermediate
ConcurrentModificationException when the debug output is turned on.
Would you please help review the fix?
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8159822
WEBREV: http://cr.openjdk.java.net/~igerasim/8159822/00/webrev/
Missed synchronized-s:
154 private Connections getConnections(Object id) {
155 ConnectionsRef ref = map.get(id);
156 return (ref != null) ? ref.getConnections() : null;
157 }
...gets called via:
168 public void expire(long threshold) {
169 synchronized (map) {
...
179 }
180 expungeStaleConnections();
181 }
...and also via:
188 private static void expungeStaleConnections() {
...
192 Connections conns = releaseRef.getConnections();
...
203 }
204 }
But this is ConnectionsWeakRef.getConnections() not
Pool.getConnections(Object).
As far as I can see, an instance of ConnectionsWeakRef cannot be
accessed concurrently.
...
117 public PooledConnection getPooledConnection(Object id, long
timeout,
118 PooledConnectionFactory factory) throws
NamingException {
...
// no synchronized prior here
127 expungeStaleConnections();
...
expungeStaleConnections() should be safe to call concurrently as
long as ReferenceQueue.poll() is thread-safe.
With kind regards,
Ivan
