commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1135111 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Date Mon, 13 Jun 2011 15:39:11 GMT
On 13/06/2011 16:22, sebb wrote:
> On 13 June 2011 15:20,  <markt@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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message