commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1135111 - /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
Date Mon, 13 Jun 2011 15:22:14 GMT
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.

But why bother with the double check?

> +                    this._factory = factory;
> +                } else {
> +                    throw new IllegalStateException("Factory already set");
> +                }
> +            }
>         } else {
> -            clear();
> +            throw new IllegalStateException("Factory already set");
>         }
> -        _factory = factory;
>     }
>
>     /**
> @@ -1998,6 +1998,7 @@ public class GenericObjectPool<T> extend
>
>     /** My {@link PoolableObjectFactory}. */
>     private PoolableObjectFactory<T> _factory;

Needs to be volatile to ensure visibility to other threads, both for
the locking and for subsequent reads by other threads.

> +    private Object factoryLock = new Object();

Locks should be final.
If they are mutable, they can fail as locks.

>
>     /**
>      * My idle object eviction {@link TimerTask}, if any.
> @@ -2007,7 +2008,7 @@ public class GenericObjectPool<T> extend
>     /**
>      * All of the objects currently associated with this pool in any state. It
>      * excludes objects that have been destroyed. The size of
> -     * {@link #_allObjects} will always be less than or equal to {@linl
> +     * {@link #_allObjects} will always be less than or equal to {@link
>      * #_maxActive}.
>      */
>     private Map<T, PooledObject<T>> _allObjects = null;
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message