From a8c12dca49f79b97f17153a83bb8cb7c34abade9 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 23 Mar 2021 13:53:28 -0300 Subject: [PATCH 1/4] Check liveness in PDO_ODBC PDO drivers allow for it, and procedural ODBC has its own facility for it, but PDO_ODBC doesn't. If a connection is severed (for example, on IBM i, ending a databse job, or killing the network connection elsewhere), a persistent connection could get stuck. This adapts the procedural ODBC code to PDO for handling connection liveness, so PDO can reconnect if needed. A discussion about the method to check liveness is linked; this might not be the best method, but it's what procedural ODBC uses, so it's consistent. --- ext/pdo_odbc/odbc_driver.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index 790ee87851639..379b3fd5e53cf 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -372,6 +372,26 @@ static int odbc_handle_get_attr(pdo_dbh_t *dbh, zend_long attr, zval *val) return 0; } +static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) +{ + RETCODE ret; + UCHAR d_name[32]; + SQLSMALLINT len; + pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; + + /* + * XXX: Should we be using SQL_ATTR_CONNECTION_DEAD? Procedural ODBC + * uses this check, but it might be problematic per #20298 + */ + ret = SQLGetInfo(H->dbc, SQL_DATA_SOURCE_READ_ONLY, d_name, + sizeof(d_name), &len); + + if(ret != SQL_SUCCESS || len == 0) { + return FAILURE; + } + return SUCCESS; +} + static const struct pdo_dbh_methods odbc_methods = { odbc_handle_closer, odbc_handle_preparer, @@ -384,7 +404,7 @@ static const struct pdo_dbh_methods odbc_methods = { NULL, /* last id */ pdo_odbc_fetch_error_func, odbc_handle_get_attr, /* get attr */ - NULL, /* check_liveness */ + odbc_handle_check_liveness, /* check_liveness */ NULL, /* get_driver_methods */ NULL, /* request_shutdown */ NULL, /* in transaction, use PDO's internal tracking mechanism */ From 9a59d24a29aabc2fb0e5b791787a7abce27d6579 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Fri, 26 Mar 2021 14:39:07 -0300 Subject: [PATCH 2/4] style nitpick --- ext/pdo_odbc/odbc_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index 379b3fd5e53cf..f3b7a9cdfb531 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -386,7 +386,7 @@ static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) ret = SQLGetInfo(H->dbc, SQL_DATA_SOURCE_READ_ONLY, d_name, sizeof(d_name), &len); - if(ret != SQL_SUCCESS || len == 0) { + if (ret != SQL_SUCCESS || len == 0) { return FAILURE; } return SUCCESS; From 62545051a424403b8d327b9591bec755303a96e7 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Fri, 26 Mar 2021 14:58:30 -0300 Subject: [PATCH 3/4] Use the proper connection dead check in PDO_ODBC --- ext/pdo_odbc/odbc_driver.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index f3b7a9cdfb531..215b858d4fb8a 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -375,18 +375,13 @@ static int odbc_handle_get_attr(pdo_dbh_t *dbh, zend_long attr, zval *val) static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) { RETCODE ret; - UCHAR d_name[32]; - SQLSMALLINT len; + SQLUINTEGER dead; pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; - /* - * XXX: Should we be using SQL_ATTR_CONNECTION_DEAD? Procedural ODBC - * uses this check, but it might be problematic per #20298 - */ - ret = SQLGetInfo(H->dbc, SQL_DATA_SOURCE_READ_ONLY, d_name, - sizeof(d_name), &len); + /* ODBC 3.5; procedural ODBC uses SQLGetInfo read only instead */ + ret = SQLGetConnectAttr(H->dbc, SQL_ATTR_CONNECTION_DEAD, &dead, 0, NULL); - if (ret != SQL_SUCCESS || len == 0) { + if (ret != SQL_SUCCESS || dead == SQL_CD_TRUE) { return FAILURE; } return SUCCESS; From 12bcc691e1666f02178b16c7a42a407182cb42cc Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Tue, 30 Mar 2021 11:03:08 -0300 Subject: [PATCH 4/4] Revert back to using SQLGetInfo for PDO_ODBC Using the dead attr in the liveness check is ODBC >=3.5 and doesn't seem to work reliably across drivers; for example, the IBM i Access driver seems to have issues with it. --- ext/pdo_odbc/odbc_driver.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index 215b858d4fb8a..9d493accd3491 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -375,13 +375,18 @@ static int odbc_handle_get_attr(pdo_dbh_t *dbh, zend_long attr, zval *val) static zend_result odbc_handle_check_liveness(pdo_dbh_t *dbh) { RETCODE ret; - SQLUINTEGER dead; + UCHAR d_name[32]; + SQLSMALLINT len; pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; - /* ODBC 3.5; procedural ODBC uses SQLGetInfo read only instead */ - ret = SQLGetConnectAttr(H->dbc, SQL_ATTR_CONNECTION_DEAD, &dead, 0, NULL); + /* + * SQL_ATTR_CONNECTION_DEAD is tempting, but only in ODBC 3.5, + * and not all drivers implement it properly + */ + ret = SQLGetInfo(H->dbc, SQL_DATA_SOURCE_READ_ONLY, d_name, + sizeof(d_name), &len); - if (ret != SQL_SUCCESS || dead == SQL_CD_TRUE) { + if (ret != SQL_SUCCESS || len == 0) { return FAILURE; } return SUCCESS;