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.

Sign up for GitHub

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

Rework SQLCancel logistics to avoid possible races. #567

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rework SQLCancel logistics to avoid possible races. #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions include/freetds/odbc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,31 @@ void odbc_check_struct_extra(void *p);
static inline void odbc_check_struct_extra(void *p TDS_UNUSED) {}
#endif

#define ODBC_CHECK_CANCEL(handle) \
if (IS_HSTMT(handle)) { \
TDS_STMT * _stmt = (TDS_STMT *) handle; \
if (_stmt->cancel_requested) _SQLCancel(_stmt); \
}

#define ODBC_RETURN(handle, rc) \
do { odbc_check_struct_extra(handle); \
ODBC_CHECK_CANCEL(handle) \
return handle->errs.lastrc = (rc); } while(0)
#define ODBC_RETURN_(handle) \
do { odbc_check_struct_extra(handle); \
ODBC_CHECK_CANCEL(handle) \
return handle->errs.lastrc; } while(0)

#define ODBC_EXIT(handle, rc) \
do { SQLRETURN _odbc_rc = handle->errs.lastrc = (rc); \
odbc_check_struct_extra(handle); \
ODBC_CHECK_CANCEL(handle) \
tds_mutex_unlock(&handle->mtx); \
return _odbc_rc; } while(0)
#define ODBC_EXIT_(handle) \
do { SQLRETURN _odbc_rc = handle->errs.lastrc; \
odbc_check_struct_extra(handle); \
ODBC_CHECK_CANCEL(handle) \
tds_mutex_unlock(&handle->mtx); \
return _odbc_rc; } while(0)

Expand Down Expand Up @@ -441,6 +451,9 @@ struct _hstmt
TDS_ODBC_SPECIAL_ROWS special_row;
/* do NOT free cursor, free from socket or attach to connection */
TDSCURSOR *cursor;

/* unsafe to combine with prepared_query flags */
volatile bool cancel_requested;
};

typedef struct _henv TDS_ENV;
Expand Down Expand Up @@ -587,6 +600,7 @@ void tvp_free(SQLTVP *tvp);
* odbc.c
*/

SQLRETURN _SQLCancel(TDS_STMT * stmt);
SQLRETURN _SQLRowCount(SQLHSTMT hstmt, SQLLEN FAR * pcrow);

