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] [Updated] (LANG-739) ToStringBuilder leaks memory if toString method causes hash code to be changed
Date Wed, 10 Aug 2011 09:29:27 GMT

     [ https://issues.apache.org/jira/browse/LANG-739?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Philippe Renon updated LANG-739:
--------------------------------

    Description: 
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.


  was:
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 differen change 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.



> 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
>
> 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 is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message