PadreIDE/Padre

Tests fail with DBD-SQLite-1.48: DBD::SQLite::db do failed: Safety level may not be changed inside a transaction

Closed this issue · 7 comments

I have Padre-0.90 (because later versions require Wx::Scintilla which so not available for me owing to bundling) and for example t/15-locale.t test fails 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 

This worked with DBD-SQLite 1.46. It fails with 1.48. I bisected the problem to DBD-SQLite's 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)

[...]
--- a/lib/DBD/SQLite.pm
+++ b/lib/DBD/SQLite.pm
@@ -205,6 +205,16 @@ sub prepare {
 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')) {
+            return DBD::SQLite::db::_do($dbh, $statement);
+        }
+    }
+
     my @copy = @{[@bind_values]};
     my $rows = 0;

It can be a new bug in DBD-SQLite, or misuse in Padre.

Can somebody verify this is still issue with latest Padre?

If you think this is DBD-SQLite's bug, can you please help me to minimize the reproducer so it become useful bug report for DBD-SQLite?

I reported this issue to DBD-SQLite too DBD-SQLite/DBD-SQLite#10.

DBD-SQLite maintainer's response was fast and accurate. The bug is in Padre::Locker. This patch fixes it:

From da952f1dcfb7d2bafd963b55af6338a93e11f2c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
Date: Fri, 26 Jun 2015 08:18:26 +0200
Subject: [PATCH] Change synchronous SQLite level before opening transaction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since DBD-SQLite-1.48, t/15-locale.t test fails with:

DBD::SQLite::db do failed: Safety level may not be changed inside a transaction

DBD-SQLite-1.48 optimization revealed a bug in Padre::Locker where
SQLite's pragma synchronous was set after Padre::DBD->begin was
called.

This patch changes the order of the pragma() and begin() calls to
conform to SQLite requirements.

<https://github.com/PadreIDE/Padre/issues/17>
<https://github.com/DBD-SQLite/DBD-SQLite/issues/10>

Signed-off-by: Petr Písař <ppisar@redhat.com>
---
 lib/Padre/Locker.pm | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/Padre/Locker.pm b/lib/Padre/Locker.pm
index 7eaf919..009466b 100644
--- a/lib/Padre/Locker.pm
+++ b/lib/Padre/Locker.pm
@@ -102,8 +102,6 @@ sub shutdown {
 sub db_increment {
    my $self = shift;
    unless ( $self->{db_depth}++ ) {
-       Padre::DB->begin;
-
        # Database operations we lock on are the most likely to
        # involve writes. So opportunistically prevent blocking
        # on filesystem sync confirmation. This should make
@@ -111,6 +109,7 @@ sub db_increment {
        # corruption if (and only if) there is a power outage,
        # operating system crash, or catastrophic hardware failure.
        Padre::DB->pragma( synchronous => 0 );
+       Padre::DB->begin;
    }
    return;
 }
-- 
2.1.0

This patch worked for me with Padre 1.00

This fix has been merged and it will be part of the next release ( Not sure yet when this will happen ).

This patch worked for me with Padre 1.00 as well - thank you very much ppisar, you have saved my day.

kzl86 commented

Also, I couldn't start Padre 1.00 under PCBSD10. Now it works, thanks to you.

Three years later: Padre is up to date (1.00)
Bug is still in cpan and cpanm.