2.3.12 transaction problem within skiplist DB->foreach()

[Bron Gondwana]

On Thu, 28 Aug 2008 19:52:19 -0400, "John Capo" <jc at irbs.com> said:
> I'm convinced this is a foreach() bug.  myforeach() is a copy of
> myfetch() with a loop in the middle for the callback.  This bug has
> been there since day one but showed up as just another unexplained
> IOERROR message untill the asserts were added in 2.3.12.
> 
> Patch attached that's running on two test boxes.

Oooh, I see what you mean.  Yes - of course.  The double pointer 'txn **tid'
gets passed in, and may be needed DURING the foreach by subsidiary code,
but that's not possible because it doesn't actually get set until the end of
the function.  How annoying.

Your patch looks viable, though it causes unreachable code later in the
function, and I'd probably update myfetch to be matching in behaviour
just for clarity.  I'll put that together now.

Bron.

> Quoting John Capo (jc at irbs.com):
> > . OK User logged in
> > . rename user/abox user/bbox
> > * BYE Fatal error: Internal error: assertion failed: cyrusdb_skiplist.c: 627: db->lock_status == UNLOCKED
> > 
> > This results from attempting to update annotations.db in a DB->foreach() callback.
> > 
> >     assert(db->lock_status == UNLOCKED)  in write_lock()
> > 
> > cmd_rename() -> annotatemore_rename() -> annotatemore_findall() -> foreach() -> rename_cb()
> > 
> > /* foreach allows for subsidary mailbox operations in 'cb'.
> >    if there is a txn, 'cb' must make use of it.
> > */
> > 
> > That comment makes sense but the transaction structure created in
> > foreach() is not made available to the callback.  foreach() does
> > provide the transaction structure it created when foreach() returns
> > but that's too late for the callback to use the transaction.
> > 
> > Sometimes the same assert() is hit when syncserver tries to create
> > a new mailbox.  I haven't look into this one yet.
> > 
> > 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 ???
> > 
> > If the comment above is correct, the bug is foreach() not providing
> > the transaction to the callback.   Annotation code maybe the only place
> > where updates are done from a foreach() callback.  Finding all the rocks
> > and procs is tedious.
> > 
> > John Capo
> > 
> > 
> > 
-- 
  Bron Gondwana
  brong at fastmail.fm