On 8/22/06, James M Snell <[EMAIL PROTECTED]> wrote:
As Garrett has pointed out, the current approach to handling the Abdera
configuration model has a number of important thread safety and class
loader issues that had largely been ignored in the initial drop of the
code. After talking with Garrett a bit, we both feel that now might be
the best time to address those issues, before we get much further down
the road.
There are several important challenges with the current code:
1. The use of static singletons for the default Parser, Factory and
XPath instances is not thread safe and can cause problems when Abdera is
deployed in managed environments like WebSphere, Tomcat, etc.
2. The current ConfigProperties and ServiceUtil implementations are not
thread safe and don't properly handle class loaders. For instance, if
the Abdera jars are included in tomcat's classpath instead of a webapps
classpath, the webapp cannot override the default config properties.
3. Using ThreadLocal (as I originally attempted to do) actually won't
work. I remembered after checking that code in a bit earlier today that
there were some significant problems with using ThreadLocal in managed
environments and thread pools. Specifically, the thread local storage
might not be cleared out when the thread is returned to the pool. There
are right ways of using ThreadLocal and the way I was intending to use
it isn't it.
There are other problems but that's enough to get started :-)
What I propose is a significant modification to the config mechanism.
Rather than acquiring instances of the Parser, Factory, etc using the
xxxx.INSTANCE pattern (e.g. Parser.INSTANCE, Factory.INSTANCE, etc) we
would create a top level instance object that would completely avoid
using statics for configuration properties.
Abdera abdera = new Abdera();
Parser parser = abdera.getParser();
Factory factory = abdera.getFactory();
For what it's worth, there are a number of use cases that just need to
grab a Parser, or just need to grab a Factory, so it wouldn't be
outside the realm of possibility to provide something like:
Parser p = Abdera.getParser();
This would, of course, do the equivalent of new Abders().getParser()
under the hood, but would be nicer looking for people who don't care
about the overhead of looking up what underlying implementation to do
etc.
This would be a significant change that would touch a lot of the current
code and tests.
If there isn't another sane work around for the various problems, I'd
say go for it, make the huge sweeping change now before people get
attached to the current way of doing things.
I never really liked the .INSTANCE stuff anyway ;-)
=garrett
