hopefully fixed #62370 (segfaults during operations on disconnected handle

Changes

@@ -6,6 +6,8 @@ Changes for Perl extension DBD-SQLite
     - Resolved #61361: Problems building 1.31 with system SQLite (ISHIGAKI)
     - Resolved #61117: Supporting database as an alias for dbname in
       DSN (ISHIGAKI)
+    - Resolved #62370: Segfaults during operations on disconnected
+      handles (ISHIGAKI)
 
 1.31 Wed 15 Sep 2010
     - Production release, identical to 1.30_06

dbdimp.c

@@ -272,6 +272,11 @@ sqlite_db_commit(SV *dbh, imp_dbh_t *imp_dbh)
     dTHX;
     int rc;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to commit on inactive database handle");
+        return FALSE;
+    }
+
     if (DBIc_is(imp_dbh, DBIcf_AutoCommit)) {
         /* We don't need to warn, because the DBI layer will do it for us */
         return TRUE;
@@ -302,6 +307,11 @@ sqlite_db_rollback(SV *dbh, imp_dbh_t *imp_dbh)
     dTHX;
     int rc;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to rollback on inactive database handle");
+        return FALSE;
+    }
+
     if (DBIc_is(imp_dbh, DBIcf_BegunWork)) {
         DBIc_off(imp_dbh, DBIcf_BegunWork);
         DBIc_on(imp_dbh,  DBIcf_AutoCommit);
@@ -370,7 +380,6 @@ sqlite_db_disconnect(SV *dbh, imp_dbh_t *imp_dbh)
             sqlite_error(dbh, rc, sqlite3_errmsg(imp_dbh->db));
         }
     }
-    imp_dbh->db = NULL;
 
     av_undef(imp_dbh->functions);
     SvREFCNT_dec(imp_dbh->functions);
@@ -394,6 +403,8 @@ sqlite_db_destroy(SV *dbh, imp_dbh_t *imp_dbh)
     if (DBIc_ACTIVE(imp_dbh)) {
         sqlite_db_disconnect(dbh, imp_dbh);
     }
+    imp_dbh->db = NULL;
+
     DBIc_IMPSET_off(imp_dbh);
 }
 
