zopefoundation/ZEO — pull request #1 by ossmkitty (2013-03-04 13:22:17+00:00)
Title: ClientStorage.py can deadlock in tpc_begin

`ClientStorage.tpc_begin` acquires the `self._tpc_cond` Condition and then goes into a loop in which it calls `_tpc_cond.wait`. The Condition is released in two places, one being if the loop decides to prematurely exit ("Duplicate tpc_begin for same transaction"), and one being if the loop finishes as expected.

However, if the loop throws an exception, then the `_tpc_cond` Condition is never released and subsequent calls to `tpc_begin` for that instance will simply hang in a deadlock. The solution seems to be to ensure that `_tpc_cond` is released in a `finally` block. (This conforms to the rest of ClientStorage, which is careful to acquire locks and release them in a `finally` block.)

The attached patch moves the release into a `finally` block; it makes a similar change to `end_transaction`. (The diff looks scary due to the whitespace change, but the only real changes are the introduction of `try` and `finally`, and removing one (now) duplicate `release`.)

## Details on exception

Our application uses Gevent and greenlets. The call to `_tpc_cond.wait` serves as a cooperative-scheduling junction, putting that greenlet (co-routine) to sleep and letting a different one run. A sleeping greenlet can be told to exit, causing it to raise GreenletExit and unwind the stack next time it executes (exiting a greenlet is implemented this way specifically to allow `finally` blocks the chance to run). However, if a greenlet is asked to exit while in `tpc_begin` and waiting on this condition, `_tpc_cond.wait` appears to raise the GreenletExit exception, no `finally` block runs and the Condition is effectively leaked leading to deadlock.

I do not know if there is a situation in plain CPython that can cause Condition's `wait` method to ever throw an exception (interrupts?), but it does seem like good defensive programming to release the Condition in a `finally` block.

(Initially mentioned as [Bug #1048644](https://bugs.launchpad.net/zodb/+bug/1048644) in launchpad, moving here to match the code move.)