Skip Menu |

This queue is for tickets about the DBD-SQLite CPAN distribution.

Report information
The Basics
Id: 97598
Status: resolved
Priority: 0/
Queue: DBD-SQLite

People
Owner: Nobody in particular
Requestors: cpan-dbd-qlite [...] twotwentytwo.net
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.42
Fixed in: (no value)



Subject: Crash on disconnect with virtual tables (FTS4)
DBD::SQLite Version: 1.42 SQLite Version: 3.8.5 Perl Version: perl 5, version 14, subversion 2 (v5.14.2) built for x86_64-linux-gnu-thread-multi Linux Version: Ubuntu 12.04.4 LTS (precise), Linux 3.8.0-33-generic x86_64 This bug seems very similar to Bug #50503 reported resolved back in 2009. https://rt.cpan.org/Public/Bug/Display.html?id=50503 The attached Perl script will cause one of these 4 crashes: - Seg fault - Glibc corrupt double-linked list - Glibc corrupt unsorted chunks - Hangs without any activity Example crash messages: Segmentation fault (core dumped) *** glibc detected *** /usr/bin/perl: free(): corrupted unsorted chunks: 0x0000000001770840 *** *** glibc detected *** /usr/bin/perl: corrupted double-linked list: 0x0000000000d13ae0 *** The problem appears to be in the sqlite_db_disconnect() function in dbdimp.c. This function attempts to call sqlite3_finalize() on all statements returned by sqlite3_next_stmt() if sqlite3_close() does not succeed. As comments in the code suggest, this is not the proper thing to do if sqlite3_close() does not succeed: /* ** This cause segfaults when we have virtual tables, as sqlite3 ** seems to try to finalize the statements for the tables (freed ** here) while closing. So we need to find other ways to do the ** right thing. */ /* COMPAT: sqlite3_next_stmt is only available for 3006000 or newer */ while ( (pStmt = sqlite3_next_stmt(imp_dbh->db, 0)) != NULL ) { sqlite3_finalize(pStmt); } sqlite3_next_stmt() will return all statements associated with the database connection, even those owned internally by virtual tables such as FTS virtual tables. These statements will end up getting finalized twice when the virtual tables are disconnected by the sqlite implementation. The solution is to keep a list of only the statements that have been prepared by the module and not finalized. If sqlite3_close() returns SQLITE_BUSY, loop through the list and finalize only those statements. Then, call sqlite3_close() again and close will succeed. The sqlite implementation will finalize the other statements as needed. I have attached a patch that fixes the issue, although it may not be implemented using the most appropriate or best practices for writing perl modules in C. Unfortunately, I am not very experienced in writing perl modules in C. The patch works by keeping a simple linked list of statements in imp_dbh_st and adding and removing statements as they are prepared and finalized respectively. Any statements that haven't been finalized on disconnect get finalized before the last attempt is made to close the connection. All test cases pass and the attached test script works as expected after applying the patch. Thanks, Rob
Subject: dbd-sqlite-1.42-disconnect.patch
diff -ruN DBD-SQLite-1.42-orig/dbdimp.c DBD-SQLite-1.42/dbdimp.c --- DBD-SQLite-1.42-orig/dbdimp.c 2014-01-08 11:44:12.000000000 -0600 +++ DBD-SQLite-1.42/dbdimp.c 2014-07-28 11:29:36.454949336 -0500 @@ -299,6 +299,7 @@ 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); @@ -423,7 +424,34 @@ croak_if_db_is_null(); - rc = sqlite3_close(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 ); + } + +#if 0 if (rc != SQLITE_OK) { /* ** Most probably we still have unfinalized statements. @@ -444,7 +472,7 @@ sqlite_error(dbh, rc, sqlite3_errmsg(imp_dbh->db)); } } - +#endif av_undef(imp_dbh->functions); SvREFCNT_dec(imp_dbh->functions); imp_dbh->functions = (AV *)NULL; @@ -632,6 +660,12 @@ 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); @@ -1076,11 +1110,29 @@ 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); diff -ruN DBD-SQLite-1.42-orig/dbdimp.h DBD-SQLite-1.42/dbdimp.h --- DBD-SQLite-1.42-orig/dbdimp.h 2013-05-29 00:46:50.000000000 -0500 +++ DBD-SQLite-1.42/dbdimp.h 2014-07-28 11:29:39.682949339 -0500 @@ -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; @@ -36,6 +44,7 @@ bool allow_multiple_statements; bool use_immediate_transaction; bool see_if_its_a_number; + stmt_list_s * stmt_list; }; /* Statement Handle */
Subject: dbd-sqlite-bug.pl
#!/usr/bin/perl use warnings; use strict; use DBI; my $dbfile = "test.db"; unlink $dbfile; my $dbh = DBI->connect("dbi:SQLite:$dbfile"); $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; $dbh->do( <<EOF ); create virtual table test_fts using fts4 ( col1, col2, ) EOF $dbh->commit; sub insert { my ($dbh, $val1, $val2) = @_; my $sth = $dbh->prepare( "insert or replace into test_fts ( col1, col2 ) values ( ?, ? )" ); $sth->execute( $val1, $val2 ) or die "sth->execute() failed: $DBI::errstr. Stopped"; #$sth->finish; #$dbh->commit; } insert $dbh, "abc", "123"; insert $dbh, "def", "456"; insert $dbh, "abc", "123"; insert $dbh, "def", "456"; insert $dbh, "abc", "123"; $dbh->commit; my $sth = $dbh->prepare("select * from test_fts where col2 match '123'"); $sth->execute; while ( my @row = $sth->fetchrow_array ) { #print Dumper @row; print join " ", @row, "\n"; } #$sth->finish; $dbh->commit; $dbh->disconnect; print "The end\n";
Thanks for the awesome patch! Applied to the master, and shipped 1.43_07 with the fix. https://github.com/DBD-SQLite/DBD-SQLite/commit/539d79f080bbc0a6db8d2d690b9aa4536931e8c0 On Wed Jul 30 01:04:39 2014, http://invalid-username.pip.verisignlabs.com/ wrote: Show quoted text
> DBD::SQLite Version: 1.42 > SQLite Version: 3.8.5 > Perl Version: perl 5, version 14, subversion 2 (v5.14.2) built for > x86_64-linux-gnu-thread-multi > Linux Version: Ubuntu 12.04.4 LTS (precise), Linux 3.8.0-33-generic > x86_64 > > This bug seems very similar to Bug #50503 reported resolved back in > 2009. > https://rt.cpan.org/Public/Bug/Display.html?id=50503 > > The attached Perl script will cause one of these 4 crashes: > > - Seg fault > - Glibc corrupt double-linked list > - Glibc corrupt unsorted chunks > - Hangs without any activity > > Example crash messages: > Segmentation fault (core dumped) > *** glibc detected *** /usr/bin/perl: free(): corrupted unsorted > chunks: 0x0000000001770840 *** > *** glibc detected *** /usr/bin/perl: corrupted double-linked list: > 0x0000000000d13ae0 *** > > The problem appears to be in the sqlite_db_disconnect() function in > dbdimp.c. This function attempts to call sqlite3_finalize() on all > statements returned by sqlite3_next_stmt() if sqlite3_close() does not > succeed. > > As comments in the code suggest, this is not the proper thing to do if > sqlite3_close() does not succeed: > > /* > ** This cause segfaults when we have virtual tables, as sqlite3 > ** seems to try to finalize the statements for the tables (freed > ** here) while closing. So we need to find other ways to do the > ** right thing. > */ > /* COMPAT: sqlite3_next_stmt is only available for 3006000 or newer */ > while ( (pStmt = sqlite3_next_stmt(imp_dbh->db, 0)) != NULL ) { > sqlite3_finalize(pStmt); > } > > sqlite3_next_stmt() will return all statements associated with the > database connection, even those owned internally by virtual tables > such as FTS virtual tables. These statements will end up getting > finalized twice when the virtual tables are disconnected by the sqlite > implementation. > > The solution is to keep a list of only the statements that have been > prepared by the module and not finalized. If sqlite3_close() returns > SQLITE_BUSY, loop through the list and finalize only those statements. > Then, call sqlite3_close() again and close will succeed. The sqlite > implementation will finalize the other statements as needed. > > I have attached a patch that fixes the issue, although it may not be > implemented using the most appropriate or best practices for writing > perl modules in C. Unfortunately, I am not very experienced in writing > perl modules in C. > > The patch works by keeping a simple linked list of statements in > imp_dbh_st and adding and removing statements as they are prepared and > finalized respectively. Any statements that haven't been finalized on > disconnect get finalized before the last attempt is made to close the > connection. > > All test cases pass and the attached test script works as expected > after applying the patch. > > Thanks, > Rob
Closed as 1.44 was released. Thanks.