On 13 June 2011 16:39, Mark Thomas <ma...@apache.org> wrote: > 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.
Indeed, but the idea is to prevent accidental changes in the code itself, as well as document that the lock needs to be immutable. > Mark > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
