[ 
https://issues.apache.org/jira/browse/LANG-739?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13805299#comment-13805299
 ] 

Philippe Renon commented on LANG-739:
-------------------------------------

Had a quick look the patch and I think it has an issue. 
Because the only reference to the IDKey is held by the WeakHashMap, there is a 
risk that it gets garbage collected at any time... 
Additionaly I think that the HashCodeBuilder does not use a WeakHashMap.

> ToStringBuilder leaks memory if toString method causes hash code to be changed
> ------------------------------------------------------------------------------
>
>                 Key: LANG-739
>                 URL: https://issues.apache.org/jira/browse/LANG-739
>             Project: Commons Lang
>          Issue Type: Bug
>          Components: lang.builder.*
>    Affects Versions: 2.3
>            Reporter: Philippe Renon
>             Fix For: Review Patch
>
>         Attachments: LANG-739-patch.txt
>
>
> We have the following abstract class:
> {code}
> public class AbstractMessageItem {
>     private String toString;
>     public boolean equals(final Object obj) {
>         return EqualsBuilder.reflectionEquals(this, obj);
>     }
>     public int hashCode() {
>         return HashCodeBuilder.reflectionHashCode(this);
>     }
>     public String toString() {
>         if (toString == null) {
>             toString = ToStringBuilder.reflectionToString(this);
>         }
>         return toString;
>     }
> }
> {code}
> We also have two concrete classes extending the above class and one of them 
> has a reference to the other.
> Now, if we call toString() on the 1st one, this will in turn call toString() 
> on the second one.
> The call to toString() on the second one will cause its hash code to be 
> changed and as a consequence will also change the hashCode of the first one 
> *while* computing its toString().
> This causes the _infinite loop avoidance_ mechanism (i.e. the registry) to 
> fail to unregister some objects and memory will be leaked.
> I believe that this leak can be avoided by using the system identity hash 
> code when registering objects (as is done in HashCodeBuilder) instead of the 
> user hash code.
> I know the issue can be worked around by removing the toString field (and 
> loosing a dubious "performance enhancement" hack) or by making it transient, 
> but I think that other "mutating" toString() methods can happen in the field 
> (sometimes for good reasons) and fixing ToStringBuilder can be of help in 
> some cases.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to