Skip to content

Commit 0abc6aa

Browse files
committed
Fixed Implict Result segfault under stress
1 parent f2eff0d commit 0abc6aa

File tree

4 files changed

+8
-30
lines changed

4 files changed

+8
-30
lines changed

src/njsConnection.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ static bool njsConnection_executePostAsync(njsBaton *baton, napi_env env,
614614
metadata))
615615

616616
// return result set
617-
if (!njsResultSet_new(baton, env, baton->dpiStmtHandle, NULL,
617+
if (!njsResultSet_new(baton, env, baton->dpiStmtHandle,
618618
baton->queryVars, baton->numQueryVars, !baton->getRS,
619619
&resultSet))
620620
return false;
@@ -1336,8 +1336,8 @@ static bool njsConnection_getImplicitResults(njsBaton *baton,
13361336
implicitResult->numQueryVars, env, baton))
13371337
return false;
13381338
if (!njsResultSet_new(baton, env, implicitResult->stmt,
1339-
baton->dpiStmtHandle, implicitResult->queryVars,
1340-
implicitResult->numQueryVars, !baton->getRS, &resultSet))
1339+
implicitResult->queryVars, implicitResult->numQueryVars,
1340+
!baton->getRS, &resultSet))
13411341
return false;
13421342
implicitResult->stmt = NULL;
13431343
implicitResult->queryVars = NULL;

src/njsModule.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,6 @@ struct njsPool {
614614
struct njsResultSet {
615615
NJS_INSTANCE_HEAD
616616
dpiStmt *handle;
617-
dpiStmt *dependsOnHandle;
618617
njsConnection *conn;
619618
uint32_t numQueryVars;
620619
njsVariable *queryVars;
@@ -863,8 +862,8 @@ bool njsPool_newFromBaton(njsBaton *baton, napi_env env, napi_value *poolObj);
863862
// definition of functions for njsResultSet class
864863
//-----------------------------------------------------------------------------
865864
bool njsResultSet_new(njsBaton *baton, napi_env env, dpiStmt *handle,
866-
dpiStmt *dependsOnHandle, njsVariable *vars, uint32_t numVars,
867-
bool autoClose, napi_value *rsObj);
865+
njsVariable *vars, uint32_t numVars, bool autoClose,
866+
napi_value *rsObj);
868867

869868

870869
//-----------------------------------------------------------------------------

src/njsResultSet.c

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,7 @@ static bool njsResultSet_closeAsync(njsBaton *baton)
9999
{
100100
njsResultSet *rs = (njsResultSet*) baton->callingInstance;
101101

102-
// implicit result sets cannot be closed, but releasing the reference to
103-
// the statements will allow the parent result set to be closed
104-
if (rs->dependsOnHandle) {
105-
dpiStmt_release(rs->dependsOnHandle);
106-
rs->dependsOnHandle = NULL;
107-
} else if (dpiStmt_close(baton->dpiStmtHandle, NULL, 0) < 0) {
102+
if (dpiStmt_close(baton->dpiStmtHandle, NULL, 0) < 0) {
108103
njsBaton_setErrorDPI(baton);
109104
rs->handle = baton->dpiStmtHandle;
110105
baton->dpiStmtHandle = NULL;
@@ -165,10 +160,6 @@ static void njsResultSet_finalize(napi_env env, void *finalizeData,
165160
dpiStmt_release(rs->handle);
166161
rs->handle = NULL;
167162
}
168-
if (rs->dependsOnHandle) {
169-
dpiStmt_release(rs->dependsOnHandle);
170-
rs->dependsOnHandle = NULL;
171-
}
172163
free(rs);
173164
}
174165

@@ -235,10 +226,6 @@ static bool njsResultSet_getRowsAsync(njsBaton *baton)
235226
if ((!ok || !moreRows) && rs->autoClose) {
236227
dpiStmt_release(rs->handle);
237228
rs->handle = NULL;
238-
if (rs->dependsOnHandle) {
239-
dpiStmt_release(rs->dependsOnHandle);
240-
rs->dependsOnHandle = NULL;
241-
}
242229
}
243230

244231
return ok;
@@ -402,8 +389,7 @@ static bool njsResultSet_getRowsProcessArgs(njsBaton *baton, napi_env env,
402389
// connection.
403390
//-----------------------------------------------------------------------------
404391
bool njsResultSet_new(njsBaton *baton, napi_env env, dpiStmt *handle,
405-
dpiStmt *dependsOnHandle, njsVariable *vars, uint32_t numVars,
406-
bool autoClose, napi_value *rsObj)
392+
njsVariable *vars, uint32_t numVars, bool autoClose, napi_value *rsObj)
407393
{
408394
napi_value connObj;
409395
njsResultSet *rs;
@@ -421,15 +407,8 @@ bool njsResultSet_new(njsBaton *baton, napi_env env, dpiStmt *handle,
421407
NJS_CHECK_NAPI(env, napi_set_named_property(env, *rsObj, "_connection",
422408
connObj))
423409

424-
// if this result set depends on another ODPI-C statement remaining open,
425-
// add a referene to it to make sure it doesn't get closed (used by
426-
// implicit results)
427-
if (dependsOnHandle && dpiStmt_addRef(dependsOnHandle) < 0)
428-
return njsBaton_setErrorDPI(baton);
429-
430410
// perform some initializations
431411
rs->handle = handle;
432-
rs->dependsOnHandle = dependsOnHandle;
433412
rs->conn = (njsConnection*) baton->callingInstance;
434413
rs->numQueryVars = numVars;
435414
rs->queryVars = vars;

src/njsVariable.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ bool njsVariable_getScalarValue(njsVariable *var, njsVariableBuffer *buffer,
406406
case DPI_NATIVE_TYPE_STMT:
407407
if (dpiStmt_addRef(data->value.asStmt) < 0)
408408
return njsBaton_setErrorDPI(baton);
409-
if (!njsResultSet_new(baton, env, data->value.asStmt, NULL,
409+
if (!njsResultSet_new(baton, env, data->value.asStmt,
410410
buffer->queryVars, buffer->numQueryVars, false, value)) {
411411
dpiStmt_release(data->value.asStmt);
412412
return false;

0 commit comments

Comments
 (0)