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

Transaction problem after page change, WITH POSSIBLE DATA LOSS #666

Closed
brodycj opened this issue Apr 13, 2017 · 17 comments
Closed

Transaction problem after page change, WITH POSSIBLE DATA LOSS #666

brodycj opened this issue Apr 13, 2017 · 17 comments

Comments

@brodycj
Copy link
Contributor

brodycj commented Apr 13, 2017

Discussion so far

Originally reported by @svranch in brodycj/cordova-sqlite-ext#56:

I am running in to a problem where I get this ERROR:

Unable to begin transaction: cannot start a transaction within a transaction.

It's really a bummer. 😞

My answer in brodycj/cordova-sqlite-ext#56 (comment) with my original theory:

Considering that this is a multi-page app, my theory is that this happens if the JavaScript part starts a transaction and then the app goes to another page before the transaction finishes.

In case of a multi-page app I think the safest workaround is to execute all SELECTs and changes using db.executeSql instead of using the standard transaction mechanism.

Followup from @svranch in brodycj/cordova-sqlite-ext#56 (comment):

But why do I seem to think you have to use the standard transaction mechanism? Do you know? Is it a rule or standard for sqlite? Or what?

Further explanation with my theory again

I tried to explain before that in case of a multi-page app you do NOT want to use the standard transaction mechanism such as:

db.transaction(function(tx) {
  tx.executeSql(...);
}, ...);

if your app has multiple pages and there may be a page change that happens in the middle of a transaction.

My theory is that a BEGIN statement would be executed and then no COMMIT or ROLLBACK before the app changes pages. If your app then attempts to execute another transaction the plugin would attempt to do the BEGIN and fail with the "cannot start a transaction within a transaction" error.

Data risk situation

If this situation would happen say in page 1 and then something in page 2 attempts to store data using db.executeSql or db.sqlBatch the plugin would store the data and call the success callback (if present) with the abandoned transaction still open. Once the app stops the data stored by page 2 would be lost since the transaction started by BEGIN is never closed.

Recommended workaround

In case of multi-page apps please do NOT use standard transactions (db.transaction(function(tx) { ... }, ...)). Please use single-statement transactions (db.executeSql(...)) or batch SQL (db.sqlBatch(...)) instead.

Possible alternative solutions

Alt 1: Update the native code to always open or reopen the database file upon open request from JavaScript. (The JavaScript does take care of reusing database access connections whenever possible, this should have no effect on the native side.) It may be good for the JavaScript to also send a cleanup signal upon startup.

Alt 2: Upon startup the JavaScript sends a cleanup or reset signal to the native side which should then close all existing database access connections. It would be good for the native side to open or reopen a database file connection upon request, or signal an error. This would be a variation of alt 1.

Alt 3: Whenever the JavaScript opens a new database connection it would then send a ROLLBACK SQL statement and wait for success or error callback before sending any application SQL statements for processing. This would be a good to deal with outdated platform implementations such as WP8 in Cordova-legacy-build-support.

Testing

It would be good to make a multi-page test app based on https://github.com/brodybits/cordova-sqlite-test-app (or https://github.com/brodybits/cordova-sqlite-storage-starter-app) that starts a transaction and then changes to another page that attempts db.transaction(function(tx){...},...) and then shows success or error with a message.

It should be possible for this test app to show the error with the "cannot start a transaction within a transaction" message until this issue is resolved. This test app can then be used to verify that this issue does not reappear in the future.

@brodycj
Copy link
Contributor Author

brodycj commented Apr 14, 2017

Qualification of data risk

This is only a problem until something on the second page issues a transaction. The transaction would fail and issue a ROLLBACK, causing this problem to go away. (Right now readTransaction does not use BEGIN/END though.)

First attempt to reproduce

First attempt in brodycj/cordova-sqlite-test-app#4 to reproduce in my test app did not work.

Reproduction in selfTest

See PR #667

Possible fix

See PR #668 (includes reproduction in #667)

Moving forward

I will probably base the fix off an older version that will be merged into some legacy versions such as Cordova-sqlite-evplus-legacy-workers-free, Cordova-sqlite-evplus-legacy-attach-detach-free, and Cordova-sqlite-legacy-build-support. In certain versions such as Cordova-sqlite-evplus-legacy-free I may just disable some code on the native side to avoid this data risk. The fix will then be propagated to the more current versions.

