On 13/06/2011 16:22, sebb wrote: > On 13 June 2011 15:20, <ma...@apache.org> wrote: >> Author: markt >> Date: Mon Jun 13 14:20:56 2011 >> New Revision: 1135111 >> >> URL: http://svn.apache.org/viewvc?rev=1135111&view=rev >> Log: >> Remove deprecation of setFactory method >> >> Modified: >> >> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java >> >> Modified: >> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1135111&r1=1135110&r2=1135111&view=diff >> ============================================================================== >> --- >> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java >> (original) >> +++ >> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java >> Mon Jun 13 14:20:56 2011 >> @@ -1479,30 +1479,30 @@ public class GenericObjectPool<T> extend >> } >> >> /** >> - * Sets the {@link PoolableObjectFactory factory} this pool uses to >> create >> - * new instances. Trying to change the <code>factory</code> while there >> are >> - * borrowed objects will throw an {@link IllegalStateException}. If >> there >> - * are instances idle in the pool when this method is invoked, these >> will be >> - * destroyed using the original factory. >> + * <p>Sets the poolable object factory associated with this pool.</p> >> + * >> + * <p>If this method is called when the factory has previously been >> set an >> + * IllegalStateException is thrown.</p> >> * >> * @param factory >> * the {@link PoolableObjectFactory} used to create new >> * instances. >> - * @throws IllegalStateException >> - * when the factory cannot be set at this time >> - * @deprecated to be removed in version 2.0 >> + * @throws IllegalStateException if the factory has already been set >> */ >> @Override >> - @Deprecated >> public void setFactory(PoolableObjectFactory<T> factory) >> throws IllegalStateException { >> - assertOpen(); >> - if (0 < getNumActive()) { >> - throw new IllegalStateException("Objects are already active"); >> + if (this._factory == null) { >> + synchronized (factoryLock) { >> + if (this._factory == null) { > > This is double-checked locking, which won't work reliably unless > _factory is volatile.
Yep. I'll fix that. > But why bother with the double check? We have to have the sync so may as well fail fast where we can. > Locks should be final. > If they are mutable, they can fail as locks. I'll fix that too although a client is going to have to go to a fair amount of trouble to actually change that. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
