On 13 December 2011 19:59, sebb <[email protected]> wrote: > On 13 December 2011 19:32, Mark Thomas <[email protected]> wrote: >> On 13/12/2011 18:40, [email protected] 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: [email protected] >> For additional commands, e-mail: [email protected] >> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
