From 8b35bb979aae8851320d66abf1683e0030f55dcf Mon Sep 17 00:00:00 2001 From: Kenichi Ishigaki Date: Wed, 3 Nov 2010 13:44:04 +0000 Subject: [PATCH] hopefully fixed #62370 (segfaults during operations on disconnected handle --- Changes | 2 + dbdimp.c | 108 +++++++++++- t/08_busy.t | 6 +- t/rt_62370_diconnected_handles_operation.t | 182 +++++++++++++++++++++ 4 files changed, 288 insertions(+), 10 deletions(-) create mode 100644 t/rt_62370_diconnected_handles_operation.t diff --git a/Changes b/Changes index 040c989..4e953d9 100644 --- a/Changes +++ b/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 diff --git a/dbdimp.c b/dbdimp.c index 0a1a473..86ec824 100644 --- a/dbdimp.c +++ b/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"); - return FALSE; /* -> undef in lib/DBD/SQLite.pm */ + sqlite_error(sth, -2, "attempt to prepare on inactive database handle"); + return FALSE; /* -> undef in lib/DBD/SQLite.pm */ } #if 0 if (*statement == '\0') { - sqlite_error(sth, -2, "attempt to prepare empty statement"); - return FALSE; /* -> undef in lib/DBD/SQLite.pm */ + sqlite_error(sth, -2, "attempt to prepare empty statement"); + 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; diff --git a/t/08_busy.t b/t/08_busy.t index 6e7748d..1d0f9cd 100644 --- a/t/08_busy.t +++ b/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; - undef $dbh; + unless ($^O eq 'MSWin32') { # ignore fork emulation + $dbh->{InactiveDestroy} = 1; + undef $dbh; + } my $dbh2 = DBI->connect("dbi:SQLite:$dbfile", '', '', { diff --git a/t/rt_62370_diconnected_handles_operation.t b/t/rt_62370_diconnected_handles_operation.t new file mode 100644 index 0000000..6e735c2 --- /dev/null +++ b/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: $@"; + } + } +}