commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Antranig Basman <antra...@caret.cam.ac.uk>
Subject [transaction] Memory leaks in GenericLockManager, FileResourceManager
Date Mon, 22 May 2006 22:00:06 GMT
In recent load testing I've uncovered a number of leaks in 
commons-transaction, in the two components mentioned above.
I believe the reason these have not previously come to light might
well be that I am using the "lightweight transaction" model, 
whereby no transaction argument is supplied to the ResourceManager,
and it creates a "lightweight transaction" intended to endure while
the stream is kept open.
I guess this code path is not well tested, in favour of the
heavyweight transaction model since I have found so far I believe 
three leaks.

The most straightforward of them is in GenericLockManager, where
the lock owner is not being released properly - I have
the following:

Index:
E:/workspace/commons-transaction-test/src/java/org/apache/commons/transaction/locking/GenericLockManager.java
===================================================================
---
E:/workspace/commons-transaction-test/src/java/org/apache/commons/transaction/locking/GenericLockManager.java
(revision 408762)
+++
E:/workspace/commons-transaction-test/src/java/org/apache/commons/transaction/locking/GenericLockManager.java
(working copy)
@@ -337,6 +337,8 @@
                 lock.release(ownerId);
                 locks.remove(lock);
             }
+            // AMB fix leak 
+            globalOwners.remove(ownerId);
         }
     }
     

There seem to be two leaks in FileResourceManager,
but I gave up trying to patch them precisely on seeing that they
both centred around the globalOpenResources table, which as far
as I can see is not used anywhere in the codebase, and only 
seems to serve the function of hanging onto FileInputStream objects 
long after they should have been collected :P - after a few attempts I
just commented out all references to this member entirely.

Can we just get away with synchronizing on "openResourcs" inside
context.closerResources() and context.registerResource?

If anyone is interested I could renew my efforts to find more
fine-grained patches - for example in the current version rev
375638 there should definitely at line 1628 be a call to

 globalOpenResources.remove(is);

following the call to 
globalTransactions.remove(txId);

I can't recall where the 3rd leak site was, but it was certainly
resolved by commenting out globalOpenResources.

Antranig.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Mime
View raw message