[jbossts-issues] [JBoss JIRA] Commented: (JBTM-352) Memory leak in TransactionReaper._timeouts

[Andrew Dinn (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBTM-352?page=comments#action_12405216 ] 
            
Andrew Dinn commented on JBTM-352:
----------------------------------

Well spotted. Unfortunately, this is probably not a valid fix. Essentially you are using the Uid as a key for entries in map _timeouts and this is inadequate for the general case.

The reason for using Reapable objects as the key is that it allows the comparison to be defined by the Reapable -- this is possible because when Hashmap does a get or put with key 'k' it compares against existing entries using k.equals(entry.key). So, any Reapable can implement 'equals' to ensure that it compares correctly against other Reapables. If we switch to keying using Uids then this test will by bypassed. Your 'fix' may work with your test but it may break other cases.

This feature is used when inserting and removing ControlWrapper objects. If the argument is a ControlWrapper the equals method checks whether both ControlWrappers are local. If so it checks whether their ControlImple fields are 'equal'. If either ControlWrapper is non-local then it has to invoke is_same_transaction on the associated Coordinators.

The equals method in ControlWrapper also contains a check for the case that the argument is  a Uid. If so it compares the supplied Uid argument to the wrapper's Uid. This is intended to catch the case where a Uid is supplied as argument to TransactionReaper.remove(). The reason this is failing is because the comparison is being done the wrong way round i.e. by calling Uid.equals(ControllWrapper) rather than ControlWrapper.equals(Uid).

This looks to me like a failed attempt to optimize the remove operation. A correct but, possibly, slow implementation might be to change all calls to remove which supply a Uid and instead supply a ControlWrapper constructed using a Control available where the call is made. I will look for a better option before attempting to implement this fix.

By the way, I only found two places where a non-Reapable is supplied, both in OTSManager.destroyControl. In one case the argument is a Uid, in the other a Control! I am not yet sure if the second case is correct or an error.

> Memory leak in TransactionReaper._timeouts
> ------------------------------------------
>
>                 Key: JBTM-352
>                 URL: http://jira.jboss.com/jira/browse/JBTM-352
>             Project: JBoss Transaction Manager
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: Transaction Core
>    Affects Versions: 4.3.0.GA
>         Environment: Compiled 4.3.0.GA for the EAP 4.3 as JTS according to the instructions (by setting JBOSS_HOME to jboss-5.0.0.Beta4) on RHEL4.
> Run ~ 1500 JMS transactions in EAP 4.3 all configuration on RHEL4 to see it leaks.
> Used Sun jdk 1.5.0.14 
>            Reporter: Ivan Szanto
>         Assigned To: Andrew Dinn
>             Fix For: 4.4.CR1
>
>         Attachments: TransactionReaper.java
>
>
> The leak can also be seen by using the debugger to check the number of entries in TransactionReaper._timeouts after a couple of transactions.
> It appears that the TransactionReaper.insert method uses ControlWrapper objects as keys when putting elements into the hashtable, but the TransactionReaper.remove method sometimes unsuccessfully tries to use Uid ojbects to remove them.
> I will post a corrected version of TransactionReaper.java.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira