ASF Bugzilla – Attachment 26179 Details for
Bug 33934
[standard] memory leak in jstl c:set tag
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Fix for memory leak in c:set
jstl33934.patch (text/plain), 23.56 KB, created by
Jeremy Boynes
on 2010-10-16 13:47:22 UTC
(
hide
)
Description:
Fix for memory leak in c:set
Filename:
MIME Type:
Creator:
Jeremy Boynes
Created:
2010-10-16 13:47:22 UTC
Size:
23.56 KB
patch
obsolete
>Index: src/test/java/org/apache/taglibs/standard/tag/common/core/SetSupportTest.java >=================================================================== >--- src/test/java/org/apache/taglibs/standard/tag/common/core/SetSupportTest.java (revision 1023287) >+++ src/test/java/org/apache/taglibs/standard/tag/common/core/SetSupportTest.java (working copy) >@@ -64,8 +64,6 @@ > > bean = new Bean(); > >- tag = new SetSupport(); >- tag.setPageContext(pageContext); > > ExpressionFactory expressionFactory = createMock(ExpressionFactory.class); > JspApplicationContext applicationContext = createMock(JspApplicationContext.class); >@@ -85,9 +83,9 @@ > > @Test > public void testSyntax1WithNoScope() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); >- tag.valueSpecified = true; >- tag.value = VALUE; > > // verify mapper is checked but that no action is taken > expect(vm.resolveVariable(VAR)).andReturn(null); >@@ -99,10 +97,10 @@ > > @Test > public void testSyntax1WithNullScope() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); > tag.setScope(null); >- tag.valueSpecified = true; >- tag.value = VALUE; > > // verify mapper is checked but that no action is taken > expect(vm.resolveVariable(VAR)).andReturn(null); >@@ -114,10 +112,10 @@ > > @Test > public void testSyntax1WithPageScope() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); > tag.setScope("page"); >- tag.valueSpecified = true; >- tag.value = VALUE; > > // verify mapper is checked but that no action is taken > expect(vm.resolveVariable(VAR)).andReturn(null); >@@ -129,10 +127,10 @@ > > @Test > public void testSyntax1WithNonPageScope() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); > tag.setScope("request"); >- tag.valueSpecified = true; >- tag.value = VALUE; > > // verify mapper is not checked > pageContext.setAttribute(VAR, VALUE, PageContext.REQUEST_SCOPE); >@@ -143,9 +141,9 @@ > > @Test > public void testSyntax1WithNullValueAndNoScope() throws JspException { >+ tag = new MockSetSupport(null); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); >- tag.valueSpecified = true; >- tag.value = null; > > // verify mapper is checked but that no action is taken > expect(vm.resolveVariable(VAR)).andReturn(null); >@@ -157,10 +155,10 @@ > > @Test > public void testSyntax1WithNullValueAndNonPageScope() throws JspException { >+ tag = new MockSetSupport(null); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); > tag.setScope("request"); >- tag.valueSpecified = true; >- tag.value = null; > > // verify mapper is checked but that no action is taken > expect(vm.resolveVariable(VAR)).andReturn(null); >@@ -174,11 +172,9 @@ > public void testSyntax3WithMap() throws JspException { > @SuppressWarnings("unchecked") > Map<String, Object> target = createMock(Map.class); >- tag.target = target; >- tag.property = PROPERTY; >- tag.valueSpecified = true; >- tag.value = VALUE; >- >+ tag = new MockSetSupport(VALUE, target, PROPERTY); >+ tag.setPageContext(pageContext); >+ > expect(target.put(PROPERTY, VALUE)).andStubReturn(null); > replay(target); > tag.doEndTag(); >@@ -189,10 +185,8 @@ > public void testSyntax3WithMapWhenPropertyIsNull() throws JspException { > @SuppressWarnings("unchecked") > Map<String, Object> target = createMock(Map.class); >- tag.target = target; >- tag.property = null; >- tag.valueSpecified = true; >- tag.value = VALUE; >+ tag = new MockSetSupport(VALUE, target, null); >+ tag.setPageContext(pageContext); > > expect(target.put(null, VALUE)).andStubReturn(null); > replay(target); >@@ -204,10 +198,8 @@ > public void testSyntax3WithMapWhenValueIsNull() throws JspException { > @SuppressWarnings("unchecked") > Map<String, Object> target = createMock(Map.class); >- tag.target = target; >- tag.property = PROPERTY; >- tag.valueSpecified = true; >- tag.value = null; >+ tag = new MockSetSupport(null, target, PROPERTY); >+ tag.setPageContext(pageContext); > > expect(target.remove(PROPERTY)).andStubReturn(null); > replay(target); >@@ -217,10 +209,8 @@ > > @Test > public void testSyntax3WithBean() throws JspException { >- tag.target = bean; >- tag.property = PROPERTY; >- tag.valueSpecified = true; >- tag.value = VALUE; >+ tag = new MockSetSupport(VALUE, bean, PROPERTY); >+ tag.setPageContext(pageContext); > > tag.doEndTag(); > Assert.assertEquals(VALUE, bean.getProperty()); >@@ -228,10 +218,8 @@ > > @Test > public void testSyntax3WithBeanAndNullValue() throws JspException { >- tag.target = bean; >- tag.property = PROPERTY; >- tag.valueSpecified = true; >- tag.value = null; >+ tag = new MockSetSupport(null, bean, PROPERTY); >+ tag.setPageContext(pageContext); > > tag.doEndTag(); > Assert.assertNull(bean.getProperty()); >@@ -239,10 +227,8 @@ > > @Test > public void testSyntax3WithBeanAndUndefinedProperty() throws JspException { >- tag.target = bean; >- tag.property = "undefined"; >- tag.valueSpecified = true; >- tag.value = VALUE; >+ tag = new MockSetSupport(VALUE, bean, "undefined"); >+ tag.setPageContext(pageContext); > > try { > tag.doEndTag(); >@@ -253,10 +239,8 @@ > > @Test > public void testSyntax3WithBeanAndReadOnlyProperty() throws JspException { >- tag.target = bean; >- tag.property = "readOnly"; >- tag.valueSpecified = true; >- tag.value = VALUE; >+ tag = new MockSetSupport(VALUE, bean, "readOnly"); >+ tag.setPageContext(pageContext); > > try { > tag.doEndTag(); >@@ -267,10 +251,8 @@ > > @Test > public void testSyntax3WhenTargetIsNull() throws JspException { >- tag.target = null; >- tag.property = PROPERTY; >- tag.valueSpecified = true; >- tag.value = VALUE; >+ tag = new MockSetSupport(VALUE, null, PROPERTY); >+ tag.setPageContext(pageContext); > > try { > tag.doEndTag(); >@@ -287,9 +269,9 @@ > */ > @Test > public void test49526WhenNotMapped() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); >- tag.valueSpecified = true; >- tag.value = VALUE; > > // verify mapper is checked but that no action is taken > expect(vm.resolveVariable(VAR)).andReturn(null); >@@ -306,9 +288,9 @@ > */ > @Test > public void test49526WhenAlreadyMapped() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); >- tag.valueSpecified = true; >- tag.value = VALUE; > > // verify mapper is checked and the mapped variable removed > ValueExpression ve = createMock(ValueExpression.class); >@@ -327,9 +309,9 @@ > */ > @Test > public void test49526WhenNotUsingPageContext() throws JspException { >+ tag = new MockSetSupport(VALUE); >+ tag.setPageContext(pageContext); > tag.setVar(VAR); >- tag.valueSpecified = true; >- tag.value = VALUE; > tag.setScope("request"); > > // verify mapper is not checked >@@ -341,21 +323,19 @@ > > @Test > public void testResultFromValueAttribute() { >- tag.valueSpecified = true; >- tag.value = VALUE; >+ tag = new MockSetSupport(VALUE); > Assert.assertSame(VALUE, tag.getResult()); > } > > @Test > public void testResultFromNullValueAttribute() { >- tag.valueSpecified = true; >- tag.value = null; >+ tag = new MockSetSupport(null); > Assert.assertNull(tag.getResult()); > } > > @Test > public void testResultFromBodyContent() { >- tag.valueSpecified = false; >+ tag = new MockSetSupport(); > BodyContent bodyContent = createMock(BodyContent.class); > expect(bodyContent.getString()).andStubReturn(" Hello "); > replay(bodyContent); >@@ -365,19 +345,67 @@ > > @Test > public void testResultFromNullBodyContent() { >- tag.valueSpecified = false; >+ tag = new MockSetSupport(); > tag.setBodyContent(null); > Assert.assertEquals("", tag.getResult()); > } > > @Test > public void testResultFromEmptyBodyContent() { >- tag.valueSpecified = false; >+ tag = new MockSetSupport(); > BodyContent bodyContent = createMock(BodyContent.class); > expect(bodyContent.getString()).andStubReturn(null); > Assert.assertEquals("", tag.getResult()); > } > >+ public static class MockSetSupport extends SetSupport { >+ private final boolean valueSpecified; >+ private final Object value; >+ private final Object target; >+ private final String property; >+ >+ public MockSetSupport() { >+ this.value = null; >+ this.valueSpecified = false; >+ this.target = null; >+ this.property = null; >+ } >+ >+ public MockSetSupport(Object value, Object target, String property) { >+ this.value = value; >+ this.valueSpecified = true; >+ this.target = target; >+ this.property = property; >+ } >+ >+ public MockSetSupport(Object value) { >+ this.value = value; >+ this.valueSpecified = true; >+ this.target = null; >+ this.property = null; >+ } >+ >+ @Override >+ protected boolean isValueSpecified() { >+ return valueSpecified; >+ } >+ >+ @Override >+ protected Object evalValue() { >+ return value; >+ } >+ >+ @Override >+ protected Object evalTarget() { >+ return target; >+ } >+ >+ @Override >+ protected String evalProperty() { >+ return property; >+ } >+ } >+ > public static class Bean { > private String property; > >Index: src/main/java/org/apache/taglibs/standard/tag/el/core/SetTag.java >=================================================================== >--- src/main/java/org/apache/taglibs/standard/tag/el/core/SetTag.java (revision 1023287) >+++ src/main/java/org/apache/taglibs/standard/tag/el/core/SetTag.java (working copy) >@@ -13,118 +13,73 @@ > * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > * See the License for the specific language governing permissions and > * limitations under the License. >- */ >+ */ > > package org.apache.taglibs.standard.tag.el.core; > >-import javax.servlet.jsp.JspException; >+import javax.el.ValueExpression; > >-import org.apache.taglibs.standard.tag.common.core.NullAttributeException; > import org.apache.taglibs.standard.tag.common.core.SetSupport; > > /** >- * <p>A handler for <set>, which redirects the browser to a >- * new URL. >+ * JSTL 1.0 compatible version of <set> that accepts expression text for attribute values. > * > * @author Shawn Bayern > */ > > public class SetTag extends SetSupport { > >- //********************************************************************* >- // 'Private' state (implementation details) >+ private boolean valueSpecified; >+ private ValueExpression valueExpression; >+ private ValueExpression targetExpression; >+ private ValueExpression propertyExpression; > >- private String value_; // stores EL-based property >- private String target_; // stores EL-based property >- private String property_; // stores EL-based property >- >- >- //********************************************************************* >- // Constructor >- > public SetTag() { >- super(); >- init(); > } > >- >- //********************************************************************* >- // Tag logic >- >- // evaluates expression and chains to parent >- public int doStartTag() throws JspException { >- >- // evaluate any expressions we were passed, once per invocation >- evaluateExpressions(); >- >- // chain to the parent implementation >- return super.doStartTag(); >- } >- >- >- // Releases any resources we may have (or inherit) >+ @Override > public void release() { >+ valueExpression = null; >+ targetExpression = null; >+ propertyExpression = null; >+ valueSpecified = false; > super.release(); >- init(); > } > >+ public void setValue(String value) { >+ valueExpression = createExpression(value, Object.class); >+ valueSpecified = true; >+ } > >- //********************************************************************* >- // Accessor methods >+ public void setTarget(String target) { >+ targetExpression = createExpression(target, Object.class); >+ } > >- public void setValue(String value_) { >- this.value_ = value_; >- this.valueSpecified = true; >+ public void setProperty(String property) { >+ propertyExpression = createExpression(property, String.class); > } > >- public void setTarget(String target_) { >- this.target_ = target_; >+ private ValueExpression createExpression(String property, Class<?> type) { >+ return getExpressionFactory().createValueExpression(pageContext.getELContext(), property, type); > } > >- public void setProperty(String property_) { >- this.property_ = property_; >+ @Override >+ protected boolean isValueSpecified() { >+ return valueSpecified; > } > >+ @Override >+ protected Object evalValue() { >+ return valueExpression.getValue(pageContext.getELContext()); >+ } > >- //********************************************************************* >- // Private (utility) methods >- >- // (re)initializes state (during release() or construction) >- private void init() { >- // null implies "no expression" >- value_ = target_ = property_ = null; >+ @Override >+ protected Object evalTarget() { >+ return targetExpression.getValue(pageContext.getELContext()); > } > >- /* Evaluates expressions as necessary */ >- private void evaluateExpressions() throws JspException { >- /* >- * Note: we don't check for type mismatches here; we assume >- * the expression evaluator will return the expected type >- * (by virtue of knowledge we give it about what that type is). >- * A ClassCastException here is truly unexpected, so we let it >- * propagate up. >- */ >- >- // 'value' >- try { >- value = ExpressionUtil.evalNotNull( >- "set", "value", value_, Object.class, this, pageContext); >- } catch (NullAttributeException ex) { >- // explicitly let 'value' be null >- value = null; >- } >- >- // 'target' >- target = ExpressionUtil.evalNotNull( >- "set", "target", target_, Object.class, this, pageContext); >- >- // 'property' >- try { >- property = (String) ExpressionUtil.evalNotNull( >- "set", "property", property_, String.class, this, pageContext); >- } catch (NullAttributeException ex) { >- // explicitly let 'property' be null >- property = null; >- } >+ @Override >+ protected String evalProperty() { >+ return (String) propertyExpression.getValue(pageContext.getELContext()); > } > } >Index: src/main/java/org/apache/taglibs/standard/tag/rt/core/SetTag.java >=================================================================== >--- src/main/java/org/apache/taglibs/standard/tag/rt/core/SetTag.java (revision 1023287) >+++ src/main/java/org/apache/taglibs/standard/tag/rt/core/SetTag.java (working copy) >@@ -20,29 +20,59 @@ > import org.apache.taglibs.standard.tag.common.core.SetSupport; > > /** >- * <p>Tag handler for <set> in JSTL's rtexprvalue library.</p> >+ * JSTL 1.1 compatible version of <set> that accepts expression results for attribute values. > * > * @author Shawn Bayern > */ > > public class SetTag extends SetSupport { >+ private boolean valueSpecified; >+ private Object value; >+ private Object target; >+ private String property; > >- //********************************************************************* >- // Accessors >+ public SetTag() { >+ } > >- // for tag attribute > public void setValue(Object value) { > this.value = value; > this.valueSpecified = true; > } > >- // for tag attribute > public void setTarget(Object target) { > this.target = target; > } > >- // for tag attribute > public void setProperty(String property) { > this.property = property; > } >+ >+ @Override >+ public void release() { >+ value = null; >+ target = null; >+ property = null; >+ valueSpecified = false; >+ super.release(); >+ } >+ >+ @Override >+ protected boolean isValueSpecified() { >+ return valueSpecified; >+ } >+ >+ @Override >+ protected Object evalValue() { >+ return value; >+ } >+ >+ @Override >+ protected Object evalTarget() { >+ return target; >+ } >+ >+ @Override >+ protected String evalProperty() { >+ return property; >+ } > } >Index: src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java >=================================================================== >--- src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java (revision 1023287) >+++ src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java (working copy) >@@ -26,6 +26,7 @@ > > import java.util.Map; > >+import javax.servlet.jsp.JspApplicationContext; > import javax.servlet.jsp.JspException; > import javax.servlet.jsp.JspFactory; > import javax.servlet.jsp.JspTagException; >@@ -44,22 +45,13 @@ > /** > * <p>Support for handlers of the <set> tag.</p> > * >- * <p>The protected <code>value</code> and <code>valueSpecified</code> >- * attributes must be set in sync. That is, if you set the value then >- * you should set <code>valueSpecified</code> to <code>true<code>, if you unset the value, then >- * you should set <code>valueSpecified</code> to <code>false<code>. </p> >- * > * @author Shawn Bayern > */ >-public class SetSupport extends BodyTagSupport { >+public abstract class SetSupport extends BodyTagSupport { > > //********************************************************************* > // Internal state > >- protected Object value; // tag attribute >- protected boolean valueSpecified; // status >- protected Object target; // tag attribute >- protected String property; // tag attribute > private String var; // tag attribute > private String scope; // tag attribute > >@@ -71,20 +63,14 @@ > * not provide other constructors and are expected to call the > * superclass constructor. > */ >- public SetSupport() { >+ protected SetSupport() { > super(); >- init(); > } > >- // resets local state >- private void init() { >- value = target = property = var = scope = null; >- valueSpecified = false; >- } >- > // Releases any resources we may have (or inherit) > public void release() { >- init(); >+ var = null; >+ scope = null; > super.release(); > } > >@@ -94,27 +80,30 @@ > > public int doEndTag() throws JspException { > >- // what we'll store in scope:var >- Object result = getResult(); >- > // decide what to do with the result > if (var != null) { >- exportToVariable(result); >- } else if (target == null) { >- // can happen if target evaluates to null >- throw new JspTagException(Resources.getMessage("SET_INVALID_TARGET")); >- } else if (target instanceof Map) { >- exportToMapProperty(result); >+ exportToVariable(getResult()); > } else { >- exportToBeanProperty(result); >+ Object target = evalTarget(); >+ if (target == null) { >+ // can happen if target evaluates to null >+ throw new JspTagException(Resources.getMessage("SET_INVALID_TARGET")); >+ } >+ >+ String property = evalProperty(); >+ if (target instanceof Map) { >+ exportToMapProperty(target, property, getResult()); >+ } else { >+ exportToBeanProperty(target, property, getResult()); >+ } > } > > return EVAL_PAGE; > } > > Object getResult() { >- if (valueSpecified) { >- return value; >+ if (isValueSpecified()) { >+ return evalValue(); > } else if (bodyContent == null) { > return ""; > } else { >@@ -128,6 +117,35 @@ > } > > /** >+ * Indicates that the value attribute was specified. >+ * If no value attribute is supplied then the value is taken from the tag's body content. >+ * >+ * @return true if the value attribute was specified >+ */ >+ protected abstract boolean isValueSpecified(); >+ >+ /** >+ * Evaluate the value attribute. >+ * >+ * @return the result of evaluating the value attribute >+ */ >+ protected abstract Object evalValue(); >+ >+ /** >+ * Evaluate the target attribute. >+ * >+ * @return the result of evaluating the target attribute >+ */ >+ protected abstract Object evalTarget(); >+ >+ /** >+ * Evaluate the property attribute. >+ * >+ * @return the result of evaluating the property attribute >+ */ >+ protected abstract String evalProperty(); >+ >+ /** > * Export the result into a scoped variable. > * > * @param result the value to export >@@ -173,9 +191,11 @@ > /** > * Export the result into a Map. > * >+ * @param target the Map to export into >+ * @param property the key to export into > * @param result the value to export > */ >- void exportToMapProperty(Object result) { >+ void exportToMapProperty(Object target, String property, Object result) { > @SuppressWarnings("unchecked") > Map<Object, Object> map = (Map<Object, Object>) target; > if (result == null) { >@@ -188,10 +208,12 @@ > /** > * Export the result into a bean property. > * >+ * @param target the bean to export into >+ * @param property the bean property to set > * @param result the value to export > * @throws JspTagException if there was a problem exporting the result > */ >- void exportToBeanProperty(Object result) throws JspTagException { >+ void exportToBeanProperty(Object target, String property, Object result) throws JspTagException { > PropertyDescriptor[] descriptors; > try { > descriptors = Introspector.getBeanInfo(target.getClass()).getPropertyDescriptors(); >@@ -234,11 +256,14 @@ > return null; > } > Class<?> expectedType = m.getParameterTypes()[0]; >- JspFactory jspFactory = JspFactory.getDefaultFactory(); >- ExpressionFactory expressionFactory = jspFactory.getJspApplicationContext(pageContext.getServletContext()).getExpressionFactory(); >- return expressionFactory.coerceToType(value, expectedType); >+ return getExpressionFactory().coerceToType(value, expectedType); > } > >+ protected ExpressionFactory getExpressionFactory() { >+ JspApplicationContext appContext = JspFactory.getDefaultFactory().getJspApplicationContext(pageContext.getServletContext()); >+ return appContext.getExpressionFactory(); >+ } >+ > //********************************************************************* > // Accessor methods >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 33934
: 26179