Re: Exceptions due to freemarker's use of non-thread-safe DOM library

Monday, July 18, 2011, 3:54:51 PM, Newman, John W wrote:

> I've got to push back on you guys here.  There aren't _any thread
> safe DOM implementations in java out there, so essentially your
> entire .dom package that potentially is very useful is really
> useless and flat out dangerous.

The strange thing is, if thread-safe DOM isn't important for the
industry (and so it seems, if none of the implementations are
thread-safe), why is it suddenly so important when the DOM is consumed
by FreeMarker? I mean, the DOM is always consumed by something... do
any other consumers solve this somehow? Like XSLT implementations?
Other than synchronizing on the DOM root of course (that you can do in
FM as well of course).

> Freemarker needs to either:
>
> A) Delete the entire dom package as it's impossible for anyone to
> use it with any java implementations without getting corrupt
> documents (BAD)  (unless their application has only one thread, but come on)

I have used it quite much, however that was always in single-threaded
environment (like in FMPP). What others use the DOM wrapper for, I
don't know... funny that this issue was never brought up.

> B) Add the appropriate locks to your code that is using the dom
> library in an unsafe manner.  If xerces were the only
> non-thread-safe implementation, you'd be right.   But EVERY
> implementation that exists out there is not thread safe by design.

Note that the promise that FM will make them thread-safe is implied at
best. At least I hope the documentation doesn't say such a thing. It's
just like with any non-thread safe POJO... putting them into the
data-model doesn't make them more or less thread safe. But of course,
this case really need warnings everywhere.

> C) Provide your own implementation of org.w3c.dom that is somehow
> thread safe to be used from the NodeModel classes [good luck]
> " The correct way to fix this, assuming you want to keep using this
> particular DOM implementation, is to create your own proxy classes
> that implement the DOM interfaces, wrap and delegate to the
> underlying DOM objects, synchronizing the operations on them. You
> can use CGLIB to create these proxies really quickly. Alternatively,
> you can come up with a wrapping/delegating+synchronizing
> freemarker.ext.xml.NodeModel implementation too."
>
> I've done this and it doesn't work.  Wrapping the element to sync
> on it is all well and good, but I can't sync on
> element.child.child.child.   My template is having your library call
> element.child.child.child.nodeList()  which is not thread safe. 
> Your call to nodeList() is where the sync block is supposed to be.

I think Attila meant it so that you also use proxies for the return
values.

> "I don't think we'd need to introduce synchronization in our
> library to accommodate an implementation artifact of a particular
> data model in a completely unrelated library."
>
> I don't disagree, really this is a very unfortunate situation.  But
> actually I think you do need to introduce syncs, since there really
> is no alternative.    If you can find me any way to safely use this
> package as it sits today from multiple threads I'd love to hear it. 
> Every implementation of DOM is not thread safe, so what do we do here?

I think you need to step back and start out from the fact that DOM-s
are not thread-safe. It happens very often with other kind of objects
as well, right? And if trivial sync-ing would limit throughput too
much, what we always do is:

(a) Don't reuse them, recreate them every time they are needed

(b) If (a) is too slow, then pool them

> Thanks
> John
> -----Original Message-----
> From: Daniel Dekany [mailto:[email protected]] 
> Sent: Friday, June 03, 2011 1:55 PM
> To: Newman, John W
> Cc: FreeMarker-user ([email protected])
> Subject: Re: [FreeMarker-user] Exceptions due to freemarker's use of non-thread-safe DOM library
>
> FreeMarker data-models are in general not shareable between threads.
> After all, they may contain POJO-s which aren't thread-safe (like
> Xerces DOM trees, it seems), so it's hopeless to guarantee
> thread-safety in FreeMarker. Since sharing data-models among threads
> wasn't a design goal, TemplateModel-s (the things that wrap the
> POJO-s) aren't thread-safe either (unless by pure accident). Is
> there some part in the documentation that made you believe otherwise?
>
> Still, a data-model can be thread-safe, but then it has to be built
> carefully. Like, it seems, you would need to write a
> SyncronizedNodeModel class or like...
>
> --
> Best regards,
>  Daniel Dekany
>
>
> Friday, June 3, 2011, 6:04:46 PM, Newman, John W wrote:
>
>> Hello,
>>  
>> In production under load, occasionally we'll get this stack trace
>> from a template (using freemarker servlet).
>>  
>> java.lang.NullPointerException: 
>> (no message) 
>> at org.apache.xerces.dom.ParentNode.nodeListItem(Unknown Source) 
>> at org.apache.xerces.dom.ParentNode.item(Unknown Source) 
>> at freemarker.ext.dom.NodeListModel.<init>(NodeListModel.java:89) 
>> at freemarker.ext.dom.NodeModel.getChildNodes(NodeModel.java:302) 
>> at freemarker.ext.dom.ElementModel.get(ElementModel.java:124) 
>> at freemarker.core.Dot._getAsTemplateModel(Dot.java:76) 
>> at freemarker.core.Expression.getAsTemplateModel(Expression.java:89) 
>> at freemarker.core.IteratorBlock.accept(IteratorBlock.java:94) 
>> at freemarker.core.Environment.visit(Environment.java:209) 
>> [ a few dozen stack frames visit/accept/macro/ifblock from template itself ]
>> at freemarker.core.Environment.visit(Environment.java:209) 
>> at freemarker.core.Environment.process(Environment.java:189) 
>> at freemarker.template.Template.process(Template.java:237) 
>> at
>> freemarker.ext.servlet.FreemarkerServlet.process(FreemarkerServlet.java:458)
>> at
>> freemarker.ext.servlet.FreemarkerServlet.doGet(FreemarkerServlet.java:397)
>> at javax.servlet.http.HttpServlet.service(HttpServlet.java:690) 
>> at javax.servlet.http.HttpServlet.service(HttpServlet.java:803)  
>>  
>>  
>> Our application uses XML elements that are sometimes cloned from
>> another element.  The freemarker templates do a lot of various xml
>> operations, ${element.@Attribute} , ${element[xpath]}, etc.  things
>> like that that normally works perfectly fine.  In investigating the
>> stack trace, it really looks like 
>> https://issues.apache.org/jira/browse/XERCESJ-727
>>  
>> Please read through that, essentially the org.apache.xerces.dom
>> library is deliberately not thread safe by design.  [This is news to
>> me, a bit unexpected...]  Apparently these element implementations
>> maintain a local cache of what has already been parsed through, and
>> if a second thread jumps in it will end up stepping on this shared
>> cache causing undefined behavior.  So the burden falls on the caller
>> to synchronize on the elements before doing certain operations. (I'm not sure exactly which).
>>  
>> I've looked through the freemarker xml package a bit, and I didn't
>> find any sync blocks.  I tried isolating the problem in there and
>> getting a good repeatable test case, but unfortunately I don't
>> understand your library well enough to do so.  The best test case I
>> can provide you is attached - it's basically an adaptation of what
>> was posted in that issue.  I think the freemarker library needs to
>> add a few sync blocks here and there around these xml operations,
>> but unfortunately I can't say exactly where, possibly quite a few
>> places.  And hopefully it's not one big wide block on the outer
>> method that would kill performance.  Please look at this and let me
>> know what you think.  I was hoping to just get a patch to send, but
>> after several hours I'm not having much luck.  A simple pojo test case is attached.
>>  
>> Thanks,
>> John
>>  
>>  
>>  
>> John W. Newman
>> Programmer
>>  
>> 5750 Centre Avenue, Suite 500
>> Pittsburgh, PA 15206
>> Tel  412-204-0116
>> [email protected]
>> www.d3onc.com
>> Fax 412-365-0749
>>  
>> This e-mail may contain confidential information of the sending
>> organization. Any unauthorized or improper disclosure, copying,
>> distribution, or use of the contents of this e-mail and attached
>> document(s) is prohibited. The information contained in this e-mail
>> and attached document(s) is intended only for the personal and
>> confidential use of the recipient(s) named above. If you have
>> received this communication in error, please notify the sender
>> immediately by e-mail and delete the original e-mail and attached document(s).

-- 
Best regards,
 Daniel Dekany


------------------------------------------------------------------------------
Storage Efficiency Calculator
This modeling tool is based on patent-pending intellectual property that
has been used successfully in hundreds of IBM storage optimization engage-
ments, worldwide.  Store less, Store more with what you own, Move data to 
the right place. Try It Now! http://www.accelacomm.com/jaw/sfnl/114/51427378/