commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philippe Renon (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (LANG-739) ToStringBuilder leaks memory if toString method causes hash code to be changed
Date Fri, 25 Oct 2013 13:48:31 GMT

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

Philippe Renon edited comment on LANG-739 at 10/25/13 1:47 PM:
---------------------------------------------------------------

Had a quick look the patch. It looks good but 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.


was (Author: prenon):
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)

Mime
View raw message