/*
Expand Down Expand Up @@ -681,6 +695,7 @@ SQLRETURN odbc_set_concise_c_type(SQLSMALLINT concise_type, struct _drecord *dre

SQLLEN odbc_get_octet_len(int c_type, const struct _drecord *drec);
void odbc_convert_err_set(struct _sql_errors *errs, TDS_INT err);
int odbc_int_handler(void *handle);

/*
* prepare_query.c
Expand Down
77 changes: 45 additions & 32 deletions src/odbc/odbc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,15 @@ static void odbc_ird_check(TDS_STMT * stmt);
CHECK_##t##_EXTRA(n); \
odbc_errs_reset(&n->errs);

#define ODBC_ENTER_HSTMT INIT_HANDLE(STMT, stmt)
#define ODBC_ENTER_HSTMT_EX(post_cancel) \
INIT_HANDLE(STMT, stmt) \
if (stmt->cancel_requested) { \
_SQLCancel(stmt); \
post_cancel; \
}

#define ODBC_ENTER_HSTMT ODBC_ENTER_HSTMT_EX(odbc_errs_reset(&stmt->errs))
#define ODBC_ENTER_HSTMT_OR_BAIL ODBC_ENTER_HSTMT_EX(return SQL_ERROR)
#define ODBC_ENTER_HDBC INIT_HANDLE(DBC, dbc)
#define ODBC_ENTER_HENV INIT_HANDLE(ENV, env)
#define ODBC_ENTER_HDESC INIT_HANDLE(DESC, desc)
Expand Down Expand Up @@ -733,7 +741,7 @@ SQLExtendedFetch(SQLHSTMT hstmt, SQLUSMALLINT fFetchType, SQLROWOFFSET irow, SQL
SQLULEN bookmark;
SQLULEN out_len = 0;

ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLExtendedFetch(%p, %d, %d, %p, %p)\n",
hstmt, fFetchType, (int)irow, pcrow, rgfRowStatus);
Expand Down Expand Up @@ -901,7 +909,7 @@ SQLMoreResults(SQLHSTMT hstmt)
SQLUSMALLINT param_status;
int token_flags;

ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLMoreResults(%p)\n", hstmt);

Expand Down Expand Up @@ -1233,7 +1241,7 @@ SQLSetPos(SQLHSTMT hstmt, SQLSETPOSIROW irow, SQLUSMALLINT fOption, SQLUSMALLINT
TDSSOCKET *tds;
TDS_CURSOR_OPERATION op;
TDSPARAMINFO *params = NULL;
ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLSetPos(%p, %ld, %d, %d)\n",
hstmt, (long) irow, fOption, fLock);
Expand Down Expand Up @@ -1748,6 +1756,7 @@ _SQLAllocEnv(SQLHENV FAR * phenv, SQLINTEGER odbc_version)
env->tds_ctx = ctx;
ctx->msg_handler = odbc_errmsg_handler;
ctx->err_handler = odbc_errmsg_handler;
ctx->int_handler = odbc_int_handler;

/* ODBC has its own format */
free(ctx->locale->datetime_fmt);
Expand Down Expand Up @@ -1983,55 +1992,59 @@ SQLBindCol(SQLHSTMT hstmt, SQLUSMALLINT icol, SQLSMALLINT fCType, SQLPOINTER rgb
SQLRETURN ODBC_PUBLIC ODBC_API
SQLCancel(SQLHSTMT hstmt)
{
TDSSOCKET *tds;

/*
* FIXME this function can be called from other thread, do not free
* errors for this function
* If function is called from another thread errors are not touched
*/
/* TODO some tests required */
TDS_STMT *stmt = (TDS_STMT*)hstmt;
if (SQL_NULL_HSTMT == hstmt || !IS_HSTMT(hstmt))
return SQL_INVALID_HANDLE;
/* tdsdump_log(TDS_DBG_FUNC, "SQLCancel(%p)\n", hstmt); */
stmt->cancel_requested = true;
return SQL_SUCCESS;
}

SQLRETURN
_SQLCancel(TDS_STMT * stmt)
{
TDSSOCKET *tds;

tdsdump_log(TDS_DBG_FUNC, "SQLCancel(%p)\n", hstmt);
/* TODO some tests required */

tds_mutex_lock(&stmt->dbc->mtx);
if (!stmt->cancel_requested) {
tds_mutex_unlock(&stmt->dbc->mtx);
return SQL_SUCCESS;
}
tds = stmt->tds;
tds_mutex_unlock(&stmt->dbc->mtx);

tdsdump_log(TDS_DBG_FUNC, "_SQLCancel(%p)\n", stmt);

/* cancelling an inactive statement ?? */
if (!tds) {
ODBC_SAFE_ERROR(stmt);
ODBC_EXIT_(stmt);
}
if (tds_mutex_trylock(&stmt->mtx) == 0) {
stmt->cancel_requested = false;
ODBC_RETURN_(stmt);
} else {
CHECK_STMT_EXTRA(stmt);
odbc_errs_reset(&stmt->errs);
odbc_errs_add(&stmt->errs, "HY008", "Operation was cancelled");

/* FIXME test current statement */
/* FIXME here we are unlocked */

if (TDS_FAILED(tds_send_cancel(tds))) {
ODBC_SAFE_ERROR(stmt);
ODBC_EXIT_(stmt);
stmt->cancel_requested = false;
ODBC_RETURN_(stmt);
}

if (TDS_FAILED(tds_process_cancel(tds))) {
ODBC_SAFE_ERROR(stmt);
ODBC_EXIT_(stmt);
stmt->cancel_requested = false;
ODBC_RETURN_(stmt);
}

/* only if we processed cancel reset statement */
if (tds->state == TDS_IDLE)
odbc_unlock_statement(stmt);

ODBC_EXIT_(stmt);
stmt->cancel_requested = false;
ODBC_RETURN_(stmt);
}

/* don't access error here, just return error */
if (TDS_FAILED(tds_send_cancel(tds)))
return SQL_ERROR;
return SQL_SUCCESS;
}

ODBC_FUNC(SQLConnect, (P(SQLHDBC,hdbc), PCHARIN(DSN,SQLSMALLINT), PCHARIN(UID,SQLSMALLINT),
Expand Down Expand Up @@ -4166,7 +4179,7 @@ SQLFetch(SQLHSTMT hstmt)
SQLUSMALLINT *array_status_ptr;
} keep;

ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLFetch(%p)\n", hstmt);

Expand Down Expand Up @@ -4195,7 +4208,7 @@ SQLFetch(SQLHSTMT hstmt)
SQLRETURN ODBC_PUBLIC ODBC_API
SQLFetchScroll(SQLHSTMT hstmt, SQLSMALLINT FetchOrientation, SQLLEN FetchOffset)
{
ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLFetchScroll(%p, %d, %d)\n", hstmt, FetchOrientation, (int)FetchOffset);

Expand Down Expand Up @@ -5060,7 +5073,7 @@ SQLGetData(SQLHSTMT hstmt, SQLUSMALLINT icol, SQLSMALLINT fCType, SQLPOINTER rgb
TDSRESULTINFO *resinfo;
SQLLEN dummy_cb;

ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLGetData(%p, %u, %d, %p, %d, %p)\n",
hstmt, icol, fCType, rgbValue, (int)cbValueMax, pcbValue);
Expand Down Expand Up @@ -6303,7 +6316,7 @@ SQLGetTypeInfoW(SQLHSTMT hstmt, SQLSMALLINT fSqlType)
static SQLRETURN
_SQLParamData(SQLHSTMT hstmt, SQLPOINTER FAR * prgbValue)
{
ODBC_ENTER_HSTMT;
ODBC_ENTER_HSTMT_OR_BAIL;

tdsdump_log(TDS_DBG_FUNC, "SQLParamData(%p, %p) [param_num %d, param_data_called = %d]\n",
hstmt, prgbValue, stmt->param_num, stmt->param_data_called);
Expand Down
16 changes: 16 additions & 0 deletions src/odbc/odbc_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,4 +1208,20 @@ odbc_convert_err_set(struct _sql_errors *errs, TDS_INT err)
}
}


int
odbc_int_handler(void *handle)
{
TDS_STMT *stmt = (TDS_STMT*)handle;
if (SQL_NULL_HSTMT != handle)
ODBC_CHECK_CANCEL(handle);
/*
* Alternatively, this handler could conditionally return
* TDS_INT_CANCEL. NB: Combining those approaches yields SQL
* error 08S01 (Communication link failure) rather than the
* desired HY008 (Operation was cancelled).
*/
return TDS_INT_CONTINUE;
}

/** @} */