This content has been marked as final.
Show 4 replies
-
1. Re: Review of fix for JBWS-2304
ropalka Sep 27, 2008 3:40 PM (in response to dlofthouse)"darran.lofthouse@jboss.com" wrote:
Overall I think the mechanism to cache the Document and DocumentBuilder should be re-factored to possibly associate it with the MessageContext and the message itself so that it does not need to be in it's own ThreadLocal with a potential for leaks - however I am not sure if such a bug change would be suitable at the moment.
Anyway I just wanted to see if you have any additional thoughts on this fix before I commit.
Hi Darran,
to be honest I don't like our current DOMUtils ThreadLocals capability. ThreadLocals holding thread-specific fields should only be used when there is no other good option available. The following are top two reasons to avoid ThreadLocals:
The use of thread-specific variables tends to hide parameters that influence behaviour and can make it harder to check for errors or leakage. In this sense, thread-specific variables present the same, although less extreme, traceability problems as static global variables.
Use of thread-specific variables can detract from reusability by increasing code dependencies.
In other words if you will refactor our DOMUtils and will remove ThreadLocals dependent code, I will really like your solution ;)
Richard -
2. Re: Review of fix for JBWS-2304
dlofthouse Sep 29, 2008 5:45 AM (in response to dlofthouse)Thanks Richard,
Would it be Ok to go ahead with my initial fix to begin with and then I will work on the a complete solution including refactoring next - one issue that will need to resolved is that because DOMUtils is in common it does have some use with the other three stacks. -
3. Re: Review of fix for JBWS-2304
ropalka Sep 29, 2008 5:49 AM (in response to dlofthouse)"darran.lofthouse@jboss.com" wrote:
Would it be Ok to go ahead with my initial fix to begin with and then I will work on the a complete solution including refactoring next
Yes, no problem."darran.lofthouse@jboss.com" wrote:
- one issue that will need to resolved is that because DOMUtils is in common it does have some use with the other three stacks.
AFAICS DOMUtils ThreadLocal capability is used in native stack only ;) -
4. Re: Review of fix for JBWS-2304
dlofthouse Sep 29, 2008 1:09 PM (in response to dlofthouse)"richard.opalka@jboss.com" wrote:
AFAICS DOMUtils ThreadLocal capability is used in native stack only ;)
I will double check but I think I found one way it does get called when used by Metro to read a deployment descriptor - again leaving the Document and DocumentBuilder associated with the Thread.