@@ -409,7 +420,7 @@ sqlite_db_STORE_attrib(SV *dbh, imp_dbh_t *imp_dbh, SV *keysv, SV *valuesv)
     if (strEQ(key, "AutoCommit")) {
         if (SvTRUE(valuesv)) {
             /* commit tran? */
-            if ( (!DBIc_is(imp_dbh, DBIcf_AutoCommit)) && (!sqlite3_get_autocommit(imp_dbh->db)) ) {
+            if ( DBIc_ACTIVE(imp_dbh) && (!DBIc_is(imp_dbh, DBIcf_AutoCommit)) && (!sqlite3_get_autocommit(imp_dbh->db)) ) {
                 sqlite_trace(dbh, imp_dbh, 3, "COMMIT TRAN");
                 rc = sqlite_exec(dbh, "COMMIT TRANSACTION");
                 if (rc != SQLITE_OK) {
@@ -491,6 +502,11 @@ sqlite_db_last_insert_id(SV *dbh, imp_dbh_t *imp_dbh, SV *catalog, SV *schema, S
 {
     dTHX;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to get last inserted id on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     return sv_2mortal(newSViv((IV)sqlite3_last_insert_rowid(imp_dbh->db)));
@@ -505,14 +521,14 @@ sqlite_st_prepare(SV *sth, imp_sth_t *imp_sth, char *statement, SV *attribs)
     D_imp_dbh_from_sth;
 
     if (!DBIc_ACTIVE(imp_dbh)) {
-      sqlite_error(sth, -2, "attempt to prepare on inactive database handle");
+        sqlite_error(sth, -2, "attempt to prepare on inactive database handle");
-      return FALSE; /* -> undef in lib/DBD/SQLite.pm */
+        return FALSE; /* -> undef in lib/DBD/SQLite.pm */
     }
 
 #if 0
     if (*statement == '\0') {
-      sqlite_error(sth, -2, "attempt to prepare empty statement");
+        sqlite_error(sth, -2, "attempt to prepare empty statement");
-      return FALSE; /* -> undef in lib/DBD/SQLite.pm */
+        return FALSE; /* -> undef in lib/DBD/SQLite.pm */
     }
 #endif
 
@@ -757,6 +773,11 @@ sqlite_st_fetch(SV *sth, imp_sth_t *imp_sth)
     int chopBlanks = DBIc_is(imp_sth, DBIcf_ChopBlanks);
     int i;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(sth, -2, "attempt to fetch on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
     croak_if_stmt_is_null();
 
@@ -1116,6 +1137,10 @@ sqlite_db_busy_timeout(pTHX_ SV *dbh, int timeout )
 
     if (timeout) {
         imp_dbh->timeout = timeout;
+        if (!DBIc_ACTIVE(imp_dbh)) {
+            sqlite_error(dbh, -2, "attempt to set busy timeout on inactive database handle");
+            return -2;
+        }
         sqlite3_busy_timeout(imp_dbh->db, timeout);
     }
     return imp_dbh->timeout;
@@ -1213,9 +1238,15 @@ sqlite_db_create_function(pTHX_ SV *dbh, const char *name, int argc, SV *func)
 {
     D_imp_dbh(dbh);
     int rc;
+    SV *func_sv;
+
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to create function on inactive database handle");
+        return FALSE;
+    }
 
     /* Copy the function reference */
-    SV *func_sv = newSVsv(func);
+    func_sv = newSVsv(func);
     av_push( imp_dbh->functions, func_sv );
 
     croak_if_db_is_null();
@@ -1239,6 +1270,11 @@ sqlite_db_enable_load_extension(pTHX_ SV *dbh, int onoff)
     D_imp_dbh(dbh);
     int rc;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to enable load extension on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     /* COMPAT: sqlite3_enable_load_extension is only available for 3003006 or newer */
@@ -1443,9 +1479,15 @@ sqlite_db_create_aggregate(pTHX_ SV *dbh, const char *name, int argc, SV *aggr_p
 {
     D_imp_dbh(dbh);
     int rc;
+    SV *aggr_pkg_copy;
+
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to create aggregate on inactive database handle");
+        return FALSE;
+    }
 
     /* Copy the aggregate reference */
-    SV *aggr_pkg_copy = newSVsv(aggr_pkg);
+    aggr_pkg_copy = newSVsv(aggr_pkg);
     av_push( imp_dbh->aggregates, aggr_pkg_copy );
 
     croak_if_db_is_null();
@@ -1539,6 +1581,11 @@ sqlite_db_create_collation(pTHX_ SV *dbh, const char *name, SV *func)
 
     SV *func_sv = newSVsv(func);
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to create collation on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     /* Check that this is a proper collation function */
@@ -1603,6 +1650,11 @@ sqlite_db_collation_needed(pTHX_ SV *dbh, SV *callback)
 {
     D_imp_dbh(dbh);
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to see if collation is needed on inactive database handle");
+        return;
+    }
+
     croak_if_db_is_null();
 
     /* remember the callback within the dbh */
@@ -1645,6 +1697,11 @@ sqlite_db_progress_handler(pTHX_ SV *dbh, int n_opcodes, SV *handler)
 {
     D_imp_dbh(dbh);
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to set progress handler on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     if (!SvOK(handler)) {
@@ -1671,6 +1728,11 @@ sqlite_db_commit_hook(pTHX_ SV *dbh, SV *hook)
     D_imp_dbh(dbh);
     void *retval;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to set commit hook on inactive database handle");
+        return &PL_sv_undef;
+    }
+
     croak_if_db_is_null();
 
     if (!SvOK(hook)) {
@@ -1698,6 +1760,11 @@ sqlite_db_rollback_hook(pTHX_ SV *dbh, SV *hook)
     D_imp_dbh(dbh);
     void *retval;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to set rollback hook on inactive database handle");
+        return &PL_sv_undef;
+    }
+
     croak_if_db_is_null();
 
     if (!SvOK(hook)) {
@@ -1752,6 +1819,11 @@ sqlite_db_update_hook(pTHX_ SV *dbh, SV *hook)
     D_imp_dbh(dbh);
     void *retval;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to set update hook on inactive database handle");
+        return &PL_sv_undef;
+    }
+
     croak_if_db_is_null();
 
     if (!SvOK(hook)) {
@@ -1823,6 +1895,11 @@ sqlite_db_set_authorizer(pTHX_ SV *dbh, SV *authorizer)
     D_imp_dbh(dbh);
     int retval;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to set authorizer on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     if (!SvOK(authorizer)) {
@@ -1859,6 +1936,11 @@ sqlite_db_backup_from_file(pTHX_ SV *dbh, char *filename)
     sqlite3 *pFrom;
     sqlite3_backup *pBackup;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to backup from file on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     rc = sqlite_open(filename, &pFrom);
@@ -1902,6 +1984,11 @@ sqlite_db_backup_to_file(pTHX_ SV *dbh, char *filename)
     sqlite3 *pTo;
     sqlite3_backup *pBackup;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to backup to file on inactive database handle");
+        return FALSE;
+    }
+
     croak_if_db_is_null();
 
     rc = sqlite_open(filename, &pTo);
@@ -2197,6 +2284,11 @@ int sqlite_db_register_fts3_perl_tokenizer(pTHX_ SV *dbh)
     const char zSql[] = "SELECT fts3_tokenizer(?, ?)";
     sqlite3_tokenizer_module *p = &perl_tokenizer_Module;
 
+    if (!DBIc_ACTIVE(imp_dbh)) {
+        sqlite_error(dbh, -2, "attempt to register fts3 tokenizer on inactive database handle");
+        return FALSE;
+    }
+
     rc = sqlite3_prepare_v2(imp_dbh->db, zSql, -1, &pStmt, 0);
     if( rc!=SQLITE_OK ){
         return rc;

t/08_busy.t

@@ -82,8 +82,10 @@ foreach my $call_func (@CALL_FUNCS) {
 
 	    # avoid resource collisions after fork
 	    # http://www.slideshare.net/kazuho/un-5457977
-	    $dbh->{InactiveDestroy} = 1;
+	    unless ($^O eq 'MSWin32') {  # ignore fork emulation
-	    undef $dbh;
+	        $dbh->{InactiveDestroy} = 1;
+	        undef $dbh;
+	    }
 
 	    my $dbh2 = DBI->connect("dbi:SQLite:$dbfile", '', '', 
 	    {

t/rt_62370_diconnected_handles_operation.t

@@ -0,0 +1,182 @@
+#!/usr/bin/perl
+
+use strict;
+BEGIN {
+	$|  = 1;
+	$^W = 1;
+}
+
+use t::lib::Test qw/connect_ok @CALL_FUNCS/;
+use Test::More;
+use DBD::SQLite;
+#use Test::NoWarnings;
+
+my @methods = qw(
+	commit rollback
+);
+
+plan tests => 2 * (6 + @methods) + 2 * @CALL_FUNCS * (14 + ($DBD::SQLite::sqlite_version_number >= 3006011) * 2);
+
+local $SIG{__WARN__} = sub {};  # to hide warnings/error messages
+
+# DBI methods
+
+for my $autocommit (0, 1) {
+	my $dbh = connect_ok( RaiseError => 1, PrintError => 0, AutoCommit => $autocommit );
+	$dbh->do('create table foo (id, text)');
+	$dbh->do('insert into foo values(?,?)', undef, 1, 'text');
+	{
+		local $@;
+		eval { $dbh->disconnect };
+		ok !$@, "disconnected";
+	}
+
+	for my $method (@methods) {
+		local $@;
+		eval { $dbh->$method };
+		ok $@, "$method dies with error: $@";
+	}
+
+	{
+		local $@;
+		eval { $dbh->last_insert_id(undef, undef, undef, undef) };
+		ok $@, "last_insert_id dies with error: $@";
+	}
+
+	{
+		local $@;
+		eval { $dbh->do('insert into foo (?,?)', undef, 2, 'text2') };
+		ok $@, "do dies with error: $@";
+	}
+
+	{
+		local $@;
+		eval { $dbh->selectrow_arrayref('select * from foo') };
+		ok $@, "selectrow_arrayref dies with error: $@";
+	}
+
+	{ # this should be the last test in this block
+		local $@;
+		eval { local $dbh->{AutoCommit} };
+		ok !$@, "store doesn't cause segfault";
+	}
+}
+
+# SQLite private methods
+
+for my $call_func (@CALL_FUNCS) {
+	for my $autocommit (0, 1) {
+		my $dbh = connect_ok( RaiseError => 1, PrintError => 0, AutoCommit => $autocommit );
+		$dbh->do('create table foo (id, text)');
+		$dbh->do('insert into foo values(?,?)', undef, 1, 'text');
+		{
+			local $@;
+			eval { $dbh->disconnect };
+			ok !$@, "disconnected";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(500, 'busy_timeout') };
+			ok $@, "busy timeout dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func('now', 0, sub { time }, 'create_function') };
+			ok $@, "create_function dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(1, 'enable_load_extension') };
+			ok $@, "enable_load_extension dies with error: $@";
+		}
+
+		{
+			package count_aggr;
+
+			sub new {
+				bless { count => 0 }, shift;
+			}
+
+			sub step {
+				$_[0]{count}++;
+				return;
+			}
+
+			sub finalize {
+				my $c = $_[0]{count};
+				$_[0]{count} = undef;
+
+				return $c;
+			}
+
+			package main;
+
+			local $@;
+			eval { $dbh->$call_func('newcount', 0, 'count_aggr', 'create_aggregate') };
+			ok $@, "create_aggregate dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func('by_num', sub ($$) {0}, 'create_collation') };
+			ok $@, "create_collation dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func('by_num', sub ($$) {0}, 'create_collation') };
+			ok $@, "create_collation dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(sub {1}, 'collation_needed') };
+			ok $@, "collation_needed dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(50, sub {}, 'progress_handler') };
+			ok $@, "progress_handler dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(sub {}, 'commit_hook') };
+			ok $@, "commit hook dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(sub {}, 'rollback_hook') };
+			ok $@, "rollback hook dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(sub {}, 'update_hook') };
+			ok $@, "update hook dies with error: $@";
+		}
+
+		{
+			local $@;
+			eval { $dbh->$call_func(undef, 'set_authorizer') };
+			ok $@, "set authorizer dies with error: $@";
+		}
+
+		if ($DBD::SQLite::sqlite_version_number >= 3006011) {
+			local $@;
+			eval { $dbh->$call_func('./backup_file', 'backup_from_file') };
+			ok $@, "backup from file dies with error: $@";
+		}
+
+		if ($DBD::SQLite::sqlite_version_number >= 3006011) {
+			local $@;
+			eval { $dbh->$call_func('./backup_file', 'backup_to_file') };
+			ok $@, "backup to file dies with error: $@";
+		}
+	}
+}