On 28.10.2013 13:48, Philip Martin wrote: > 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.
Bert just changed svn_sqlite__reset to do something slightly better in that the result is more predictable, regardless of previous errors. > 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? Maybe just try to run the tests with SQLite 3.7.12 ... worked like a charm for me to reproduce this error. Of course, I still have no idea whatsoever why the step failed, the error message isn't very informative. -- Brane -- Branko Čibej | Director of Subversion WANdisco // Non-Stop Data e. [email protected]
