-
Notifications
You must be signed in to change notification settings - Fork 716
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
Comments
Qualification of data riskThis 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 reproduceFirst attempt in brodycj/cordova-sqlite-test-app#4 to reproduce in my test app did not work. Reproduction in selfTestSee PR #667 Possible fixSee PR #668 (includes reproduction in #667) Moving forwardI 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-2013Cordova-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. |
Until the fix is released in the main plugin, Brodybits said...
>>
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.
<<<
Does anyone happen to know where I can find documentation for the single-statement transaction methodology? -Or where/what are it's origins?
Brodybits:
Thanks so much for your valuable help on this issue.
Thanks,
Svranch
…Sent from my iPhone
On Apr 13, 2017, at 7:52 PM, Christopher J. Brody ***@***.***> wrote:
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.
|
Answered in storesafe/cordova-sqlite-storage-help#17. |
Includes string test and test of effects of location reload/change in this version branch
Includes string test and test of effects of location reload/change in this version branch
Includes string test and test of effects of location reload/change in this version branch
Includes string test and test of effects of location reload/change in this version branch
Includes string test and test of effects of location reload/change in this version branch
Includes string test and test of effects of location reload/change in this version branch
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.
Internally verified by selfTest function.
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.
Internally verified by selfTest function.
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.
Internally verified by selfTest function.
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.
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
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
Hi brodybits, the workaround solution leads to an error on android system. Seems like we can not rollback if there was no active transaction before?
steps to reproduce:
|
Hi, I currently encounter error openDatabase undefine. Please help me thank you first javascript: var nextUser = 101; function initDatabase() { database.transaction(function(transaction) { document.addEventListener('deviceready', function() { $('#submitlogin').click(function(){
}); function goToPage2() 2nd Javascript var database = null; document.addEventListener('deviceready', function() { function getData()
} function selfTest() { NOTE: 2nd javascript is declare inside outlet_selection.html |
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. |
thank you sir brody your response
…On Nov 13, 2017 02:37, "Chris Brody" ***@***.***> wrote:
Recent change in 1e0fddf
<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.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#666 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AfeOx0fftog78RNNpXUDHGkOsfruZzIJks5s1zrSgaJpZM4M9IUI>
.
|
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. |
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. |
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
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
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. |
Should never happen due to workaround solution to #666
Should never happen due to workaround solution to #666
Closing with the following solutionAs 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. |
…-storage#666" PARTIALLY reverts commit a760cc6. (REMOVED OLD WORKAROUND SOLUTION to BUG 666; new workaround solution also not wanted.)
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); |
Discussion so far
Originally reported by @svranch in brodycj/cordova-sqlite-ext#56:
My answer in brodycj/cordova-sqlite-ext#56 (comment) with my original theory:
Followup from @svranch in brodycj/cordova-sqlite-ext#56 (comment):
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:
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.
The text was updated successfully, but these errors were encountered: