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: r1213845 - in /commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl: GenericKeyedObjectPoolFactory.java GenericObjectPoolFactory.java
Date Wed, 14 Dec 2011 08:25:09 GMT
On 13 December 2011 19:59, sebb <sebbaz@gmail.com> wrote:
> On 13 December 2011 19:32, Mark Thomas <markt@apache.org> wrote:
>> On 13/12/2011 18:40, sebb@apache.org wrote:
>>> Author: sebb
>>> Date: Tue Dec 13 18:40:48 2011
>>> New Revision: 1213845
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1213845&view=rev
>>> Log:
>>> Make factories thread-safe.
>>> [No existing tests actually used the setter methods which have now been removed]
>>
>> This commit removes functionality and doesn't achieve the stated
>> objective since the config object is accessible and not thread-safe.
>
> Oops - the config object is cloned prior to being stored by the ctor
> (and was cloned by the setter) but is not cloned by the getter.
> My bad; did not notice that.
>
> That can of course be fixed, either by cloning in the get or by
> removing the getter.
>
> Even if the setter is restored, I think the getter needs to clone the
> config to ensure thread-safety.
>
> Is it useful to be able to change the factory after creation?
> And are there any configuration items that it does not make sense to change?
>
> If so, we need unit tests.
>
>> I am close to vetoing this commit.
>
> Fine, but the factory classes still need to be fixed.

I've fixed the getters.

Just done a check of DBCP 1.4, and that calls
GenericKeyedObjectPoolFactory (does not call GenericObjectPoolFactory)
The main code creates the factory but then uses it through the
KeyedObjectPoolFactory interface, so is clearly not interested in
setters/getters.

I will start a separate thread about factory thread safety.

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

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


Mime
View raw message