Branko Čibej <brane_at_wandisco.com> writes:
> On 28.10.2013 12:24, Philip Martin wrote:
>> Branko Čibej <brane_at_wandisco.com> 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*
Received on 2013-10-28 13:49:10 CET