[PATCH] Set lock pointer at the start of foreach (Re: 2.3.12 transaction problem within skiplist DB->foreach())

Bron Gondwana 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[77241]: Failed to access inbox for yadda
> Aug 28 14:04:25 m4 syncserver[77241]: Fatal error: Internal error: assertion failed: cyrusdb_skiplist.c: 622: db-> lock_status == UNLOCKED
> Aug 28 14:04:25 m4 syncserver[77241]: skiplist: closed while still locked
> 
> Foreach bug, annotation code DB API violation, or ???

Ok - it was a foreach bug :(

Ken,

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
functions.

Specifically, it:

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
   much.

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.

Regards,

Bron.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cyrus-skiplist-locking-rework-2.3.12.diff
Type: text/x-diff
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 mailing list