Hi Antranig!
2006/5/23, Antranig Basman <[EMAIL PROTECTED]>:
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);
}
}
Hmmm. Do you argue that the owner can be removed from the list of
owners as it no longer holds any locks at this particular point of
time? That's what I presume.
I wonder, is it possible that another thread acquires a new lock for
that specific owner in the meantime? So that this removal would
discard that. Which of course would be an error.
Is that possible?
Anyway, there really should be a way to get rid of an owner if it
really is no longer needed. Maybe a new method for that?
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.
globalOpenResources keeps track of all open resources. That is
important as they will have to be closed when the manager is shut
down.
Can we just get away with synchronizing on "openResourcs" inside
context.closerResources() and context.registerResource?
No need to synchronize on openResourcs (oops, there is a typo!). What
for? Those methodes are synchronized (on the context) anyway.
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.
That seems reasonable to me. However, the whole topic is so
complicated I wouldn't want to change that before more intense
thinking. I would be pleased if you could have a closed look to those
leaks and test the patches properly.
Cheers
Oliver
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
