Skip to content

Commit

Permalink
Fix internal handling of canFreeHandle on ODBCResult::New()
Browse files Browse the repository at this point in the history
ODBCResult instances which are created from an ODBCStatement instance
should not be allowed to destroy themselves (or free the hSTMT to be
more specific) because the ODBCStatement can still be used to do stuff
with that hSTMT. Only ODBCResult instances that are created from an
ODBCConnection instance are allowed to actually destroy the hSTMT.

Previously, we were not properly derefencing the canFreeHandle flag
passed to ODBCResult::New() so it was evaluating to true in every case.
This was causing the ODBCResult::CloseSync() method to always destroy
the handle and thus cause the behaviour described in #32.
  • Loading branch information
wankdanker committed Apr 29, 2013
1 parent abbd3d6 commit 0855800
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 6 deletions.
8 changes: 6 additions & 2 deletions lib/odbc.js
Expand Up @@ -140,14 +140,18 @@ Database.prototype.query = function (sql, params, cb) {
result.fetchAll(function (err, data) {
var moreResults = result.moreResultsSync();

//close the result before calling back
//if there are not more result sets
if (!moreResults) {
result.closeSync();
}

cb(err, data, moreResults);

if (moreResults) {
return fetchMore();
}
else {
result.closeSync();

return next();
}
});
Expand Down
9 changes: 6 additions & 3 deletions src/odbc_result.cpp
Expand Up @@ -98,10 +98,10 @@ Handle<Value> ODBCResult::New(const Arguments& args) {
HENV hENV = static_cast<HENV>(js_henv->Value());
HDBC hDBC = static_cast<HDBC>(js_hdbc->Value());
HSTMT hSTMT = static_cast<HSTMT>(js_hstmt->Value());
bool canFreeHandle = static_cast<bool>(js_canFreeHandle->Value());
bool* canFreeHandle = static_cast<bool *>(js_canFreeHandle->Value());

//create a new OBCResult object
ODBCResult* objODBCResult = new ODBCResult(hENV, hDBC, hSTMT, canFreeHandle);
ODBCResult* objODBCResult = new ODBCResult(hENV, hDBC, hSTMT, *canFreeHandle);

DEBUG_PRINTF("ODBCResult::New m_hDBC=%X m_hDBC=%X m_hSTMT=%X canFreeHandle=%X\n",
objODBCResult->m_hENV,
Expand Down Expand Up @@ -645,14 +645,17 @@ Handle<Value> ODBCResult::FetchAllSync(const Arguments& args) {
*/

Handle<Value> ODBCResult::CloseSync(const Arguments& args) {
DEBUG_PRINTF("ODBCResult::Close\n");
DEBUG_PRINTF("ODBCResult::CloseSync\n");

HandleScope scope;

OPT_INT_ARG(0, closeOption, SQL_DESTROY);

ODBCResult* result = ObjectWrap::Unwrap<ODBCResult>(args.Holder());

DEBUG_PRINTF("ODBCResult::CloseSync closeOption=%i m_canFreeHandle=%i\n",
closeOption, result->m_canFreeHandle);

if (closeOption == SQL_DESTROY && result->m_canFreeHandle) {
result->Free();
}
Expand Down
8 changes: 7 additions & 1 deletion src/odbc_statement.cpp
Expand Up @@ -650,7 +650,13 @@ Handle<Value> ODBCStatement::BindSync(const Arguments& args) {
return scope.Close(True());
}
else {
//TODO: throw an error object
ThrowException(ODBC::GetSQLError(
stmt->m_hENV,
stmt->m_hDBC,
stmt->m_hSTMT,
(char *) "[node-odbc] Error in ODBCStatement::BindSync"
));

return scope.Close(False());
}

Expand Down
76 changes: 76 additions & 0 deletions test/test-prepareSync-multiple-execution.js
@@ -0,0 +1,76 @@
var common = require("./common")
, odbc = require("../")
, db = new odbc.Database()
, assert = require("assert")
;

var count = 0;
var iterations = 10;

db.open(common.connectionString, function(err) {
if(err) {
console.log(err)

return finish(1);
}

common.createTables(db, function (err, data) {
if (err) {
console.log(err);

return finish(2);
}

var stmt = db.prepareSync("insert into " + common.tableName + " (colint, coltext) VALUES (?, ?)");
assert.equal(stmt.constructor.name, "ODBCStatement");

recursive(stmt);
});
});

function finish(retValue) {
console.log("finish exit value: %s", retValue);

db.close(function (err) {
if (err) console.log (err);

process.exit(retValue || 0);
});
}

function recursive (stmt) {
try {
var result = stmt.bindSync([4, 'hello world']);
assert.equal(result, true);
}
catch (e) {
console.log(e.message);
finish(3);
}

stmt.execute(function (err, result) {
if (err) {
console.log(err.message);

return finish(4);
}

result.closeSync();
count += 1;

console.log("count %s, iterations %s", count, iterations);

if (count <= iterations) {
setTimeout(function(){
recursive(stmt);
},100);
}
else {
console.log(db.querySync("select * from " + common.tableName));

common.dropTables(db, function () {
return finish(0);
});
}
});
}

0 comments on commit 0855800

Please sign in to comment.