Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

critical path in lookup of DBPortPool #67

Closed
wants to merge 1 commit into from

3 participants

@gabrielenizzoli

critical path in lookup of DBPortPool: if 2 threads pass by and find no
DBPortPool for the same addr, then 2 DBPortPool objects will be created
and returned, but only one of them is actually stored in the _pool map
(side effect is that on close() only one DBPortPool will be closed);
note that this patch will make the _pool object a non synchronized map
to avoid double synchronization

@gabrielenizzoli gabrielenizzoli critical path in lookup of DBPortPool: if 2 threads pass by and find no
DBPortPool for the same addr, then 2 DBPortPool objects will be created
and returned, but only one of them is actually stored in the _pool map
(side effect is that on close() only one DBPortPool will be closed);
note that this patch will make the _pool object a non synchronized map
to avoid double synchronization
7fdbf94
@jyemin jyemin was assigned
@trishagee

Apologies for the delay in response. I agree that your code would definitely work, but the race condition you talk about is not actually a problem - the existing second check inside the synchonized block will stop the second thread from creating a new pool. That first check is an optimisation to allow a fast, non-synchronized access to the collection to return quickly in the case that a pool currently exists.

I agree that this code is not very readable and can certainly be simplified, and my colleagues and I are in the process of doing just that - the 3.0 branch has a much cleaner architecture for managing connections, and simpler code too. Feel free to take a look.

@trishagee trishagee closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 16, 2012
  1. @gabrielenizzoli

    critical path in lookup of DBPortPool: if 2 threads pass by and find no

    gabrielenizzoli authored
    DBPortPool for the same addr, then 2 DBPortPool objects will be created
    and returned, but only one of them is actually stored in the _pool map
    (side effect is that on close() only one DBPortPool will be closed);
    note that this patch will make the _pool object a non synchronized map
    to avoid double synchronization
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 10 deletions.
  1. +5 −10 src/main/com/mongodb/DBPortPool.java
View
15 src/main/com/mongodb/DBPortPool.java
@@ -54,16 +54,11 @@
DBPortPool get( ServerAddress addr ){
- DBPortPool p = _pools.get( addr );
-
- if (p != null)
- return p;
-
synchronized (_pools) {
- p = _pools.get( addr );
- if (p != null) {
+ DBPortPool p = _pools.get( addr );
+
+ if (p != null)
return p;
- }
p = new DBPortPool( addr , _options );
_pools.put( addr , p);
@@ -85,9 +80,9 @@ DBPortPool get( ServerAddress addr ){
}
}
+ return p;
}
- return p;
}
void close(){
@@ -115,7 +110,7 @@ private ObjectName createObjectName( ServerAddress addr ) throws MalformedObject
}
final MongoOptions _options;
- final Map<ServerAddress,DBPortPool> _pools = Collections.synchronizedMap( new HashMap<ServerAddress,DBPortPool>() );
+ final Map<ServerAddress,DBPortPool> _pools = new HashMap<ServerAddress,DBPortPool>();
final MBeanServer _server;
}
Something went wrong with that request. Please try again.