On Mar 13, 2007, at 6:46 PM, Dennis Byrne wrote:
David has found a few interesting things about the FactoryFinder
implementation. I think we can all agree that the memory leak can
be fixed, I have mixed feelings about the use of synchronization
here. The code is invoked from many places in the app and I am
mostly concerned about performance.
If FactoryFinder.setFactory() is only invoked from the context
startup listener, or the servlet's init method, do we need
synchronization?
I think so. I think that hashMap.get(key) can return screwy results
if its concurrent with hashmap.put(otherkey, value). Using a
ConcurrentHashMap would at least give answers that made sense, but
the guarantees on not changing factory implementation halfway through
would probably be weaker.
I'm sure that a totally unsynchronized implementation would pass the
tck, so the real question is how rigorously you want to enforce the
(probably poorly stated) requirements. It might be worth trying to
measure if "total synchronization" is in fact a bottleneck.
Personally I think this whole FactoryFinder thing is somewhat a bad
idea and that obtaining the factories once when the app starts and
injecting them into any object that could possibly want to use them
would avoid your performance concerns and clarify the structure of
the framework. You may have noticed my opinion of discovery on
another thread :-)
thanks
david jencks
Dennis Byrne
On 3/13/07, David Jencks (JIRA) <dev@myfaces.apache.org> wrote:
[ https://issues.apache.org/jira/browse/MYFACES-1558?
page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Jencks updated MYFACES-1558:
----------------------------------
Attachment: MYFACES-1558-jsf11.patch
Here's a patch for the jsf11 branch. I believe its the same as the
patch for jsf 1.2 branch with generics removed, but I spent less
time on it than the first one. The entire jsf11 project builds
fine with this.
> FactoryFinder not thread safe, and has a memory leak
> ----------------------------------------------------
>
> Key: MYFACES-1558
> URL: https://issues.apache.org/jira/browse/
MYFACES-1558
> Project: MyFaces Core
> Issue Type: Bug
> Components: General
> Affects Versions: 1.2.0-SNAPSHOT
> Reporter: David Jencks
> Assigned To: Dennis Byrne
> Attachments: MYFACES-1558-jsf11.patch, MYFACES-1558.patch
>
>
> FactoryFinder has insufficient synchronization to be thread safe,
and has a memory leak in _registeredFactoryNames.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
--
Dennis Byrne
