[DBD-SQLite] [PATCH v2] Don't set imp_sth->stmt to NULL on error path

[Yuriy Kaminskiy]

Yuriy Kaminskiy wrote:
> Yuriy Kaminskiy wrote:
>> Hello!
>> I noticed strange problem on error path handling.
>>
>> Not sure if this sqlite error, DBD::SQLite error, or some misunderstanding 
>> on my
>> part.
>>
>> Verified on DBD::SQLite-1.33 and 1.34_1 (with bundled sqlite.c) and 
>> DBI-1.616.
>>
>> my $sth=$dbh->prepare("SELECT * FROM t");
>> $sth->execute; # success - 0E0
>> my $row1 = $sth->fetch; # returns [1]
>> $sth->finish;
>> sleep 5;
>> # ^^ here another instance issued BEGIN EXCLUSIVE ...
>> $sth->execute; # ... so this returns "database is locked";
>> # it's legitimate/expected behavior
>> #
>> $sth->finish; # just in case (clears error)
>> #
>> sleep 5;
>> # ^^ here another instance already COMMIT'ed, DB is not locked anymore
>> #
>> $sth->execute; # success - 0E0
>> my $row2 = $sth->fetch; # returns undef
>> #
>> # WTF?
>> #
>> # If I prepare another statement (cloned from first!),
>> # it returns row as expected:
>> #
>> my $sth2 = $dbh->prepare($sth->{Statement});
>> $sth2->execute; # success
>> my $row3 = $sth2->fetch; # returns [1]
> 
> With attached patch it works as expected. Seems passes regression test, even
> under valgrind (but maybe regression test should be improved, so that it won't
> pass it *before* this patch).
> 
> This bug was introduced (or, maybe, "incompletely fixed") by svn revision 5944
> (author: adamk/subject: "Changed finalize to reset per RT #32100 et al"/date:
> 2009-04-04 07:01:40), that replaced sqlite3_finalize with sqlite3_reset, but
> have not removed stmt NULLification.

v2: remove tabs, split unrelated changes to separate patch

Subject: fix $sth invalidation and memory leak after $sth->execute error
Regression/left-over from r5944 "Changed finalize to reset per RT #32100 et al"

--- DBD-SQLite-1.34_01/dbdimp.c.orig	2011-09-21 20:19:06.000000000 +0400
+++ DBD-SQLite-1.34_01/dbdimp.c	2011-10-05 07:41:00.000000000 +0400
@@ -802,7 +802,6 @@
             if (sqlite3_reset(imp_sth->stmt) != SQLITE_OK) {
                 sqlite_error(sth, imp_sth->retval, sqlite3_errmsg(imp_dbh->db));
             }
-            imp_sth->stmt = NULL;
             return -6; /* -> undef in SQLite.xsi */
     }
 }

Subject: explicitly set stmt to NULL after sqlite3_finalize

--- DBD-SQLite-1.34_01/dbdimp.c.orig	2011-09-21 20:19:06.000000000 +0400
+++ DBD-SQLite-1.34_01/dbdimp.c	2011-10-05 07:41:00.000000000 +0400
@@ -557,6 +557,7 @@
         sqlite_error(sth, rc, sqlite3_errmsg(imp_dbh->db));
         if (imp_sth->stmt) {
             rc = sqlite3_finalize(imp_sth->stmt);
+            imp_sth->stmt = NULL;
             if (rc != SQLITE_OK) {
                 sqlite_error(sth, rc, sqlite3_errmsg(imp_dbh->db));
             }
@@ -967,6 +968,7 @@
 
             /* finalize sth when active connection */
             rc = sqlite3_finalize(imp_sth->stmt);
+            imp_sth->stmt = NULL;
             if (rc != SQLITE_OK) {
                 sqlite_error(sth, rc, sqlite3_errmsg(imp_dbh->db));
             }

_______________________________________________
DBD-SQLite mailing list
DBD-SQLite@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbd-sqlite