Legacy versions from 2012-2013

Cordova-SQLitePlugin-legacy-iOS version seems to be affected by this issue (https://github.com/litehelpers/Cordova-SQLitePlugin-legacy-iOS/blob/legacy-ios/src/ios/SQLitePlugin.m#L183-L185), now renamed and with description updated to indicate that it suffers from this bug.

Cordova-SQLitePlugin-legacy-WP version also seems to suffer from this bug, now renamed and with description updated to indicate this.

From inspection Cordova-SQLitePlugin-legacy-Android and the upstream https://github.com/davibe/Phonegap-SQLitePlugin versions do not suffer from the data risk reported here.

@svranch
Copy link

svranch commented Apr 17, 2017 via email

@brodycj
Copy link
Contributor Author

brodycj commented Apr 17, 2017

Does anyone happen to know where I can find documentation for the single-statement transaction methodology?

Answered in storesafe/cordova-sqlite-storage-help#17.

@brodycj brodycj changed the title Unable to begin tx [transaction within a transaction] AFTER PAGE CHANGE Transaction problem after page change, WITH POSSIBLE DATA LOSS Apr 19, 2017
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 19, 2017
Includes string test and test of effects of location reload/change
in this version branch
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 19, 2017
Includes string test and test of effects of location reload/change
in this version branch
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 19, 2017
Includes string test and test of effects of location reload/change
in this version branch
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 19, 2017
Includes string test and test of effects of location reload/change
in this version branch
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 19, 2017
Includes string test and test of effects of location reload/change
in this version branch
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 20, 2017
Includes string test and test of effects of location reload/change
in this version branch
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 20, 2017
Includes string test and test of effects of location reload/change
in this version branch along with another internal check

NOTE: selfTest with these changes also succeeds on Windows & WP8 platforms.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 20, 2017
Internally verified by selfTest function.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 21, 2017
Includes string test and test of effects of location reload/change
in this version branch along with another internal check

NOTE: selfTest with these changes also succeeds on Windows & WP8 platforms.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 21, 2017
Internally verified by selfTest function.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 21, 2017
Includes string test and test of effects of location reload/change
in this version branch along with another internal check

NOTE: selfTest with these changes also succeeds on Windows & WP8 platforms.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 21, 2017
Internally verified by selfTest function.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Apr 24, 2017
Includes string test and test of effects of location reload/change
in this version branch along with another internal check

NOTE: selfTest with these changes also succeeds on Windows & WP8 platforms.
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Jun 14, 2017
Merge branch 'cordova-sqlite-legacy-express-core' into cordova-sqlite-ext-master

Part of openDatabase in nextTick as a hack to solve problem with
workaround solution to storesafe#666 &
pre-populated database on Windows

Some additional test & doc fixes
brodycj pushed a commit to brodycj/Cordova-sqlite-storage-common-dev that referenced this issue Jun 15, 2017
Merge branch 'cordova-sqlite-legacy-express-core' into cordova-sqlite-ext-master

Part of openDatabase in nextTick as a hack to solve problem with
workaround solution to storesafe#666 &
pre-populated database on Windows

Some additional test & doc fixes
@luka5
Copy link

luka5 commented Jul 14, 2017

Hi brodybits,

the workaround solution leads to an error on android system.
I tested version 2.0.3 and 2.0.4 on android API level 25, 23 and 16 inside the standard cordova example app. Version 2.0.3 works fine, for the new one I get the error: cannot rollback - no transaction is active

Seems like we can not rollback if there was no active transaction before?

I/chromium: [INFO:CONSOLE(44)] "Received Event: deviceready", source: file:///android_asset/www/js/index.js (44)
I/chromium: [INFO:CONSOLE(175)] "OPEN database: demo.db", source: file:///android_asset/www/plugins/cordova-sqlite-storage/www/SQLitePlugin.js (175)
I/chromium: [INFO:CONSOLE(106)] "new transaction is waiting for open operation", source: file:///android_asset/www/plugins/cordova-sqlite-storage/www/SQLitePlugin.js (106)
I/chromium: [INFO:CONSOLE(179)] "OPEN database: demo.db - OK", source: file:///android_asset/www/plugins/cordova-sqlite-storage/www/SQLitePlugin.js (179)
I/chromium: [INFO:CONSOLE(80)] "DB opened: demo.db", source: file:///android_asset/www/plugins/cordova-sqlite-storage/www/SQLitePlugin.js (80)
W/System.err: java.sql.SQLException: sqlite3_step failure: cannot rollback - no transaction is active
W/System.err:     at io.liteglue.SQLiteGlueConnection$SQLGStatement.step(SQLiteGlueConnection.java:135)
W/System.err:     at io.sqlc.SQLiteConnectorDatabase.executeSQLiteStatement(SQLiteConnectorDatabase.java:214)
W/System.err:     at io.sqlc.SQLiteConnectorDatabase.executeSqlBatch(SQLiteConnectorDatabase.java:114)
W/System.err:     at io.sqlc.SQLitePlugin$DBRunner.run(SQLitePlugin.java:340)
W/System.err:     at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
W/System.err:     at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
W/System.err:     at java.lang.Thread.run(Thread.java:761)
V/executeSqlBatch: SQLitePlugin.executeSql[Batch](): Error=sqlite3_step failure: cannot rollback - no transaction is active
W/System.err: java.sql.SQLException: sqlite3_step failure: cannot rollback - no transaction is active
W/System.err:     at io.liteglue.SQLiteGlueConnection$SQLGStatement.step(SQLiteGlueConnection.java:135)
W/System.err:     at io.sqlc.SQLiteConnectorDatabase.executeSQLiteStatement(SQLiteConnectorDatabase.java:214)
W/System.err:     at io.sqlc.SQLiteConnectorDatabase.executeSqlBatch(SQLiteConnectorDatabase.java:114)
W/System.err:     at io.sqlc.SQLitePlugin$DBRunner.run(SQLitePlugin.java:340)
W/System.err:     at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
W/System.err:     at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
W/System.err:     at java.lang.Thread.run(Thread.java:761)
V/executeSqlBatch: SQLitePlugin.executeSql[Batch](): SQL Error code = 1 message = sqlite3_step failure: cannot rollback - no transaction is active

steps to reproduce:

cordova create app com.some.site app
cordova plugin add --save cordova-sqlite-storage
cordova platform add android
cordova build android
cordova run android

on device ready:
db = window.sqlitePlugin.openDatabase({name: 'demo.db', location: 'default'});

@renznamoc21
Copy link

Hi,

I currently encounter error openDatabase undefine.
after i execute this: window.sqlitePlugin.openDatabase({name: 'sample.db', location: 'default'}); in my second Javascript; my scenario trying to do is to create database the accessible throughout the application

Please help me thank you

first javascript:
var database = null;

var nextUser = 101;

function initDatabase() {
database = window.sqlitePlugin.openDatabase({name: 'sample.db', location: 'default'});

database.transaction(function(transaction) {
transaction.executeSql('CREATE TABLE IF NOT EXISTS SampleTable (name, score)');
},function (error){ navigator.notification.alert(error.message); },function() {navigator.notification.alert('success');} );
}

document.addEventListener('deviceready', function() {
initDatabase();
});

$('#submitlogin').click(function(){
database.transaction(function(transaction) {
transaction.executeSql('INSERT INTO SampleTable VALUES (?,?)',
[1, 2]);
transaction.executeSql('INSERT INTO SampleTable VALUES (?,?)',
[3, 4]);
}, function(error) {
navigator.notification.alert('ADD records after delay ERROR');
}, function() {
navigator.notification.alert('ADD 100 records after delay OK');
});

goToPage2();

});

function goToPage2()
{
window.location='outlet_selection.html';
}

2nd Javascript

var database = null;
function openDB (){
return window.sqlitePlugin.openDatabase({name: 'sample.db', location: 'default'});
}

document.addEventListener('deviceready', function() {
database = openDB();
getData();
echoTest();
selfTest();
});

function getData()
{
database.transaction(function (tx) {
tx.executeSql('SELECT name FROM SampleTable', [], function (tx, rs) {
alert(rs.rows.length);
for (i = 0; rs.rows.length > i; i++) {
rs.rows.item(i).name;
}

	}, function (tx, error) {
		alert('SELECT error: ' + error.message);
		});
},function(error){alert(error.message);});

}
function echoTest() {
window.sqlitePlugin.echoTest(function() {
navigator.notification.alert('Echo test OK');
}, function(error) {
navigator.notification.alert('Echo test ERROR: ' + error.message);
},function() {navigator.notification.alert('success');});
}

function selfTest() {
window.sqlitePlugin.selfTest(function() {
navigator.notification.alert('Self test OK');
}, function(error) {
navigator.notification.alert('Self test ERROR: ' + error.message);
},function() {navigator.notification.alert('success');});
}

NOTE: 2nd javascript is declare inside outlet_selection.html

@brodycj
Copy link
Contributor Author

brodycj commented Nov 12, 2017

Recent change in 1e0fddf to trigger ROLLBACK in the next event tick does help pass pre-populated database test in cordova-sqlite-ext but may not be right in case an application would attempt to store data directly after opening the database (without waiting for the openDatabase success callback) in a multi-page app. I hope to have a better solution later today.

@renznamoc21
Copy link

renznamoc21 commented Nov 12, 2017 via email

@brodycj
Copy link
Contributor Author

brodycj commented Dec 8, 2017

New workaround solution applied last month is to attempt to close the database before opening, ignoring any close error. Solution will be applied to the other sqlite plugin versions.

@brodycj
Copy link
Contributor Author

brodycj commented Dec 10, 2017

As demonstrated in #730 the new workaround solution will not work on built-in Android database implementation if external Android-sqlite-connector / Android-sqlite-native-driver libraries are not used. Needs further testing and investigation.

brodycj pushed a commit to brodycj/cordova-sqlite-legacy that referenced this issue Dec 14, 2017
Needed for new BUG 666 workaround solution to pass selfTest
in case of builtin android.database implementation
(no Android-sqlite-connector / Android-sqlite-native-driver libraries).

Ref:
- storesafe/cordova-sqlite-storage#730
- storesafe/cordova-sqlite-storage#666
brodycj pushed a commit to brodycj/cordova-sqlite-legacy that referenced this issue Dec 14, 2017
Close db before opening (ignore close error)

Now tested on builtin android.database implementation
(no Android-sqlite-connector / Android-sqlite-native-driver libs).

Ref:
- storesafe/cordova-sqlite-storage#666
- storesafe/cordova-sqlite-storage#730
@brodycj
Copy link
Contributor Author

brodycj commented Dec 28, 2017

As demonstrated in #730 the new workaround solution will not work on built-in Android database implementation if external Android-sqlite-connector / Android-sqlite-native-driver libraries are not used. Needs further testing and investigation.

Now fixed by 8192bc8.

I am still in the process of merging the fix to the other plugin versions, hope to finish this within the next couple days.

@brodycj
Copy link
Contributor Author

brodycj commented Jan 18, 2018

Closing with the following solution

As discussed above this issue is solved with the following workaround solution: attempt to close database by name before opening the database access handle, ignoring close error result.

In case the database was already opened before a location change, the plugin would close the database which would void any transaction in progress before opening.

This solution is now included in all LiteHelpers plugin versions with the following exceptions:

I hope to resolve these exceptions within the near future. Closing now.

@brodycj brodycj closed this as completed Jan 18, 2018
brodycj pushed a commit to litehelpers/cordova-sqlite-evmax-ext-workers-legacy-build-free that referenced this issue Feb 4, 2018
…-storage#666"

PARTIALLY reverts commit a760cc6.

(REMOVED OLD WORKAROUND SOLUTION to BUG 666;
new workaround solution also not wanted.)
@ghost
Copy link

ghost commented Jun 19, 2019

Hi, I'm using sql lite + cordova version 9.0.0 (cordova-lib@9.0.1) and so my application will be "phonegap" but I have problems finding database data, for example after a user login I do redirect with window.location.replace (url);
the data is lost, ie the database is exist but is empty.
I'm using transactions.
nota: in this moment I test only browsers and I use "cordova-sqlite-storage": "^3.2.0",
I ask you for help, thanks !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants