----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17228/#review33408 -----------------------------------------------------------
Ship it! Committed 1563452. Please close the review. - Ryan Baxter On Jan. 28, 2014, 2:10 a.m., ChunYan Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17228/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2014, 2:10 a.m.) > > > Review request for shindig, Dan Dumont, Ryan Baxter, Rich Thompson, Stanton > Sievers, and Zhi Hong Yang. > > > Bugs: SHINDIG-1893 > https://issues.apache.org/jira/browse/SHINDIG-1893 > > > Repository: shindig > > > Description > ------- > > During our gadget development, several gadgets are referring to the same > gadget template. The rendering of those gadgets is problematic sometimes. > > When the problem occur, the Java stack trace look like this: > > Stack Dump = java.lang.NullPointerException > at org.apache.xerces.dom.ParentNode.nodeListItem(Unknown Source) > at org.apache.xerces.dom.ParentNode.item(Unknown Source) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processChildNodes(DefaultTemplateProcessor.java:134) > at > org.apache.shindig.gadgets.templates.tags.TemplateBasedTagHandler.processTemplate(TemplateBasedTagHandler.java:68) > at > org.apache.shindig.gadgets.templates.tags.TemplateBasedTagHandler.process(TemplateBasedTagHandler.java:58) > at > org.apache.shindig.gadgets.templates.XmlTemplateLibrary$LibraryTagHandler.process(XmlTemplateLibrary.java:305) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processElementInner(DefaultTemplateProcessor.java:341) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processElement(DefaultTemplateProcessor.java:246) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processNode(DefaultTemplateProcessor.java:154) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processChildNodes(DefaultTemplateProcessor.java:134) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processTemplate(DefaultTemplateProcessor.java:126) > at > org.apache.shindig.gadgets.rewrite.TemplateRewriter.executeTemplates(TemplateRewriter.java:357) > at > org.apache.shindig.gadgets.rewrite.TemplateRewriter.rewriteImpl(TemplateRewriter.java:177) > at > org.apache.shindig.gadgets.rewrite.TemplateRewriter.rewrite(TemplateRewriter.java:129) > at > org.apache.shindig.gadgets.render.HtmlRenderer.render(HtmlRenderer.java:88) > at org.apache.shindig.gadgets.render.Renderer.render(Renderer.java:101) > at > org.apache.shindig.gadgets.servlet.GadgetRenderingServlet.render(GadgetRenderingServlet.java:112) > at > org.apache.shindig.gadgets.servlet.GadgetRenderingServlet.doGet(GadgetRenderingServlet.java:90) > at javax.servlet.http.HttpServlet.service(HttpServlet.java:575) > > Stack Dump2 = java.lang.NullPointerException > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processText(DefaultTemplateProcessor.java:174) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processNode(DefaultTemplateProcessor.java:151) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processChildNodes(DefaultTemplateProcessor.java:134) > at > org.apache.shindig.gadgets.templates.tags.TemplateBasedTagHandler.processTemplate(TemplateBasedTagHandler.java:68) > at > org.apache.shindig.gadgets.templates.tags.TemplateBasedTagHandler.process(TemplateBasedTagHandler.java:58) > at > org.apache.shindig.gadgets.templates.XmlTemplateLibrary$LibraryTagHandler.process(XmlTemplateLibrary.java:305) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processElementInner(DefaultTemplateProcess" > severity="50" version="1.0.1"> > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processElement(DefaultTemplateProcessor.java:247) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processNode(DefaultTemplateProcessor.java:154) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processChildNodes(DefaultTemplateProcessor.java:134) > at > org.apache.shindig.gadgets.templates.DefaultTemplateProcessor.processTemplate(DefaultTemplateProcessor.java:126) > at > org.apache.shindig.gadgets.rewrite.TemplateRewriter.executeTemplates(TemplateRewriter.java:358) > at > org.apache.shindig.gadgets.rewrite.TemplateRewriter.rewriteImpl(TemplateRewriter.java:178) > at > org.apache.shindig.gadgets.rewrite.TemplateRewriter.rewrite(TemplateRewriter.java:130) > at > org.apache.shindig.gadgets.render.HtmlRenderer.render(HtmlRenderer.java:88) > at org.apache.shindig.gadgets.render.Renderer.render(Renderer.java:101) > at > org.apache.shindig.gadgets.servlet.GadgetRenderingServlet.render(GadgetRenderingServlet.java:107) > at > org.apache.shindig.gadgets.servlet.GadgetRenderingServlet.doGet(GadgetRenderingServlet.java:85) > at javax.servlet.http.HttpServlet.service(HttpServlet.java:693) > at javax.servlet.http.HttpServlet.service(HttpServlet.java:806) > > The Xerces implementation is not thread safe and when we are caching the > template we can potentially have multiple threads manipulating the same > Element object. This results in random null pointer exceptions as well as > other odd exceptions in the code. The solution is to add synchronized to the > key XML element. > > > Diffs > ----- > > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/DefaultTemplateProcessor.java > 1560262 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateLibraryFactory.java > 1560262 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/XmlTemplateLibrary.java > 1560262 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/FlashTagHandler.java > 1560262 > > /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/tags/TemplateBasedTagHandler.java > 1560262 > > Diff: https://reviews.apache.org/r/17228/diff/ > > > Testing > ------- > > > Thanks, > > ChunYan Zhang > >
