On 13 December 2011 19:32, Mark Thomas <ma...@apache.org> wrote: > On 13/12/2011 18:40, s...@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. > Mark > > --------------------------------------------------------------------- > 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
