Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DBD-SQLite-1.48 terminates Padre-0.90 test with: Safety level may not be changed inside a transaction #10

Closed
ppisar opened this Issue Jun 23, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@ppisar
Copy link

ppisar commented Jun 23, 2015

As I reported to Padre PadreIDE/Padre#17, Padre's test suite fails after upgrading DBD-SQLite from 1.46 to 1.48 like this:

t/15-locale.t .. 
1..7
Xlib:  extension "RANDR" missing on display ":99".
ok 1 - An object of class 'Padre' isa 'Padre'
ok 2 - An object of class 'Padre::Wx::Main' isa 'Padre::Wx::Main'
DBD::SQLite::db do failed: Safety level may not be changed inside a transaction at (eval 183) line 37.
ok 3 - no warnings
# Looks like you planned 7 tests but ran 3.
# Looks like your test exited with 255 just after 3.
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 4/7 subtests 

I bisected the DBD-SQLite to the first failing commit

commit 7a234eb71b955064ce498e95d6ecc50d1f0580a7
Author: Kenichi Ishigaki <ishigaki@cpan.org>
Date:   Mon Feb 16 17:41:42 2015 +0900

    implemented a "do" shortcut for a special case (no attr, no bind params) (RT-35449)

I have not much experience with DBD::SQLite nor Padre, but simple logging inserted just before calling _do() like this:

sub do {
    my ($dbh, $statement, $attr, @bind_values) = @_;

    # shortcut
    if  (defined $statement && !defined $attr && !@bind_values) {
        # _do() (i.e. sqlite3_exec()) runs semicolon-separate SQL
        # statements, which is handy but insecure sometimes.
        # Use this only when it's safe or explicitly allowed.
        if (index($statement, ';') == -1 or $dbh->FETCH('sqlite_allow_multiple_statements')) {
            warn 'do(): calling _do() on $statement=<' . $statement . '>';
            return DBD::SQLite::db::_do($dbh, $statement);
        }
    }

shows the triggering SQL statement is:

do(): calling _do() on $statement=<pragma synchronous = 0> at /tmp/DBD-SQLite/blib/lib/DBD/SQLite.pm line 214.
DBD::SQLite::db do failed: Safety level may not be changed inside a transaction at (eval 181) line 37.

Do you think this is a bug in DBD-SQLite? Can I help you somehow with the debugging?

@charsbar

This comment has been minimized.

Copy link
Contributor

charsbar commented Jun 23, 2015

As the error message ("Safety level (=synchronous level) may not be changed inside a transaction") clearly states, this is fundamentally a bug in Padre::Locker that calls begin before pragma synchronous = 0, which is not allowed by SQLite itself (if you're interested, grep the error message at https://metacpan.org/source/ISHIGAKI/DBD-SQLite-1.48/sqlite3.c).

Unfortunately, the current implementation of DBD::SQLite uses sqlite3_reset in sqlite_st_execute, which seems to have hidden some transactional issues like this, and a part of the issues have been revealed by the new "fast_do" implementation in DBD::SQLite 1.48 that bypasses sqlite_st_execute and directly calls sqlite3_exec that calls an equivalent of sqlite3_finalize (instead of sqlite3_reset) internally.
(This sqlite3_reset usage is a long-standing issue of DBD::SQLite by itself, but it's another story).

I'm supposing removing the begin statement in Padre::Locker would work for both old and new DBD::SQLite (though I haven't tested that).

@ppisar

This comment has been minimized.

Copy link
Author

ppisar commented Jun 26, 2015

@zulcomp

This comment has been minimized.

Copy link

zulcomp commented Nov 11, 2016

removing the Padre::DB->begin statement in Padre::Locker does work. But if you put Padre::DB->begin statement after Padre::DB->pragma( synchronous => 0 ) then Padre will exit with error in Windows 10 64bit

@charsbar

This comment has been minimized.

Copy link
Contributor

charsbar commented Nov 14, 2016

@zulcomp Could you give me more information about 1) the versions of DBD::SQLite and Padre, and 2) the actual error message you are seeing (if not the same as the one shown above) ?

@zulcomp

This comment has been minimized.

Copy link

zulcomp commented Nov 14, 2016

               Padre 1.00

Core...
osname MSWin32
archname MSWin32-x64-multi-thread
Perl 5.24.0
Threads 2
RAM 122.8MB
Wx...
Wx 0.9928
WxWidgets 3.0.2
unicode 1
Alien::wxWidgets 0.67
Wx::Perl::ProcessStream 0.32
Wx::Scintilla 0.39
Other...
PPI 1.220
Debug::Client 0.29
PPIx::EditorTools 0.19
Config C:\Users\Faizul\AppData\Local\Perl\Padre

DBD::SQLite 1.35

@zulcomp

This comment has been minimized.

Copy link

zulcomp commented Nov 14, 2016

Padre just can't exit properly. Windows always prompt that the Padre not responding.

@charsbar

This comment has been minimized.

Copy link
Contributor

charsbar commented Nov 14, 2016

Could you try removing Padre::DB->begin in Padre::Locker and inserting Padre::DB->do('BEGIN IMMEDIATE TRANSACTION') after Padre::DB->pragma( synchronous => 0 ) and see if it works?

@charsbar

This comment has been minimized.

Copy link
Contributor

charsbar commented Dec 1, 2018

Closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.