[
https://issues.apache.org/jira/browse/LANG-739?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13802899#comment-13802899
]
Woonsan Ko commented on LANG-739:
---------------------------------
Hi [~prenon],
{quote}
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.
{quote}
What do you mean by 'using the system identity hash code when registering
objects'? I don't see any registry or any object registration code in
ToStringBuilder class (in trunk; 3.2-SNAPSHOT). Could you elaborate it more or
did I miss anything?
{quote}
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.
{quote}
HashCodeBuilder.reflectionHashCode(this) should always calculate hash code from
the current field values anyway, so I'm not sure if there can be a better way
when the operation is invoked in your #hashCode() implementation and a non
transient instance member is changing every time. I'm afraid "mutating" needs
for some good reasons should use transient member as best practice.
By the way, there are two other possible workarounds:
1 - To invoke HashCodeBuilder.reflectionHashCode(this, "toString") instead.
2 - To invoke HashCodeBuilder#append() and HashCodeBuilder#toHashCode() only
for meaningful members by yourself instead.
Regards,
Woonsan
> 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: Patch Needed
>
>
> 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)
