Skip to content

Commit

Permalink
Applied a patch from Rob to fix RT-97598, and added a test
Browse files Browse the repository at this point in the history
  • Loading branch information
charsbar committed Jul 29, 2014
1 parent 371cfe8 commit 539d79f
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 20 deletions.
70 changes: 50 additions & 20 deletions dbdimp.c
Expand Up @@ -358,6 +358,7 @@ sqlite_db_login6(SV *dbh, imp_dbh_t *imp_dbh, char *dbname, char *user, char *pa
imp_dbh->allow_multiple_statements = FALSE;
imp_dbh->use_immediate_transaction = TRUE;
imp_dbh->see_if_its_a_number = FALSE;
imp_dbh->stmt_list = NULL;

sqlite3_busy_timeout(imp_dbh->db, SQL_TIMEOUT);

Expand Down Expand Up @@ -482,26 +483,31 @@ sqlite_db_disconnect(SV *dbh, imp_dbh_t *imp_dbh)

croak_if_db_is_null();

rc = sqlite3_close(imp_dbh->db);
if (rc != SQLITE_OK) {
/*
** Most probably we still have unfinalized statements.
** Let's try to close them.
*/
/* COMPAT: sqlite3_next_stmt is only available for 3006000 or newer */
while ( (pStmt = sqlite3_next_stmt(imp_dbh->db, 0)) != NULL ) {
sqlite3_finalize(pStmt);
}

rc = sqlite3_close(imp_dbh->db);
if (rc != SQLITE_OK) {
/*
** We still have problems. probably a backup operation
** is not finished. We may need to wait for a while if
** we get SQLITE_BUSY...
*/
sqlite_error(dbh, rc, sqlite3_errmsg(imp_dbh->db));
sqlite_trace( dbh, imp_dbh, 1, "Closing DB" );
rc = sqlite3_close( imp_dbh->db );
sqlite_trace( dbh, imp_dbh, 1, form("rc = %d", rc) );
if ( SQLITE_BUSY == rc ) { /* We have unfinalized statements */
/* Only close the statements that were prepared by this module */
stmt_list_s * s;
while ( s = imp_dbh->stmt_list ) {
sqlite_trace( dbh, imp_dbh, 1, form("Finalizing statement (%p)", s->stmt) );
sqlite3_finalize( s->stmt );
imp_dbh->stmt_list = s->prev;
sqlite3_free( s );
}
imp_dbh->stmt_list = NULL;
sqlite_trace( dbh, imp_dbh, 1, "Trying to close DB again" );
rc = sqlite3_close( imp_dbh->db );
}
if ( SQLITE_OK != rc ) {
sqlite_error(dbh, rc, sqlite3_errmsg(imp_dbh->db));
}
/* The list should be empty at this point, but if for some unforseen reason
it isn't, free remaining nodes here */
stmt_list_s * s;
while( s = imp_dbh->stmt_list ) {
imp_dbh->stmt_list = s->prev;
sqlite3_free( s );
}
imp_dbh->db = NULL;

Expand Down Expand Up @@ -701,6 +707,12 @@ sqlite_st_prepare_sv(SV *sth, imp_sth_t *imp_sth, SV *sv_statement, SV *attribs)
else {
imp_sth->unprepared_statements = NULL;
}
/* Add the statement to the front of the list to keep track of
statements that might need to be finalized later on disconnect */
stmt_list_s * new_stmt = (stmt_list_s *) sqlite3_malloc( sizeof(stmt_list_s) );
new_stmt->stmt = imp_sth->stmt;
new_stmt->prev = imp_dbh->stmt_list;
imp_dbh->stmt_list = new_stmt;

DBIc_NUM_PARAMS(imp_sth) = sqlite3_bind_parameter_count(imp_sth->stmt);
DBIc_NUM_FIELDS(imp_sth) = sqlite3_column_count(imp_sth->stmt);
Expand Down Expand Up @@ -1143,11 +1155,29 @@ sqlite_st_destroy(SV *sth, imp_sth_t *imp_sth)
croak_if_stmt_is_null();

/* finalize sth when active connection */
sqlite_trace( sth, imp_sth, 1, form("Finalizing statement: %p", 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));
}

/* find the statement in the statement list and delete it */
stmt_list_s * i = imp_dbh->stmt_list;
stmt_list_s * temp = i;
while( i ) {
if ( i->stmt == imp_sth->stmt ) {
if ( temp != i ) temp->prev = i->prev;
if ( i == imp_dbh->stmt_list ) imp_dbh->stmt_list = i->prev;
sqlite_trace( sth, imp_sth, 1, form("Removing statement from list: %p", imp_sth->stmt) );
sqlite3_free( i );
break;
}
else {
temp = i;
i = i->prev;
}
}
imp_sth->stmt = NULL;
}
}
SvREFCNT_dec((SV*)imp_sth->params);
Expand Down
9 changes: 9 additions & 0 deletions dbdimp.h
Expand Up @@ -16,6 +16,14 @@
#define sqlite3_int64 sqlite_int64
#endif

/* A linked list of statements prepared by this module */
typedef struct stmt_list_s stmt_list_s;

struct stmt_list_s {
sqlite3_stmt * stmt;
stmt_list_s * prev;
};

/* Driver Handle */
struct imp_drh_st {
dbih_drc_t com;
Expand All @@ -36,6 +44,7 @@ struct imp_dbh_st {
bool allow_multiple_statements;
bool use_immediate_transaction;
bool see_if_its_a_number;
stmt_list_s * stmt_list;
};

/* Statement Handle */
Expand Down
38 changes: 38 additions & 0 deletions t/rt_97598_crash_on_disconnect_with_virtual_tables.t
@@ -0,0 +1,38 @@
#!/usr/bin/perl

use strict;
BEGIN {
$| = 1;
$^W = 1;
}

use t::lib::Test;
use Test::More tests => 3;
use Test::NoWarnings;

my $dbh = connect_ok(AutoCommit => 0);

$dbh->do($_) for (
'CREATE VIRTUAL TABLE test_fts USING fts4 (
col1,
col2,
)',
'INSERT INTO test_fts (col1, col2) VALUES ("abc", "123")',
'INSERT INTO test_fts (col1, col2) VALUES ("def", "456")',
'INSERT INTO test_fts (col1, col2) VALUES ("abc", "123")',
'INSERT INTO test_fts (col1, col2) VALUES ("def", "456")',
'INSERT INTO test_fts (col1, col2) VALUES ("abc", "123")',
);

my $sth = $dbh->prepare('SELECT * FROM test_fts WHERE col2 MATCh "123"');
$sth->execute;

while ( my @row = $sth->fetchrow_array ) {
note join " ", @row;
}
#$sth->finish;

$dbh->commit;
$dbh->disconnect;

pass "all done without segfault";

0 comments on commit 539d79f

Please sign in to comment.