On 6/16/11 9:32 AM, sebb wrote:
> On 16 June 2011 17:25, Phil Steitz <phil.ste...@gmail.com> wrote:
>> Yesterday I fixed some [dbcp] "problems" caused by the new [pool]
>> requirement that setFactory can only be called once.  The quotes are
>> because most of the problems were redundant calls to setFactory.
>> The reason that we left setFactory in [pool] is that [dbcp]'s
>> connection factory constructors call setFactory on the pool passed
>> to them.  It is an easy "mistake" to create a pool, then create a
>> connection factory and then do pool.setFactory(factory).  I
>> eliminated all of these usages from within [dbcp], but I bet a fair
>> amount of user code will similarly blow up when people upgrade.
> AIUI, the upgrade does require code to be changed because of the package 
> rename?

No question the upgrade is going to require changes.  I just want to
make those changes as transparent as possible and if possible avoid
gotchas like what happened above.
> If so, can we not drop the setFactory method and provide it as a
> constructor parameter?

We have already decided to keep the setFactory method, basically
because the [dbcp] use case above would be very hard to meet without
it, but we now use double-checked locking and do not allow it to be
reset, once it is set initially.  My proposal below is to be more
forgiving if an invocation when it is already set passes a
reference-equal factory as actual parameter.

Phil
> That would produce a compile-time error, rather than failing at run-time.
>
>>  I hate to keep backsliding here, but maybe we should to this in GOP
>> setFactory:
>>
>>             synchronized (factoryLock) {
>>                 if (this.factory == null) {
>>                     this.factory = factory;
>> -                } else {
>> +                } else if (this.factory != factory) {
>>                     throw new IllegalStateException("Factory
>> already set");
>>                 }
>>             }
>>
>> Phil
>>
>>
>> ---------------------------------------------------------------------
>> 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
>
>


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

Reply via email to