well the instanceManager.destoy should be called whatever happens I think
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber <http://www.tomitribe.com> | JavaEE Factory <https://javaeefactory-rmannibucau.rhcloud.com> 2016-07-13 20:51 GMT+02:00 Violeta Georgieva <miles...@gmail.com>: > Hi, > > 2016-07-12 23:53 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>: > > > > Hi all, > > > > is (jsp) generated code for tags suffering from the same kind of issue? > For > > a c:out I get: > > > > > > private boolean > > _jspx_meth_c_005fout_005f0(javax.servlet.jsp.PageContext > > _jspx_page_context) > > throws java.lang.Throwable { > > javax.servlet.jsp.PageContext pageContext = _jspx_page_context; > > javax.servlet.jsp.JspWriter out = _jspx_page_context.getOut(); > > // c:out > > org.apache.taglibs.standard.tag.rt.core.OutTag > > _jspx_th_c_005fout_005f0 = > > (org.apache.taglibs.standard.tag.rt.core.OutTag) > > > > _005fjspx_005ftagPool_005fc_005fout_0026_005fvalue_005fnobody.get(org.apache.taglibs.standard.tag.rt.core.OutTag.class); > > _jspx_th_c_005fout_005f0.setPageContext(_jspx_page_context); > > _jspx_th_c_005fout_005f0.setParent(null); > > // /fail.jsp(30,0) name = value type = null reqTime = true required > > = true fragment = false deferredValue = false expectedTypeName = null > > deferredMethod = false methodSignature = null > > _jspx_th_c_005fout_005f0.setValue((java.lang.Object) > > org.apache.jasper.runtime.PageContextImpl.proprietaryEvaluate("${'<tag> > > , &'}", java.lang.Object.class, > > (javax.servlet.jsp.PageContext)_jspx_page_context, null)); > > int _jspx_eval_c_005fout_005f0 = _jspx_th_c_005fout_005f0.doStartTag(); > > if (_jspx_th_c_005fout_005f0.doEndTag() == > > javax.servlet.jsp.tagext.Tag.SKIP_PAGE) { > > > > _005fjspx_005ftagPool_005fc_005fout_0026_005fvalue_005fnobody.reuse(_jspx_th_c_005fout_005f0); > > return true; > > } > > > > _005fjspx_005ftagPool_005fc_005fout_0026_005fvalue_005fnobody.reuse(_jspx_th_c_005fout_005f0); > > return false; > > } > > > > Wonder if the reuse() shouldn't be in a finally block - in particular > > for custom tags. Wdyt? > > > > > > Currently the implementation [1], [2] is based on the fact whether or not > the tag handler implements TryCatchFinally [3] > > May be we should revise it and to put these (reuse, release, > destroyInstance) always in try/finally > > Regards, > Violeta > > [1] > > https://github.com/apache/tomcat/blob/trunk/java/org/apache/jasper/compiler/Generator.java#L2562 > [2] > > https://github.com/apache/tomcat/blob/trunk/java/org/apache/jasper/compiler/Generator.java#L2585 > [3] > > http://docs.oracle.com/javaee/6/api/javax/servlet/jsp/tagext/TryCatchFinally.html > > > > > > > > > Romain Manni-Bucau > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog > > <http://rmannibucau.wordpress.com> | Github < > https://github.com/rmannibucau> | > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber > > <http://www.tomitribe.com> | JavaEE Factory > > <https://javaeefactory-rmannibucau.rhcloud.com> > > > > 2016-07-01 19:25 GMT+02:00 Mark Thomas <ma...@apache.org>: > > > > > On 01/07/2016 17:28, Romain Manni-Bucau wrote: > > > > Think org.apache.jasper.runtime.TagHandlerPool#release can be > affected > > > too > > > > - I'm not sure where it is called in the codebase but the pattern is > the > > > > same. > > > > > > I've doing a review of all calls to destroyInstance() now. There is at > > > least one further issue as well as the tag one you spotted. > > > > > > Mark > > > > > > > > > > > > > > > Romain Manni-Bucau > > > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > > > <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog > > > > <http://rmannibucau.wordpress.com> | Github < > > > https://github.com/rmannibucau> | > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber > > > > <http://www.tomitribe.com> | JavaEE Factory > > > > <https://javaeefactory-rmannibucau.rhcloud.com> > > > > > > > > 2016-07-01 18:10 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com > >: > > > > > > > >> +1 > > > >> > > > >> > > > >> Romain Manni-Bucau > > > >> @rmannibucau <https://twitter.com/rmannibucau> | Blog > > > >> <https://blog-rmannibucau.rhcloud.com> | Old Wordpress Blog > > > >> <http://rmannibucau.wordpress.com> | Github > > > >> <https://github.com/rmannibucau> | LinkedIn > > > >> <https://www.linkedin.com/in/rmannibucau> | Tomitriber > > > >> <http://www.tomitribe.com> | JavaEE Factory > > > >> <https://javaeefactory-rmannibucau.rhcloud.com> > > > >> > > > >> 2016-07-01 18:06 GMT+02:00 Mark Thomas <ma...@apache.org>: > > > >> > > > >>> On 01/07/2016 16:41, Romain Manni-Bucau wrote: > > > >>>> Hello guys, > > > >>>> > > > >>>> if a jspDestroy() throws an Exception then the instance manager is > not > > > >>>> called which can lead to some leaks depending the implementation > (see > > > >>>> org.apache.jasper.servlet.JspServletWrapper#destroy) > > > >>>> > > > >>>> Would it be possible to have a way to release/cleanup the jsp > context > > > >>>> through the instance manager or to call destroyInstance properly? > > > >>> > > > >>> I think wrapping the call to destroy in a try/catch would be the > way to > > > >>> go. It would be worth a check to see if there are any other places > > > where > > > >>> similar issues could occur with JSPs. > > > >>> > > > >>> Mark > > > >>> > > > >>> > > > >>> > --------------------------------------------------------------------- > > > >>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > > > >>> For additional commands, e-mail: dev-h...@tomcat.apache.org > > > >>> > > > >>> > > > >> > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > > > For additional commands, e-mail: dev-h...@tomcat.apache.org > > > > > > >
