Yeah, I was looking at the same thing while I was adding this bit of code in. I was trying to decide what else, if anything I should do with it.
The factories map is created initially by the AbderaConfiguration and passed into the Factory implementation on init. Each FOMFactory instance has only a single ExtensionFactoryMap instance. Assuming that most users will end up using the default Factory implementation, I wouldn't see a problem in creating a copy for each Factory instance. - James Garrett Rooney wrote: > On 4/25/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: >> Author: jmsnell >> Date: Wed Apr 25 07:46:17 2007 >> New Revision: 532373 >> >> URL: http://svn.apache.org/viewvc?view=rev&rev=532373 >> Log: >> Second attempt at improving thread safety on ExtensionFactoryMap. >> Wrap the weakhashmap with a synchronized map and synchronize the >> iterations over the factories list > > Other potential issues in this code WRT thread safety: > > We don't make a defensive copy of the factories list when we're > created, so potentially someone else can synchronize on it, resulting > in deadlock. Alternatively it could be modified without > synchronization after we're created, defeating the purpose of the > other locking we do. Similarly, we return a reference to it in > getFactories(), with identical potential badness. > > The first case can be dealt with via documentation if we don't want to > make a copy, but the second really seems like it should be dealt with > by making a copy of the list before returning it. > > -garrett >
