abdera-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James M Snell <jasn...@gmail.com>
Subject Re: svn commit: r532373 - /incubator/abdera/java/trunk/core/src/main/java/org/apache/abdera/factory/ExtensionFactoryMap.java
Date Wed, 25 Apr 2007 15:45:35 GMT
Wait, I take that back... each Factory impl is already getting it's own
copy of the Factories list.  So long as we serialize access to it, I
don't think it's going to be much of a problem.

- James

James M Snell wrote:
> 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, jmsnell@apache.org <jmsnell@apache.org> 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
>>
> 

Mime
View raw message