[PATCH] Set lock pointer at the start of foreach (Re: 2.3.12 transaction problem within skiplist DB->foreach())
brong at fastmail.fm
Fri Aug 29 02:31:27 EDT 2008
On Thu, Aug 28, 2008 at 06:17:44PM -0400, John Capo wrote:
> Aug 28 14:04:24 m4 syncserver: Failed to access inbox for yadda
> Aug 28 14:04:25 m4 syncserver: Fatal error: Internal error: assertion failed: cyrusdb_skiplist.c: 622: db-> lock_status == UNLOCKED
> Aug 28 14:04:25 m4 syncserver: skiplist: closed while still locked
> Foreach bug, annotation code DB API violation, or ???
Ok - it was a foreach bug :(
Please find attached a tested patch that fixes the problem and refactors
the bloody mess that was locking code into a couple of nice, neat
a) does away with non-malloc transactions. They added needless
complexity (I already had a separate patch dealing with some
of that which this one obsoletes). A malloc doesn't cost that
b) moves the malloc into newtxn
c) adds a lock_or_update function which can be called whenever you
need to be in a write-lock, and it just does the right thing,
either updating the current txn or calling newtxn.
d) standarises variable naming: tidptr for the 'struct txn **'
everywhere, tid for a single pointer. So code looks the same
in all functions.
e) fixes John's bug :) By putting the transaction into the double
pointer at the start rather than the end of the foreach. In
fact, by always updating the pointer at the start. It really
simplifies the pointer handling considerably.
This is the refactor I should have done back when I was dealing
with this area adding current_txn checks and all that jazz. It
was a mess of copy-paste code.
I'm running this on one server now, with no problems. I'm quite
confident that it's correct. I'll be updating the rest of our
servers in a moment.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 12936 bytes
Desc: not available
Url : http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20080829/9ec253a1/attachment.bin
More information about the Cyrus-devel