Bug 31105 - Transient locks are not released due to suspended state within ExtendedStore
Transient locks are not released due to suspended state within ExtendedStore
Status: NEW
Product: Slide
Classification: Unclassified
Component: Core
Other other
: P3 critical (vote)
: ---
Assigned To: Slide Developer List
Depends on:
Blocks: 31521
  Show dependency tree
Reported: 2004-09-07 18:49 UTC by Steve Vaughan
Modified: 2005-04-04 01:56 UTC (History)
0 users

Diff for ExtendedStore.java (4.04 KB, patch)
2004-09-07 19:10 UTC, Steve Vaughan
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Vaughan 2004-09-07 18:49:22 UTC
The field "locks" holds a HashSet instance for each active thread, containing 
the transient locks (GenericLock instances).  When a transaction is suspended, 
the set is moved from "locks" to "suspendedLocks", under a key based upon the 
Xid for the transaction branch.

When a transaction calls end(Xid,int) with SUCCESS or FAILURE, if the 
transaction is currently suspended, then the field "activeTransactionBranch" 
is cleared, leaving the locks within "suspendedLocks".

Although commit(Xid,boolean), rollback(Xid), and forget(Xid) all call 
releaseTransientLocks(), the release method depends upon 
the "activeTransactionBranch" field.  Since the activeTransactionBranch is 
null, the locks are not actually released.
Comment 1 Steve Vaughan 2004-09-07 19:10:23 UTC
Created attachment 12666 [details]
Diff for ExtendedStore.java
Comment 2 Steve Vaughan 2004-09-07 19:15:11 UTC
I've attached a patch which ensures that the locks are released, but I don't 
think that the patch is quite correct.  It seems to me that the locks should 
be held until the transaction is completed (commit(), rollback(), or forget
()).  The existing code just nulled out the set within end().  In addition, 
the existing code doesn't seem to handling JOIN correctly.  I've left in the 
comments I wrote as I was trying to debug this, to try to explain my thinking, 
but someone should definitely take a look at the lock related code.
Comment 3 Oliver Zeigermann 2004-09-07 21:20:44 UTC
Could you describe the scenario in which the error occurs? I could not reproduce
it, everything seems to run fine...

As a sidenote, the transient locks may be of some use, but not of as much use as
I hoped. They might be more useful in future versions where other stuff changes
as well. It might be an option to inactivate them in order to lower complexity.
What do you think?
Comment 4 Johan Stuyts 2005-04-04 09:56:52 UTC
This bug is also a memory leak, that's why I changed the severity to critical.

To prevent the memory leak from occuring I changed ExtendedStore in almost the 
same way as Steve did. The difference is that I don't release the transient 
locks in end(...), but only do this in forget(...), rollback(...) and commit(...
). Looking at the logs I saw that after a suspend one of forget(...), rollback(.
..) and commit(...) will be called. I don't know if this happens when an end or 
a fail is passed to end(...). If it does the releasing of the locks in end(...) 
is not necessary.

I am interested to know what the answer to the question in Steve's patch, 'Do we 
want to throw away the locks when joining?', is.