Branko Čibej <[email protected]> writes: > On 28.10.2013 12:24, Philip Martin wrote: >> Branko Čibej <[email protected]> writes: >> >>> On 28.10.2013 10:07, Branko Čibej wrote: >>>> It turns out that sqlite3_reset fails, which is why we ultimately get >>>> the assertion during pool cleanup. This appears to be due to a bug in >>>> our code; we call svn_sqlite__reset (and thus sqlite3_reset) if >>>> sqlite3_step fails (this is line 316 in sqlite.c), and that is >>>> documented to fail as per >> sqlite3_reset is documented to return an error but I don't think that's >> the same as failing, it's just propagating any sqlite3_step error. > > The result is that we get the same error in the chain twice; we don't > actually know that the statement is reset; and on top of everything > else, in such cases we do not clear the svn_sqlite__stmt_t::needs_reset > flag, which leads to another error during pool cleanup (or an abort in > maintainer mode).
So we rely on this code in svn_fs_fs__set_rep_reference, perhaps other places as well, but it works ther because the statement is an INSERT and there is only ever one step, so needs_reset is never set. We should write a regression test for out SQLite wrapper code with a statement that has a successful step before a failed step. Does that mean it has to be a SELECT? If so, how do we get it to fail after a successful step? Could we use a scalar function to do that sort of thing? -- Philip Martin | Subversion Committer WANdisco // *Non-Stop Data*
