Merge lp:~pbeaman/akiban-persistit/fix-1032701-interrupts-leave-latches into lp:akiban-persistit
Status: | Merged |
---|---|
Approved by: | Nathan Williams |
Approved revision: | 349 |
Merged at revision: | 349 |
Proposed branch: | lp:~pbeaman/akiban-persistit/fix-1032701-interrupts-leave-latches |
Merge into: | lp:akiban-persistit |
Diff against target: |
180 lines (+63/-23) 4 files modified
src/main/java/com/persistit/BufferPool.java (+13/-2) src/main/java/com/persistit/Transaction.java (+10/-1) src/main/java/com/persistit/TransactionIndex.java (+25/-19) src/test/java/com/persistit/TransactionTest2.java (+15/-1) |
To merge this branch: | bzr merge lp:~pbeaman/akiban-persistit/fix-1032701-interrupts-leave-latches |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Nathan Williams | Approve | ||
Review via email: mp+118253@code.launchpad.net |
Description of the change
Fixes bug 1032701 - Interrupt causes Thread to exit without releasing claims. This bug was caused by an interrupt occurring at a dangerous place in the code. Specifically, in the BufferPool#get(...) method, the try/finally around the actual reading of a page from disk contained a call to BufferPool#
This branch fixes this by ensuring that the invalidate(Buffer) method does not throw an interrupt. Because this is asserted by a change in the signature of the invalidate(Buffer) and because injecting timing conditions to test this is a bit awkward, there is no unit test for this. Also, there is no proof that no other code paths can cause a similar condition. However, I carefully reviewed all the finally{} blocks that could have the same effect, and also have verified that the test which originally detected the failure no longer fails.
Note that the invalidate() method polls the detach(Buffer) method, which is necessary to avoid deadlock. That it does so is not a change; the only change is that this loop is no longer interruptible.
Also changed are a couple of other items noticed while debugging this:
If a commit() fails due to an I/O failure or an interrupt, the intended action is to abort the transaction. That was being done correctly, but the _rollbackPending flag was not set, causing a spurious warning from the end() method such as:
[TransactionThr
(That the TransactionStatus is ABORTED is inconsistent with the warning.) The _rollbackPending and _rollbackCompleted flags are now managemed more correctly.
Also, an attempt to begin a nested transaction within an aborted transaction context now correctly issues an IllegalStateExc
Explanation and change make sense. Other fixes are good, too.