Skip to content

Commit

Permalink
FIX storesafe#204: close db in db.executeSql callback
Browse files Browse the repository at this point in the history
  • Loading branch information
Christopher J. Brody committed Apr 25, 2016
1 parent 764aef2 commit 7136343
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 33 deletions.
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changes

## 1.2.2-0xxx-dev

- Fix #204: close db in db.executeSql callback

## 1.2.1

- Close Android SQLiteStatement after INSERT/UPDATE/DELETE
Expand Down
10 changes: 4 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ This will invalidate **all** handle access handle objects for the database that
db.close(successcb, errorcb);
```

It is OK to close the database within a transaction callback but *NOT* within a statement callback. The following example is OK:
It is OK to close the database within a transaction callback but *NOT* within a statement callback within a transaction. The following example is OK:

```Javascript
db.transaction(function(tx) {
Expand Down Expand Up @@ -611,24 +611,22 @@ db.transaction(function(tx) {
});
```

**BUG:** It is currently NOT possible to close a database in a `db.executeSql` callback. For example:
It is also possible to close a database within a `db.executeSql` callback. For example:

```Javascript
// BROKEN DUE TO BUG:
db.executeSql("SELECT LENGTH('tenletters') AS stringlength", [], function (res) {
var stringlength = res.rows.item(0).stringlength;
console.log('got stringlength: ' + res.rows.item(0).stringlength);

// BROKEN - this will trigger the error callback DUE TO BUG:
db.close(function() {
console.log('database is closed ok');
console.log('database is closed OK');
}, function(error) {
console.log('ERROR closing database');
});
});
```

**SECOND BUG:** When a database connection is closed, any queued transactions are left hanging. All pending transactions should be errored when a database connection is closed.
**BUG:** When a database connection is closed, any queued transactions are left hanging. All pending transactions should be errored when a database connection is closed.

**NOTE:** As described above, if multiple database access handle objects are opened for the same database and one database handle access object is closed, the database is no longer available for the other database handle objects. Possible workarounds:
- It is still possible to open one or more new database handle objects on a database that has been closed.
Expand Down
26 changes: 20 additions & 6 deletions SQLitePlugin.coffee.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,21 @@

return

# FUTURE TBD:
#SQLitePlugin::dispose = (success, error) ->
# # ...
# return

SQLitePlugin::close = (success, error) ->
if @dbname of @openDBs
if txLocks[@dbname] && txLocks[@dbname].inProgress
# XXX TBD: wait for current tx then close (??)
# XXX FUTURE TBD: wait for current tx then close ??
console.log 'cannot close: transaction is in progress'
error newSQLError 'database cannot be closed while a transaction is in progress'
return

console.log 'CLOSE database: ' + @dbname

# XXX [BUG #209] closing one db handle disables other handles to same db
delete @openDBs[@dbname]

if txLocks[@dbname] then console.log 'closing db with transaction queue length: ' + txLocks[@dbname].queue.length
Expand All @@ -277,14 +281,24 @@
# XXX TODO: better to capture the result, and report it once
# the transaction has completely finished.
# This would fix BUG #204 (cannot close db in db.executeSql() callback).
mysuccess = (t, r) -> if !!success then success r
myerror = (t, e) -> if !!error then error e

resultSet = null
errorResult = null

myfn = (tx) ->
tx.addStatement(statement, params, mysuccess, myerror)
mysuccess = (t, r) -> resultSet = r

myerror = (t, e) ->
errorResult = e
return true

tx.addStatement statement, params, mysuccess, myerror
return

@addTransaction new SQLitePluginTransaction(this, myfn, null, null, false, false)
txok = -> success resultSet
txerror = -> error errorResult

@addTransaction new SQLitePluginTransaction @, myfn, txerror, txok, false, false
return

SQLitePlugin::sqlBatch = (sqlStatements, success, error) ->
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "cordova-sqlite-storage",
"version": "1.2.1",
"version": "1.2.2-0xxx-dev",
"description": "Native interface to SQLite for PhoneGap/Cordova (core version)",
"cordova": {
"id": "cordova-sqlite-storage",
Expand Down
2 changes: 1 addition & 1 deletion plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<plugin xmlns="http://www.phonegap.com/ns/plugins/1.0"
xmlns:android="http://schemas.android.com/apk/res/android"
id="cordova-sqlite-storage"
version="1.2.1">
version="1.2.2-0xxx-dev">

<name>Cordova sqlite storage plugin (core version)</name>

Expand Down
12 changes: 5 additions & 7 deletions spec/www/spec/db-open-close-delete-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,21 +415,19 @@ var mytests = function() {
});
// ** */

// XXX TODO BROKEN [BUG #204]:
it(suiteName + ' REPRODUCE BUG: close DB in db.executeSql() callback', function (done) {
it(suiteName + ' close db in db.executeSql() callback', function (done) {
var dbName = "Close-DB-in-db-executeSql-callback.db";

openDatabase({name: dbName}, function (db) {
db.executeSql("CREATE TABLE IF NOT EXISTS tt (test_data)", [], function() {
db.close(function () {
// FUTURE TBD EXPECTED RESULT:
expect('Behavior changed - please update this test').toBe('--');
// EXPECTED RESULT:
expect(true).toBe(true);
done();
}, function (error) {
// BUG REPRODUCED:
//expect(false).toBe(true);
//expect('CLOSE ERROR' + error).toBe('--');
// NOT EXPECTED:
expect(false).toBe(true);
expect('CLOSE ERROR' + error).toBe('--');
expect(true).toBe(true);
done();
});
Expand Down
30 changes: 18 additions & 12 deletions www/SQLitePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,21 +235,27 @@
};

SQLitePlugin.prototype.executeSql = function(statement, params, success, error) {
var myerror, myfn, mysuccess;
mysuccess = function(t, r) {
if (!!success) {
return success(r);
}
};
myerror = function(t, e) {
if (!!error) {
return error(e);
}
};
var errorResult, myfn, resultSet, txerror, txok;
resultSet = null;
errorResult = null;
myfn = function(tx) {
var myerror, mysuccess;
mysuccess = function(t, r) {
return resultSet = r;
};
myerror = function(t, e) {
errorResult = e;
return true;
};
tx.addStatement(statement, params, mysuccess, myerror);
};
this.addTransaction(new SQLitePluginTransaction(this, myfn, null, null, false, false));
txok = function() {
return success(resultSet);
};
txerror = function() {
return error(errorResult);
};
this.addTransaction(new SQLitePluginTransaction(this, myfn, txerror, txok, false, false));
};

SQLitePlugin.prototype.sqlBatch = function(sqlStatements, success, error) {
Expand Down

0 comments on commit 7136343

Please sign in to comment.